Redesign m_ldap to no longer rely on undefined behavior

Accessing the same LDAP* from multiple threads at once is always
undefined, even if one thread is just polling ldap_result.

Instead keep one thread per connection and issue blocking queries on the
thread.

3 files changed, 385 insertions, 414 deletions

diff --git a/include/modules/ldap.h b/include/modules/ldap.h
index 65be27687..20ca71cf8 100644
--- a/include/modules/ldap.h
+++ b/include/modules/ldap.h
@@ -1,8 +1,6 @@
 #ifndef ANOPE_LDAP_H
 #define ANOPE_LDAP_H
 
-typedef int LDAPQuery;
-
 class LDAPException : public ModuleException
 {
  public:
@@ -59,28 +57,26 @@ struct LDAPAttributes : public std::map<Anope::string, std::vector<Anope::string
 	}
 };
 
+enum QueryType
+{
+	QUERY_UNKNOWN,
+	QUERY_BIND,
+	QUERY_SEARCH,
+	QUERY_ADD,
+	QUERY_DELETE,
+	QUERY_MODIFY
+};
+
 struct LDAPResult
 {
 	std::vector<LDAPAttributes> messages;
 	Anope::string error;
 
-	enum QueryType
-	{
-		QUERY_UNKNOWN,
-		QUERY_BIND,
-		QUERY_SEARCH,
-		QUERY_ADD,
-		QUERY_DELETE,
-		QUERY_MODIFY
-	};
-
 	QueryType type;
-	LDAPQuery id;
 
 	LDAPResult()
 	{
 		this->type = QUERY_UNKNOWN;
-		this->id = -1;
 	}
 
 	size_t size() const
@@ -126,48 +122,42 @@ class LDAPProvider : public Service
 
 	/** Attempt to bind to the LDAP server as an admin
 	 * @param i The LDAPInterface the result is sent to
-	 * @return The query ID
 	 */
-	virtual LDAPQuery BindAsAdmin(LDAPInterface *i) = 0;
+	virtual void BindAsAdmin(LDAPInterface *i) = 0;
 
 	/** Bind to LDAP
 	 * @param i The LDAPInterface the result is sent to
 	 * @param who The binddn
 	 * @param pass The password
-	 * @return The query ID
 	 */
-	virtual LDAPQuery Bind(LDAPInterface *i, const Anope::string &who, const Anope::string &pass) = 0;
+	virtual void Bind(LDAPInterface *i, const Anope::string &who, const Anope::string &pass) = 0;
 
 	/** Search ldap for the specified filter
 	 * @param i The LDAPInterface the result is sent to
 	 * @param base The base DN to search
 	 * @param filter The filter to apply
-	 * @return The query ID
 	 */
-	virtual LDAPQuery Search(LDAPInterface *i, const Anope::string &base, const Anope::string &filter) = 0;
+	virtual void Search(LDAPInterface *i, const Anope::string &base, const Anope::string &filter) = 0;
 
 	/** Add an entry to LDAP
 	 * @param i The LDAPInterface the result is sent to
 	 * @param dn The dn of the entry to add
 	 * @param attributes The attributes
-	 * @return The query ID
 	 */
-	virtual LDAPQuery Add(LDAPInterface *i, const Anope::string &dn, LDAPMods &attributes) = 0;
+	virtual void Add(LDAPInterface *i, const Anope::string &dn, LDAPMods &attributes) = 0;
 
 	/** Delete an entry from LDAP
 	 * @param i The LDAPInterface the result is sent to
 	 * @param dn The dn of the entry to delete
-	 * @return The query ID
 	 */
-	virtual LDAPQuery Del(LDAPInterface *i, const Anope::string &dn) = 0;
+	virtual void Del(LDAPInterface *i, const Anope::string &dn) = 0;
 
 	/** Modify an existing entry in LDAP
 	 * @param i The LDAPInterface the result is sent to
 	 * @param base The base DN to modify
 	 * @param attributes The attributes to modify
-	 * @return The query ID
 	 */
-	virtual LDAPQuery Modify(LDAPInterface *i, const Anope::string &base, LDAPMods &attributes) = 0;
+	virtual void Modify(LDAPInterface *i, const Anope::string &base, LDAPMods &attributes) = 0;
 };
 
 #endif // ANOPE_LDAP_H
diff --git a/modules/extra/m_ldap.cpp b/modules/extra/m_ldap.cpp
index 05d54e896..ff63be5c6 100644
--- a/modules/extra/m_ldap.cpp
+++ b/modules/extra/m_ldap.cpp
@@ -1,3 +1,14 @@
+/*
+ *
+ * (C) 2011-2015 Anope Team
+ * Contact us at team@anope.org
+ *
+ * Please read COPYING and README for further details.
+ *
+ * Based on the original code of Epona by Lara.
+ * Based on the original code of Services by Andy Church.
+ */
+
 /* RequiredLibraries: ldap_r,lber */
 /* RequiredWindowsLibraries: libldap_r,liblber */
 
@@ -5,8 +16,124 @@
 #include "modules/ldap.h"
 #include <ldap.h>
 
+class LDAPService;
 static Pipe *me;
 
+class LDAPRequest
+{
+ public:
+	LDAPService *service;
+	LDAPInterface *inter;
+	LDAPMessage *message; /* message returned by ldap_ */
+	LDAPResult *result; /* final result */
+	struct timeval tv;
+	QueryType type;
+
+	LDAPRequest(LDAPService *s, LDAPInterface *i)
+		: service(s)
+		, inter(i)
+		, message(NULL)
+		, result(NULL)
+	{
+		type = QUERY_UNKNOWN;
+		tv.tv_sec = 0;
+		tv.tv_usec = 100000;
+	}
+
+	virtual ~LDAPRequest()
+	{
+		delete result;
+		if (inter != NULL)
+			inter->OnDelete();
+		if (message != NULL)
+			ldap_msgfree(message);
+	}
+
+	virtual int run() = 0;
+};
+
+class LDAPBind : public LDAPRequest
+{
+	Anope::string who, pass;
+
+ public:
+	LDAPBind(LDAPService *s, LDAPInterface *i, const Anope::string &w, const Anope::string &p)
+		: LDAPRequest(s, i)
+		, who(w)
+		, pass(p)
+	{
+		type = QUERY_BIND;
+	}
+
+	int run() anope_override;
+};
+
+class LDAPSearch : public LDAPRequest
+{
+	Anope::string base;
+	Anope::string filter;
+
+ public:
+	LDAPSearch(LDAPService *s, LDAPInterface *i, const Anope::string &b, const Anope::string &f)
+		: LDAPRequest(s, i)
+		, base(b)
+		, filter(f)
+	{
+		type = QUERY_SEARCH;
+	}
+
+	int run() anope_override;
+};
+
+class LDAPAdd : public LDAPRequest
+{
+	Anope::string dn;
+	LDAPMods attributes;
+
+ public:
+	LDAPAdd(LDAPService *s, LDAPInterface *i, const Anope::string &d, const LDAPMods &attr)
+		: LDAPRequest(s, i)
+		, dn(d)
+		, attributes(attr)
+	{
+		type = QUERY_ADD;
+	}
+
+	int run() anope_override;
+};
+
+class LDAPDel : public LDAPRequest
+{
+	Anope::string dn;
+
+ public:
+	LDAPDel(LDAPService *s, LDAPInterface *i, const Anope::string &d)
+		: LDAPRequest(s, i)
+		, dn(d)
+	{
+		type = QUERY_DELETE;
+	}
+
+	int run() anope_override;
+};
+
+class LDAPModify : public LDAPRequest
+{
+	Anope::string base;
+	LDAPMods attributes;
+
+ public:
+	LDAPModify(LDAPService *s, LDAPInterface *i, const Anope::string &b, const LDAPMods &attr)
+		: LDAPRequest(s, i)
+		, base(b)
+		, attributes(attr)
+	{
+		type = QUERY_MODIFY;
+	}
+
+	int run() anope_override;
+};
+
 class LDAPService : public LDAPProvider, public Thread, public Condition
 {
 	Anope::string server;
@@ -19,7 +146,8 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 
 	time_t last_connect;
 
-	LDAPMod **BuildMods(const LDAPMods &attributes)
+ public:
+	static LDAPMod **BuildMods(const LDAPMods &attributes)
 	{
 		LDAPMod **mods = new LDAPMod*[attributes.size() + 1];
 		memset(mods, 0, sizeof(LDAPMod*) * (attributes.size() + 1));
@@ -46,7 +174,7 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 		return mods;
 	}
 
-	void FreeMods(LDAPMod **mods)
+	static void FreeMods(LDAPMod **mods)
 	{
 		for (int i = 0; mods[i] != NULL; ++i)
 		{
@@ -58,6 +186,7 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 		delete [] mods;
 	}
 
+ private:
 	void Reconnect()
 	{
 		/* Only try one connect a minute. It is an expensive blocking operation */
@@ -71,11 +200,18 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 			throw LDAPException("Unable to connect to LDAP service " + this->name + ": " + ldap_err2string(i));
 	}
 
+	void QueueRequest(LDAPRequest *r)
+	{
+		this->Lock();
+		this->queries.push_back(r);
+		this->Wakeup();
+		this->Unlock();
+	}
+
  public:
-	typedef std::map<LDAPQuery, std::pair<time_t, LDAPInterface *> > query_queue;
-	typedef std::vector<std::pair<LDAPInterface *, LDAPResult *> > result_queue;
-	query_queue queries;
-	result_queue results;
+	typedef std::vector<LDAPRequest *> query_queue;
+	query_queue queries, results;
+	Mutex process_mutex; /* held when processing requests not in either queue */
 
 	LDAPService(Module *o, const Anope::string &n, const Anope::string &s, int po, const Anope::string &b, const Anope::string &p, time_t t) : LDAPProvider(o, n), server(s), port(po), admin_binddn(b), admin_pass(p), timeout(t), last_connect(0)
 	{
@@ -96,363 +232,210 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 
 	~LDAPService()
 	{
+		/* At this point the thread has stopped so we don't need to hold process_mutex */
+
 		this->Lock();
 
-		for (query_queue::iterator it = this->queries.begin(), it_end = this->queries.end(); it != it_end; ++it)
+		for (unsigned int i = 0; i < this->queries.size(); ++i)
 		{
-			LDAPQuery msgid = it->first;
-			LDAPInterface *i = it->second.second;
+			LDAPRequest *req = this->queries[i];
 
-			ldap_abandon_ext(this->con, msgid, NULL, NULL);
-			if (i)
-				i->OnDelete();
+			/* queries have no results yet */
+			req->result = new LDAPResult();
+			req->result->type = req->type;
+			req->result->error = "LDAP Interface is going away";
+			if (req->inter)
+				req->inter->OnError(*req->result);
+
+			delete req;
 		}
 		this->queries.clear();
 
-		for (result_queue::iterator it = this->results.begin(), it_end = this->results.end(); it != it_end; ++it)
+		for (unsigned int i = 0; i < this->results.size(); ++i)
 		{
-			LDAPInterface *i = it->first;
-			LDAPResult *r = it->second;
+			LDAPRequest *req = this->results[i];
 
-			r->error = "LDAP Interface is going away";
-			if (i)
-				i->OnError(*r);
+			/* even though this may have already finished successfully we return that it didn't */
+			req->result->error = "LDAP Interface is going away";
+			if (req->inter)
+				req->inter->OnError(*req->result);
 
-			delete r;
+			delete req;
 		}
-		this->results.clear();
 
 		this->Unlock();
 
 		ldap_unbind_ext(this->con, NULL, NULL);
 	}
 	
-	LDAPQuery BindAsAdmin(LDAPInterface *i)
+	void BindAsAdmin(LDAPInterface *i) anope_override
 	{
-		return this->Bind(i, this->admin_binddn, this->admin_pass);
+		this->Bind(i, this->admin_binddn, this->admin_pass);
 	}
 
-	LDAPQuery Bind(LDAPInterface *i, const Anope::string &who, const Anope::string &pass) anope_override
+	void Bind(LDAPInterface *i, const Anope::string &who, const Anope::string &pass) anope_override
 	{
-		berval cred;
-		cred.bv_val = strdup(pass.c_str());
-		cred.bv_len = pass.length();
-
-		LDAPQuery msgid;
-		int ret = ldap_sasl_bind(con, who.c_str(), LDAP_SASL_SIMPLE, &cred, NULL, NULL, &msgid);
-		free(cred.bv_val);
-		if (ret != LDAP_SUCCESS)
-		{
-			if (ret == LDAP_SERVER_DOWN || ret == LDAP_TIMEOUT)
-			{
-				this->Reconnect();
-				return this->Bind(i, who, pass);
-			}
-			else
-				throw LDAPException(ldap_err2string(ret));
-		}
-
-		if (i != NULL)
-		{
-			this->Lock();
-			this->queries[msgid] = std::make_pair(Anope::CurTime, i);
-			this->Unlock();
-		}
-		this->Wakeup();
-
-		return msgid;
+		LDAPBind *b = new LDAPBind(this, i, who, pass);
+		QueueRequest(b);
 	}
 
-	LDAPQuery Search(LDAPInterface *i, const Anope::string &base, const Anope::string &filter) anope_override
+	void Search(LDAPInterface *i, const Anope::string &base, const Anope::string &filter) anope_override
 	{
 		if (i == NULL)
 			throw LDAPException("No interface");
 
-		LDAPQuery msgid;
-		int ret = ldap_search_ext(this->con, base.c_str(), LDAP_SCOPE_SUBTREE, filter.c_str(), NULL, 0, NULL, NULL, NULL, 0, &msgid);
-		if (ret != LDAP_SUCCESS)
-		{
-			if (ret == LDAP_SERVER_DOWN || ret == LDAP_TIMEOUT)
-			{
-				this->Reconnect();
-				return this->Search(i, base, filter);
-			}
-			else
-				throw LDAPException(ldap_err2string(ret));
-		}
-
-		this->Lock();
-		this->queries[msgid] = std::make_pair(Anope::CurTime, i);
-		this->Unlock();
-		this->Wakeup();
-
-		return msgid;
+		LDAPSearch *s = new LDAPSearch(this, i, base, filter);
+		QueueRequest(s);
 	}
 
-	LDAPQuery Add(LDAPInterface *i, const Anope::string &dn, LDAPMods &attributes) anope_override
+	void Add(LDAPInterface *i, const Anope::string &dn, LDAPMods &attributes) anope_override
 	{
-		LDAPMod **mods = this->BuildMods(attributes);
-		LDAPQuery msgid;
-		int ret = ldap_add_ext(this->con, dn.c_str(), mods, NULL, NULL, &msgid);
-		this->FreeMods(mods);
-
-		if (ret != LDAP_SUCCESS)
-		{
-			if (ret == LDAP_SERVER_DOWN || ret == LDAP_TIMEOUT)
-			{
-				this->Reconnect();
-				return this->Add(i, dn, attributes);
-			}
-			else
-				throw LDAPException(ldap_err2string(ret));
-		}
+		LDAPAdd *add = new LDAPAdd(this, i, dn, attributes);
+		QueueRequest(add);
+	}
 
-		if (i != NULL)
-		{
-			this->Lock();
-			this->queries[msgid] = std::make_pair(Anope::CurTime, i);
-			this->Unlock();
-		}
-		this->Wakeup();
+	void Del(LDAPInterface *i, const Anope::string &dn) anope_override
+	{
+		LDAPDel *del = new LDAPDel(this, i, dn);
+		QueueRequest(del);
+	}
 
-		return msgid;
+	void Modify(LDAPInterface *i, const Anope::string &base, LDAPMods &attributes) anope_override
+	{
+		LDAPModify *mod = new LDAPModify(this, i, base, attributes);
+		QueueRequest(mod);
 	}
 
-	LDAPQuery Del(LDAPInterface *i, const Anope::string &dn) anope_override
+ private:
+	void BuildReply(int res, LDAPRequest *req)
 	{
-		LDAPQuery msgid;
-		int ret = ldap_delete_ext(this->con, dn.c_str(), NULL, NULL, &msgid);
+		LDAPResult *ldap_result = req->result = new LDAPResult();
+		req->result->type = req->type;
 
-		if (ret != LDAP_SUCCESS)
+		if (res != LDAP_SUCCESS)
 		{
-			if (ret == LDAP_SERVER_DOWN || ret == LDAP_TIMEOUT)
-			{
-				this->Reconnect();
-				return this->Del(i, dn);
-			}
-			else
-				throw LDAPException(ldap_err2string(ret));
+			ldap_result->error = ldap_err2string(res);
+			return;
 		}
 
-		if (i != NULL)
+		if (req->message == NULL)
 		{
-			this->Lock();
-			this->queries[msgid] = std::make_pair(Anope::CurTime, i);
-			this->Unlock();
+			return;
 		}
-		this->Wakeup();
 
-		return msgid;
-	}
+		/* a search result */
 
-	LDAPQuery Modify(LDAPInterface *i, const Anope::string &base, LDAPMods &attributes) anope_override
-	{
-		LDAPMod **mods = this->BuildMods(attributes);
-		LDAPQuery msgid;
-		int ret = ldap_modify_ext(this->con, base.c_str(), mods, NULL, NULL, &msgid);
-		this->FreeMods(mods);
-
-		if (ret != LDAP_SUCCESS)
+		for (LDAPMessage *cur = ldap_first_message(this->con, req->message); cur; cur = ldap_next_message(this->con, cur))
 		{
-			if (ret == LDAP_SERVER_DOWN || ret == LDAP_TIMEOUT)
+			LDAPAttributes attributes;
+
+			char *dn = ldap_get_dn(this->con, cur);
+			if (dn != NULL)
 			{
-				this->Reconnect();
-				return this->Modify(i, base, attributes);
+				attributes["dn"].push_back(dn);
+				ldap_memfree(dn);
+				dn = NULL;
 			}
-			else
-				throw LDAPException(ldap_err2string(ret));
-		}
 
-		if (i != NULL)
-		{
-			this->Lock();
-			this->queries[msgid] = std::make_pair(Anope::CurTime, i);
-			this->Unlock();
-		}
-		this->Wakeup();
-
-		return msgid;
-	}
-
- private:
-	void Timeout()
-	{
-		this->Lock();
-		for (query_queue::iterator it = this->queries.begin(), it_end = this->queries.end(); it != it_end;)
-		{
-			LDAPQuery msgid = it->first;
-			time_t created = it->second.first;
-			LDAPInterface *i = it->second.second;
-			++it;
+			BerElement *ber = NULL;
 
-			if (Anope::CurTime > created + timeout)
+			for (char *attr = ldap_first_attribute(this->con, cur, &ber); attr; attr = ldap_next_attribute(this->con, cur, ber))
 			{
-				LDAPResult *ldap_result = new LDAPResult();
-				ldap_result->id = msgid;
-				ldap_result->error = "Query timed out";
+				berval **vals = ldap_get_values_len(this->con, cur, attr);
+				int count = ldap_count_values_len(vals);
 
-				this->queries.erase(msgid);
-				this->results.push_back(std::make_pair(i, ldap_result));
+				std::vector<Anope::string> attrs;
+				for (int j = 0; j < count; ++j)
+					attrs.push_back(vals[j]->bv_val);
+				attributes[attr] = attrs;
 
-				me->Notify();
+				ldap_value_free_len(vals);
+				ldap_memfree(attr);
 			}
+
+			if (ber != NULL)
+				ber_free(ber, 0);
+
+			ldap_result->messages.push_back(attributes);
 		}
-		this->Unlock();
 	}
 
- public:
-	void Run() anope_override
+	void SendRequests()
 	{
-		while (!this->GetExitState())
-		{
-			if (this->queries.empty())
-			{
-				this->Lock();
-				this->Wait();
-				this->Unlock();
-				continue;
-			}
-			else
-				this->Timeout();
-
-			struct timeval tv = { 1, 0 };
-			LDAPMessage *result;
-			int rtype = ldap_result(this->con, LDAP_RES_ANY, 1, &tv, &result);
-			if (rtype <= 0)
-				continue;
-
-			int cur_id = ldap_msgid(result);
+		process_mutex.Lock();
 
-			this->Lock();
-
-			query_queue::iterator it = this->queries.find(cur_id);
-			if (it == this->queries.end())
-			{
-				this->Unlock();
-				ldap_msgfree(result);
-				continue;
-			}
-			LDAPInterface *i = it->second.second;
-			this->queries.erase(it);
+		query_queue q;
+		this->Lock();
+		queries.swap(q);
+		this->Unlock();
 
-			this->Unlock();
+		if (q.empty())
+		{
+			process_mutex.Unlock();
+			return;
+		}
 
-			LDAPResult *ldap_result = new LDAPResult();
-			ldap_result->id = cur_id;
+		for (unsigned int i = 0; i < q.size(); ++i)
+		{
+			LDAPRequest *req = q[i];
+			int ret = req->run();
 
-			for (LDAPMessage *cur = ldap_first_message(this->con, result); cur; cur = ldap_next_message(this->con, cur))
+			if (ret == LDAP_SERVER_DOWN || ret == LDAP_TIMEOUT)
 			{
-				int cur_type = ldap_msgtype(cur);
-
-				LDAPAttributes attributes;
-
-				char *dn = ldap_get_dn(this->con, cur);
-				if (dn != NULL)
+				/* try again */
+				try
 				{
-					attributes["dn"].push_back(dn);
-					ldap_memfree(dn);
-					dn = NULL;
+					Reconnect();
 				}
-
-				switch (cur_type)
+				catch (const LDAPException &)
 				{
-					case LDAP_RES_BIND:
-						ldap_result->type = LDAPResult::QUERY_BIND;
-						break;
-					case LDAP_RES_SEARCH_ENTRY:
-						ldap_result->type = LDAPResult::QUERY_SEARCH;
-						break;
-					case LDAP_RES_ADD:
-						ldap_result->type = LDAPResult::QUERY_ADD;
-						break;
-					case LDAP_RES_DELETE:
-						ldap_result->type = LDAPResult::QUERY_DELETE;
-						break;
-					case LDAP_RES_MODIFY:
-						ldap_result->type = LDAPResult::QUERY_MODIFY;
-						break;
-					case LDAP_RES_SEARCH_RESULT:
-						// If we get here and ldap_result->type is LDAPResult::QUERY_UNKNOWN
-						// then the result set is empty
-						ldap_result->type = LDAPResult::QUERY_SEARCH;
-						break;
-					default:
-						Log(LOG_DEBUG) << "m_ldap: Unknown msg type " << cur_type;
-						continue;
 				}
 
-				switch (cur_type)
-				{
-					case LDAP_RES_BIND:
-					{
-						int errcode = -1;
-						int parse_result = ldap_parse_result(this->con, cur, &errcode, NULL, NULL, NULL, NULL, 0);
-						if (parse_result != LDAP_SUCCESS)
-							ldap_result->error = ldap_err2string(parse_result);
-						else if (errcode != LDAP_SUCCESS)
-							ldap_result->error = ldap_err2string(errcode);
-						break;
-					}
-					case LDAP_RES_SEARCH_ENTRY:
-					{
-						BerElement *ber = NULL;
-						for (char *attr = ldap_first_attribute(this->con, cur, &ber); attr; attr = ldap_next_attribute(this->con, cur, ber))
-						{
-							berval **vals = ldap_get_values_len(this->con, cur, attr);
-							int count = ldap_count_values_len(vals);
-
-							std::vector<Anope::string> attrs;
-							for (int j = 0; j < count; ++j)
-								attrs.push_back(vals[j]->bv_val);
-							attributes[attr] = attrs;
-
-							ldap_value_free_len(vals);
-							ldap_memfree(attr);
-						}
-						if (ber != NULL)
-							ber_free(ber, 0);
-
-						break;
-					}
-					case LDAP_RES_ADD:
-					case LDAP_RES_DELETE:
-					case LDAP_RES_MODIFY:
-					{
-						int errcode = -1;
-						int parse_result = ldap_parse_result(this->con, cur, &errcode, NULL, NULL, NULL, NULL, 0);
-						if (parse_result != LDAP_SUCCESS)
-							ldap_result->error = ldap_err2string(parse_result);
-						else if (errcode != LDAP_SUCCESS)
-							ldap_result->error = ldap_err2string(errcode);
-						break;
-					}
-					default:
-						continue;
-				}
-
-				ldap_result->messages.push_back(attributes);
+				ret = req->run();
 			}
 
-			ldap_msgfree(result);
+			BuildReply(ret, req);
 
 			this->Lock();
-			this->results.push_back(std::make_pair(i, ldap_result));
+			results.push_back(req);
+			this->Unlock();
+		}
+
+		me->Notify();
+
+		process_mutex.Unlock();
+	}
+
+ public:
+	void Run() anope_override
+	{
+		while (!this->GetExitState())
+		{
+			this->Lock();
+			/* Queries can be non empty if one is pushed during SendRequests() */
+			if (queries.empty())
+				this->Wait();
 			this->Unlock();
 
-			me->Notify();
+			SendRequests();
 		}
 	}
+
+	LDAP* GetConnection()
+	{
+		return con;
+	}
 };
 
 class ModuleLDAP : public Module, public Pipe
 {
 	std::map<Anope::string, LDAPService *> LDAPServices;
+
  public:
 
 	ModuleLDAP(const Anope::string &modname, const Anope::string &creator) : Module(modname, creator, EXTRA | VENDOR)
 	{
 		me = this;
-
 	}
 
 	~ModuleLDAP()
@@ -489,6 +472,8 @@ class ModuleLDAP : public Module, public Pipe
 
 				s->SetExitState();
 				s->Wakeup();
+				s->Join();
+				delete s;
 				this->LDAPServices.erase(cname);
 			}
 		}
@@ -528,32 +513,36 @@ class ModuleLDAP : public Module, public Pipe
 		for (std::map<Anope::string, LDAPService *>::iterator it = this->LDAPServices.begin(); it != this->LDAPServices.end(); ++it)
 		{
 			LDAPService *s = it->second;
+
+			s->process_mutex.Lock();
 			s->Lock();
-			for (LDAPService::query_queue::iterator it2 = s->queries.begin(); it2 != s->queries.end();)
+
+			for (unsigned int i = s->queries.size(); i > 0; --i)
 			{
-				LDAPQuery msgid = it2->first;
-				LDAPInterface *i = it2->second.second;
-				++it2;
+				LDAPRequest *req = s->queries[i - 1];
+				LDAPInterface *li = req->inter;
 
-				if (i && i->owner == m)
+				if (li && li->owner == m)
 				{
-					i->OnDelete();
-					s->queries.erase(msgid);
+					s->queries.erase(s->queries.begin() + i - 1);
+					delete req;
 				}
 			}
-			for (unsigned i = s->results.size(); i > 0; --i)
+			for (unsigned int i = s->results.size(); i > 0; --i)
 			{
-				LDAPInterface *li = s->results[i - 1].first;
-				LDAPResult *r = s->results[i - 1].second;
+				LDAPRequest *req = s->results[i - 1];
+				LDAPInterface *li = req->inter;
 
 				if (li && li->owner == m)
 				{
 					s->results.erase(s->results.begin() + i - 1);
-					delete r;
+					delete req;
 				}
 			}
+
 			s->Unlock();
-		} 
+			s->process_mutex.Unlock();
+		}
 	}
 
 	void OnNotify() anope_override
@@ -562,15 +551,16 @@ class ModuleLDAP : public Module, public Pipe
 		{
 			LDAPService *s = it->second;
 
-			LDAPService::result_queue results;
+			LDAPService::query_queue results;
 			s->Lock();
 			results.swap(s->results);
 			s->Unlock();
 
-			for (unsigned i = 0; i < results.size(); ++i)
+			for (unsigned int i = 0; i < results.size(); ++i)
 			{
-				LDAPInterface *li = results[i].first;
-				LDAPResult *r = results[i].second;
+				LDAPRequest *req = results[i];
+				LDAPInterface *li = req->inter;
+				LDAPResult *r = req->result;
 
 				if (li != NULL)
 				{
@@ -583,11 +573,50 @@ class ModuleLDAP : public Module, public Pipe
 						li->OnResult(*r);
 				}
 
-				delete r;
+				delete req;
 			}
 		} 
 	}
 };
 
+int LDAPBind::run()
+{
+	berval cred;
+	cred.bv_val = strdup(pass.c_str());
+	cred.bv_len = pass.length();
+
+	int i = ldap_sasl_bind_s(service->GetConnection(), who.c_str(), LDAP_SASL_SIMPLE, &cred, NULL, NULL, NULL);
+
+	free(cred.bv_val);
+
+	return i;
+}
+
+int LDAPSearch::run()
+{
+	return ldap_search_ext_s(service->GetConnection(), base.c_str(), LDAP_SCOPE_SUBTREE, filter.c_str(), NULL, 0, NULL, NULL, &tv, 0, &message);
+}
+
+int LDAPAdd::run()
+{
+	LDAPMod **mods = LDAPService::BuildMods(attributes);
+	int i = ldap_add_ext_s(service->GetConnection(), dn.c_str(), mods, NULL, NULL);
+	LDAPService::FreeMods(mods);
+	return i;
+}
+
+int LDAPDel::run()
+{
+	return ldap_delete_ext_s(service->GetConnection(), dn.c_str(), NULL, NULL);
+}
+
+int LDAPModify::run()
+{
+	LDAPMod **mods = LDAPService::BuildMods(attributes);
+	int i = ldap_modify_ext_s(service->GetConnection(), base.c_str(), mods, NULL, NULL);
+	LDAPService::FreeMods(mods);
+	return i;
+}
+
 MODULE_INIT(ModuleLDAP)
 
diff --git a/modules/extra/m_ldap_authentication.cpp b/modules/extra/m_ldap_authentication.cpp
index 01bafac45..23f47c5e6 100644
--- a/modules/extra/m_ldap_authentication.cpp
+++ b/modules/extra/m_ldap_authentication.cpp
@@ -30,36 +30,29 @@ struct IdentifyInfo
 
 class IdentifyInterface : public LDAPInterface
 {
-	std::map<LDAPQuery, IdentifyInfo *> requests;
+	IdentifyInfo *ii;
 
  public:
-	IdentifyInterface(Module *m) : LDAPInterface(m) { }
+	IdentifyInterface(Module *m, IdentifyInfo *i) : LDAPInterface(m), ii(i) { }
 
-	void Add(LDAPQuery id, IdentifyInfo *ii)
+	~IdentifyInterface()
 	{
-		std::map<LDAPQuery, IdentifyInfo *>::iterator it = this->requests.find(id);
-		if (it != this->requests.end())
-			delete it->second;
-		this->requests[id] = ii;
+		delete ii;
 	}
 
-	void OnResult(const LDAPResult &r) anope_override
+	void OnDelete() anope_override
 	{
-		std::map<LDAPQuery, IdentifyInfo *>::iterator it = this->requests.find(r.id);
-		if (it == this->requests.end())
-			return;
-		IdentifyInfo *ii = it->second;
-		this->requests.erase(it);
+		delete this;
+	}
 
+	void OnResult(const LDAPResult &r) anope_override
+	{
 		if (!ii->lprov)
-		{
-			delete ii;
 			return;
-		}
 
 		switch (r.type)
 		{
-			case LDAPResult::QUERY_SEARCH:
+			case QUERY_SEARCH:
 			{
 				if (!r.empty())
 				{
@@ -68,9 +61,9 @@ class IdentifyInterface : public LDAPInterface
 						const LDAPAttributes &attr = r.get(0);
 						ii->dn = attr.get("dn");
 						Log(LOG_DEBUG) << "m_ldap_authenticationn: binding as " << ii->dn;
-						LDAPQuery id = ii->lprov->Bind(this, ii->dn, ii->req->GetPassword());
-						this->Add(id, ii);
-						return;
+
+						ii->lprov->Bind(new IdentifyInterface(this->owner, ii), ii->dn, ii->req->GetPassword());
+						ii = NULL;
 					}
 					catch (const LDAPException &ex)
 					{
@@ -79,7 +72,7 @@ class IdentifyInterface : public LDAPInterface
 				}
 				break;
 			}
-			case LDAPResult::QUERY_BIND:
+			case QUERY_BIND:
 			{
 				if (ii->admin_bind)
 				{
@@ -87,10 +80,9 @@ class IdentifyInterface : public LDAPInterface
 					try
 					{
 						Log(LOG_DEBUG) << "m_ldap_authentication: searching for " << sf;
-						LDAPQuery id = ii->lprov->Search(this, basedn, sf);
-						this->Add(id, ii);
+						ii->lprov->Search(new IdentifyInterface(this->owner, ii), basedn, sf);
 						ii->admin_bind = false;
-						return;
+						ii = NULL;
 					}
 					catch (const LDAPException &ex)
 					{
@@ -120,40 +112,28 @@ class IdentifyInterface : public LDAPInterface
 			default:
 				break;
 		}
-
-		delete ii;
 	}
 
 	void OnError(const LDAPResult &r) anope_override
 	{
-		std::map<LDAPQuery, IdentifyInfo *>::iterator it = this->requests.find(r.id);
-		if (it == this->requests.end())
-			return;
-		IdentifyInfo *ii = it->second;
-		this->requests.erase(it);
-		delete ii;
 	}
 };
 
 class OnIdentifyInterface : public LDAPInterface
 {
-	std::map<LDAPQuery, Anope::string> requests;
+	Anope::string uid;
 
  public:
-	OnIdentifyInterface(Module *m) : LDAPInterface(m) { }
+	OnIdentifyInterface(Module *m, const Anope::string &i) : LDAPInterface(m), uid(i) { }
 
-	void Add(LDAPQuery id, const Anope::string &nick)
+	void OnDelete() anope_override
 	{
-		this->requests[id] = nick;
+		delete this;
 	}
 
 	void OnResult(const LDAPResult &r) anope_override
 	{
-		std::map<LDAPQuery, Anope::string>::iterator it = this->requests.find(r.id);
-		if (it == this->requests.end())
-			return;
-		User *u = User::Find(it->second);
-		this->requests.erase(it);
+		User *u = User::Find(uid);
 
 		if (!u || !u->Account() || r.empty())
 			return;
@@ -180,7 +160,6 @@ class OnIdentifyInterface : public LDAPInterface
 
 	void OnError(const LDAPResult &r) anope_override
 	{
-		this->requests.erase(r.id);
 		Log(this->owner) << r.error;
 	}
 };
@@ -201,11 +180,9 @@ class OnRegisterInterface : public LDAPInterface
 	}
 };
 
-class NSIdentifyLDAP : public Module
+class ModuleLDAPAuthentication : public Module
 {
 	ServiceReference<LDAPProvider> ldap;
-	IdentifyInterface iinterface;
-	OnIdentifyInterface oninterface;
 	OnRegisterInterface orinterface;
 
 	PrimitiveExtensibleItem<Anope::string> dn;
@@ -214,11 +191,10 @@ class NSIdentifyLDAP : public Module
 	Anope::string disable_register_reason;
 	Anope::string disable_email_reason;
  public:
-	NSIdentifyLDAP(const Anope::string &modname, const Anope::string &creator) :
-		Module(modname, creator, EXTRA | VENDOR), ldap("LDAPProvider", "ldap/main"), iinterface(this), oninterface(this), orinterface(this),
+	ModuleLDAPAuthentication(const Anope::string &modname, const Anope::string &creator) :
+		Module(modname, creator, EXTRA | VENDOR), ldap("LDAPProvider", "ldap/main"), orinterface(this),
 		dn(this, "m_ldap_authentication_dn")
 	{
-
 		me = this;
 	}
 
@@ -271,16 +247,7 @@ class NSIdentifyLDAP : public Module
 			return;
 
 		IdentifyInfo *ii = new IdentifyInfo(u, req, this->ldap);
-		try
-		{
-			LDAPQuery id = this->ldap->BindAsAdmin(&this->iinterface);
-			this->iinterface.Add(id, ii);
-		}
-		catch (const LDAPException &ex)
-		{
-			delete ii;
-			Log(this) << ex.GetReason();
-		}
+		this->ldap->BindAsAdmin(new IdentifyInterface(this, ii));
 	}
 
 	void OnNickIdentify(User *u) anope_override
@@ -292,15 +259,7 @@ class NSIdentifyLDAP : public Module
 		if (!d || d->empty())
 			return;
 
-		try
-		{
-			LDAPQuery id = this->ldap->Search(&this->oninterface, *d, "(" + email_attribute + "=*)");
-			this->oninterface.Add(id, u->nick);
-		}
-		catch (const LDAPException &ex)
-		{
-			Log(this) << ex.GetReason();
-		}
+		this->ldap->Search(new OnIdentifyInterface(this, u->GetUID()), *d, "(" + email_attribute + "=*)");
 	}
 
 	void OnNickRegister(User *, NickAlias *na, const Anope::string &pass) anope_override
@@ -308,37 +267,30 @@ class NSIdentifyLDAP : public Module
 		if (!this->disable_register_reason.empty() || !this->ldap)
 			return;
 
-		try
-		{
-			this->ldap->BindAsAdmin(NULL);
-
-			LDAPMods attributes;
-			attributes.resize(4);
-
-			attributes[0].name = "objectClass";
-			attributes[0].values.push_back("top");
-			attributes[0].values.push_back(object_class);
+		this->ldap->BindAsAdmin(NULL);
 
-			attributes[1].name = username_attribute;
-			attributes[1].values.push_back(na->nick);
+		LDAPMods attributes;
+		attributes.resize(4);
 
-			if (!na->nc->email.empty())
-			{
-				attributes[2].name = email_attribute;
-				attributes[2].values.push_back(na->nc->email);
-			}
+		attributes[0].name = "objectClass";
+		attributes[0].values.push_back("top");
+		attributes[0].values.push_back(object_class);
 
-			attributes[3].name = this->password_attribute;
-			attributes[3].values.push_back(pass);
+		attributes[1].name = username_attribute;
+		attributes[1].values.push_back(na->nick);
 
-			Anope::string new_dn = username_attribute + "=" + na->nick + "," + basedn;
-			this->ldap->Add(&this->orinterface, new_dn, attributes);
-		}
-		catch (const LDAPException &ex)
+		if (!na->nc->email.empty())
 		{
-			Log(this) << ex.GetReason();
+			attributes[2].name = email_attribute;
+			attributes[2].values.push_back(na->nc->email);
 		}
+
+		attributes[3].name = this->password_attribute;
+		attributes[3].values.push_back(pass);
+
+		Anope::string new_dn = username_attribute + "=" + na->nick + "," + basedn;
+		this->ldap->Add(&this->orinterface, new_dn, attributes);
 	}
 };
 
-MODULE_INIT(NSIdentifyLDAP)
+MODULE_INIT(ModuleLDAPAuthentication)