Added more useful functions to our LDAP API, allow adding newly registered accounts to LDAP, removed some unnecessary OnPre events and fixed unloading all modules

11 files changed, 319 insertions, 55 deletions

diff --git a/data/example.conf b/data/example.conf
index 709a7a310..24120d128 100644
--- a/data/example.conf
+++ b/data/example.conf
@@ -2059,6 +2059,8 @@ ldap
 {
 	server = "ldap://127.0.0.1"
 	port = 389
+	admin_binddn = "cn=Manager,dc=anope,dc=org"
+	admin_password = "secret"
 }
 
 /*
@@ -2076,6 +2078,12 @@ m_ldap_authentication
 	binddn = "ou=users,dc=anope,dc=org"
 
 	/*
+	 * The object class used by LDAP to store user account information.
+	 * Used for adding new users to LDAP if disable_ns_register is false
+	 */
+	object_class = "anopeUser";
+
+	/*
 	 * The attribute value used for account names.
 	 */
 	username_attribute = "uid"
@@ -2087,14 +2095,20 @@ m_ldap_authentication
 	email_attribute = "email"
 
 	/*
+	 * The attribute value used for passwords.
+	 * Used when registering new accounts in LDAP.
+	 */
+	password_attribute = "userPassword"
+
+	/*
 	 * Enable to have this module disable /nickserv register.
 	 */
-	disable_ns_register = true
+	disable_ns_register = false
 
 	/*
-	 * The reason to give the users who try to /ns register.
+	 * The reason to give the users who try to /ns register if
+	 * disable_ns_register is enabled.
 	 */
-	disable_reason = "Registration has been disabled."
 	#disable_reason = "To register on this network visit http://some.misconfigured.site/register"
 }
 
diff --git a/include/modules.h b/include/modules.h
index 415808def..c3f533609 100644
--- a/include/modules.h
+++ b/include/modules.h
@@ -579,18 +579,10 @@ class CoreExport Module : public Extensible
 	 */
 	virtual void OnDatabaseWriteMetadata(void (*WriteMetadata)(const Anope::string &, const Anope::string &), ChannelInfo *ci) { }
 
-	/** Called before services restart
-	*/
-	virtual void OnPreRestart() { }
-
 	/** Called when services restart
 	*/
 	virtual void OnRestart() { }
 
-	/** Called before services shutdown
-	 */
-	virtual void OnPreShutdown() { }
-
 	/** Called when services shutdown
 	 */
 	virtual void OnShutdown() { }
@@ -1100,7 +1092,7 @@ enum Implementation
 
 		/* Other */
 		I_OnReload, I_OnPreServerConnect, I_OnNewServer, I_OnServerConnect, I_OnPreUplinkSync, I_OnServerDisconnect, I_OnPreCommandRun,
-		I_OnPreCommand, I_OnPostCommand, I_OnPreRestart, I_OnRestart, I_OnPreShutdown, I_OnShutdown, I_OnSignal,
+		I_OnPreCommand, I_OnPostCommand, I_OnRestart, I_OnShutdown, I_OnSignal,
 		I_OnServerQuit, I_OnTopicUpdated,
 		I_OnEncrypt, I_OnDecrypt,
 		I_OnChannelModeSet, I_OnChannelModeUnset, I_OnUserModeSet, I_OnUserModeUnset, I_OnChannelModeAdd, I_OnUserModeAdd,
diff --git a/modules/core/gl_main.cpp b/modules/core/gl_main.cpp
index 84eea0e20..4b512ca0a 100644
--- a/modules/core/gl_main.cpp
+++ b/modules/core/gl_main.cpp
@@ -60,7 +60,7 @@ class GlobalCore : public Module
 		this->SetAuthor("Anope");
 		this->SetType(CORE);
 
-		Implementation i[] = { I_OnPreRestart, I_OnPreShutdown, I_OnNewServer };
+		Implementation i[] = { I_OnRestart, I_OnShutdown, I_OnNewServer };
 		ModuleManager::Attach(i, this, 3);
 
 		ModuleManager::RegisterService(&this->myglobal);
@@ -88,13 +88,13 @@ class GlobalCore : public Module
 		delete Global;
 	}
 
-	void OnPreRestart()
+	void OnRestart()
 	{
 		if (Config->GlobalOnCycle)
 			global->SendGlobal(global->Bot(), "", Config->GlobalOnCycleMessage);
 	}
 	
-	void OnPreShutdown()
+	void OnShutdown()
 	{
 		if (Config->GlobalOnCycle)
 			global->SendGlobal(global->Bot(), "", Config->GlobalOnCycleMessage);
diff --git a/modules/extra/cs_set_misc.cpp b/modules/extra/cs_set_misc.cpp
index 4bd7a772a..f71363795 100644
--- a/modules/extra/cs_set_misc.cpp
+++ b/modules/extra/cs_set_misc.cpp
@@ -75,7 +75,7 @@ class CSSetMisc : public Module
 
 	void RemoveAll()
 	{
-		if (Commands.empty())
+		if (!chanserv || Commands.empty())
 			return;
 
 		Command *set = FindCommand(chanserv->Bot(), "SET");
diff --git a/modules/extra/db_mysql_live.cpp b/modules/extra/db_mysql_live.cpp
index 8faa11a1f..f1f0b855f 100644
--- a/modules/extra/db_mysql_live.cpp
+++ b/modules/extra/db_mysql_live.cpp
@@ -163,11 +163,11 @@ class MySQLLiveModule : public Module
 	MySQLLiveModule(const Anope::string &modname, const Anope::string &creator) :
 		Module(modname, creator), SQL("mysql/main"), ACS("asynch_commands")
 	{
-		Implementation i[] = { I_OnFindChan, I_OnFindNick, I_OnFindCore, I_OnPreShutdown };
+		Implementation i[] = { I_OnFindChan, I_OnFindNick, I_OnFindCore, I_OnShutdown };
 		ModuleManager::Attach(i, this, 4);
 	}
 
-	void OnPreShutdown()
+	void OnShutdown()
 	{
 		Implementation i[] = { I_OnFindChan, I_OnFindNick, I_OnFindCore };
 		for (size_t j = 0; j < 3; ++j)
diff --git a/modules/extra/ldap.h b/modules/extra/ldap.h
index acb9d8f53..6fbba7c11 100644
--- a/modules/extra/ldap.h
+++ b/modules/extra/ldap.h
@@ -1,3 +1,5 @@
+#ifndef ANOPE_LDAP_H
+#define ANOPE_LDAP_H
 
 typedef int LDAPQuery;
 
@@ -9,6 +11,21 @@ class LDAPException : public ModuleException
 	virtual ~LDAPException() throw() { }
 };
 
+struct LDAPModification
+{
+	enum LDAPOperation
+	{
+		LDAP_ADD,
+		LDAP_DEL,
+		LDAP_REPLACE
+	};
+
+	LDAPOperation op;
+	Anope::string name;
+	std::vector<Anope::string> values;
+};
+typedef std::vector<LDAPModification> LDAPMods;
+
 struct LDAPAttributes : public std::map<Anope::string, std::vector<Anope::string> >
 {
 	size_t size(const Anope::string &attr) const
@@ -50,7 +67,10 @@ struct LDAPResult
 	enum QueryType
 	{
 		QUERY_BIND,
-		QUERY_SEARCH
+		QUERY_SEARCH,
+		QUERY_ADD,
+		QUERY_DELETE,
+		QUERY_MODIFY
 	};
 
 	QueryType type;
@@ -61,6 +81,11 @@ struct LDAPResult
 		return this->messages.size();
 	}
 
+	bool empty() const
+	{
+		return this->messages.empty();
+	}
+
 	const LDAPAttributes &get(size_t sz) const
 	{
 		if (sz >= this->messages.size())
@@ -86,14 +111,55 @@ class LDAPInterface
 	virtual void OnError(const LDAPResult &err) { }
 };
 
-
 class LDAPProvider : public Service
 {
  public:
 	LDAPProvider(Module *c, const Anope::string &n) : Service(c, n) { }
 
+	/** 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;
+
+	/** 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;
 
+	/** 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;
+
+	/** 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;
+
+	/** 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;
+
+	/** 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;
 };
 
+#endif // ANOPE_LDAP_H
diff --git a/modules/extra/m_ldap.cpp b/modules/extra/m_ldap.cpp
index 157bc70ac..188c9d02f 100644
--- a/modules/extra/m_ldap.cpp
+++ b/modules/extra/m_ldap.cpp
@@ -10,16 +10,57 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 {
 	Anope::string server;
 	int port;
+	Anope::string admin_binddn;
+	Anope::string admin_pass;
 
 	LDAP *con;
 
+	LDAPMod **BuildMods(const LDAPMods &attributes)
+	{
+		LDAPMod **mods = new LDAPMod*[attributes.size() + 1];
+		memset(mods, 0, sizeof(LDAPMod*) * (attributes.size() + 1));
+		for (unsigned x = 0; x < attributes.size(); ++x)
+		{
+			const LDAPModification &l = attributes[x];
+			mods[x] = new LDAPMod();
+
+			if (l.op == LDAPModification::LDAP_ADD)
+				mods[x]->mod_op = LDAP_MOD_ADD;
+			else if (l.op == LDAPModification::LDAP_DEL)
+				mods[x]->mod_op = LDAP_MOD_DELETE;
+			else if (l.op == LDAPModification::LDAP_REPLACE)
+				mods[x]->mod_op = LDAP_MOD_REPLACE;
+			else if (l.op != 0)
+				throw LDAPException("Unknown LDAP operation");
+			mods[x]->mod_type = strdup(l.name.c_str());
+			mods[x]->mod_values = new char*[l.values.size() + 1];
+			memset(mods[x]->mod_values, 0, sizeof(char *) * (l.values.size() + 1));
+			for (unsigned j = 0, c = 0; j < l.values.size(); ++j)
+				if (!l.values[j].empty())
+					mods[x]->mod_values[c++] = strdup(l.values[j].c_str());
+		}
+		return mods;
+	}
+
+	void FreeMods(LDAPMod **mods)
+	{
+		for (int i = 0; mods[i] != NULL; ++i)
+		{
+			free(mods[i]->mod_type);
+			for (int j = 0; mods[i]->mod_values[j] != NULL; ++j)
+				free(mods[i]->mod_values[j]);
+			delete [] mods[i]->mod_values;
+		}
+		delete [] mods;
+	}
+
  public:
 	typedef std::map<int, LDAPInterface *> query_queue;
 	typedef std::vector<std::pair<LDAPInterface *, LDAPResult *> > result_queue;
 	query_queue queries;
 	result_queue results;
 
-	LDAPService(Module *o, const Anope::string &n, const Anope::string &s, int po) : LDAPProvider(o, "ldap/" + n), server(s), port(po)
+	LDAPService(Module *o, const Anope::string &n, const Anope::string &s, int po, const Anope::string &b, const Anope::string &p) : LDAPProvider(o, "ldap/" + n), server(s), port(po), admin_binddn(b), admin_pass(p)
 	{
 		if (ldap_initialize(&this->con, this->server.c_str()) != LDAP_SUCCESS)
 			throw LDAPException("Unable to connect to LDAP service " + this->name + ": " + Anope::LastError());
@@ -52,6 +93,11 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 
 		ldap_unbind_ext(this->con, NULL, NULL);
 	}
+	
+	LDAPQuery BindAsAdmin(LDAPInterface *i)
+	{
+		return this->Bind(i, this->admin_binddn, this->admin_pass);
+	}
 
 	LDAPQuery Bind(LDAPInterface *i, const Anope::string &who, const Anope::string &pass)
 	{
@@ -94,6 +140,67 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 		return msgid;
 	}
 
+	LDAPQuery Add(LDAPInterface *i, const Anope::string &dn, LDAPMods &attributes)
+	{
+		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)
+			throw LDAPException(ldap_err2string(ret));
+
+		if (i != NULL)
+		{
+			this->Lock();
+			this->queries[msgid] = i;
+			this->Unlock();
+		}
+		this->Wakeup();
+
+		return msgid;
+	}
+
+	LDAPQuery Del(LDAPInterface *i, const Anope::string &dn)
+	{
+		LDAPQuery msgid;
+		int ret = ldap_delete_ext(this->con, dn.c_str(), NULL, NULL, &msgid);
+
+		if (ret != LDAP_SUCCESS)
+			throw LDAPException(ldap_err2string(ret));
+
+		if (i != NULL)
+		{
+			this->Lock();
+			this->queries[msgid] = i;
+			this->Unlock();
+		}
+		this->Wakeup();
+
+		return msgid;
+	}
+
+	LDAPQuery Modify(LDAPInterface *i, const Anope::string &base, LDAPMods &attributes)
+	{
+		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)
+			throw LDAPException(ldap_err2string(ret));
+
+		if (i != NULL)
+		{
+			this->Lock();
+			this->queries[msgid] = i;
+			this->Unlock();
+		}
+		this->Wakeup();
+
+		return msgid;
+	}
+
 	void Run()
 	{
 		while (!this->GetExitState())
@@ -121,6 +228,7 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 			if (it == this->queries.end())
 			{
 				this->Unlock();
+				ldap_msgfree(result);
 				continue;
 			}
 			LDAPInterface *i = it->second;
@@ -140,9 +248,30 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 				switch (cur_type)
 				{
 					case LDAP_RES_BIND:
-					{
 						ldap_result->type = LDAPResult::QUERY_BIND;
+						break;
+					case LDAP_RES_SEARCH_ENTRY:
+						ldap_result->type = LDAPResult::QUERY_SEARCH;
+					case LDAP_RES_SEARCH_RESULT:
+						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;
+					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)
@@ -153,8 +282,6 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 					}
 					case LDAP_RES_SEARCH_ENTRY:
 					{
-						ldap_result->type = LDAPResult::QUERY_SEARCH;
-
 						BerElement *ber = NULL;
 						for (char *attr = ldap_first_attribute(this->con, cur, &ber); attr; attr = ldap_next_attribute(this->con, cur, ber))
 						{
@@ -173,8 +300,19 @@ class LDAPService : public LDAPProvider, public Thread, public Condition
 
 						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:
-						Log(LOG_DEBUG) << "m_ldap: Unknown msg type " << cur_type;
 						continue;
 				}
 
@@ -262,10 +400,12 @@ class ModuleLDAP : public Module, public Pipe
 			{
 				Anope::string server = config.ReadValue("ldap", "server", "127.0.0.1", i);
 				int port = config.ReadInteger("ldap", "port", "389", i, true);
+				Anope::string admin_binddn = config.ReadValue("ldap", "admin_binddn", "", i);
+				Anope::string admin_password = config.ReadValue("ldap", "admin_password", "", i);
 
 				try
 				{
-					LDAPService *ss = new LDAPService(this, connname, server, port);
+					LDAPService *ss = new LDAPService(this, connname, server, port, admin_binddn, admin_password);
 					this->LDAPServices.insert(std::make_pair(connname, ss));
 					ModuleManager::RegisterService(ss);
 
diff --git a/modules/extra/m_ldap_authentication.cpp b/modules/extra/m_ldap_authentication.cpp
index f2caac63d..87d082e44 100644
--- a/modules/extra/m_ldap_authentication.cpp
+++ b/modules/extra/m_ldap_authentication.cpp
@@ -118,7 +118,7 @@ class OnIdentifyInterface : public LDAPInterface
 		User *u = finduser(it->second);
 		this->requests.erase(it);
 
-		if (!u || !u->Account())
+		if (!u || !u->Account() || r.empty())
 			return;
 
 		try
@@ -146,25 +146,44 @@ class OnIdentifyInterface : public LDAPInterface
 	}
 };
 
+class OnRegisterInterface : public LDAPInterface
+{
+ public:
+	OnRegisterInterface(Module *m) : LDAPInterface(m) { }
+
+	void OnResult(const LDAPResult &r)
+	{
+		Log() << "m_ldap_authentication: Successfully added newly created account to LDAP";
+	}
+
+	void OnError(const LDAPResult &r)
+	{
+		Log() << "m_ldap_authentication: Error adding newly created account to LDAP: " << r.getError();
+	}
+};
+
 class NSIdentifyLDAP : public Module
 {
 	service_reference<LDAPProvider> ldap;
 	IdentifyInterface iinterface;
 	OnIdentifyInterface oninterface;
+	OnRegisterInterface orinterface;
 
 	Anope::string binddn;
+	Anope::string object_class;
 	Anope::string username_attribute;
+	Anope::string password_attribute;
 	bool disable_register;
 	Anope::string disable_reason;
  public:
 	NSIdentifyLDAP(const Anope::string &modname, const Anope::string &creator) :
-		Module(modname, creator), ldap("ldap/main"), iinterface(this), oninterface(this)
+		Module(modname, creator), ldap("ldap/main"), iinterface(this), oninterface(this), orinterface(this)
 	{
 		this->SetAuthor("Anope");
 		this->SetType(SUPPORTED);
 
-		Implementation i[] = { I_OnReload, I_OnPreCommand, I_OnCheckAuthentication, I_OnNickIdentify };
-		ModuleManager::Attach(i, this, 4);
+		Implementation i[] = { I_OnReload, I_OnPreCommand, I_OnCheckAuthentication, I_OnNickIdentify, I_OnNickRegister };
+		ModuleManager::Attach(i, this, 5);
 		ModuleManager::SetPriority(this, PRIORITY_FIRST);
 
 		OnReload();
@@ -175,7 +194,9 @@ class NSIdentifyLDAP : public Module
 		ConfigReader config;
 
 		this->binddn = config.ReadValue("m_ldap_authentication", "binddn", "", 0);
+		this->object_class = config.ReadValue("m_ldap_authentication", "object_class", "", 0);
 		this->username_attribute = config.ReadValue("m_ldap_authentication", "username_attribute", "", 0);
+		this->password_attribute = config.ReadValue("m_ldap_authentication", "password_attribute", "", 0);
 		email_attribute = config.ReadValue("m_ldap_authentication", "email_attribute", "", 0);
 		this->disable_register = config.ReadFlag("m_ldap_authentication", "disable_ns_register", "false", 0);
 		this->disable_reason = config.ReadValue("m_ldap_authentication", "disable_reason", "", 0);
@@ -183,7 +204,7 @@ class NSIdentifyLDAP : public Module
 
 	EventReturn OnPreCommand(CommandSource &source, Command *command, const std::vector<Anope::string> &params)
 	{
-		if (this->disable_register && nickserv && command->service == nickserv->Bot() && command->name == "REGISTER")
+		if (this->disable_register && !this->disable_reason.empty() && nickserv && command->service == nickserv->Bot() && command->name == "REGISTER")
 		{
 			source.Reply(_(this->disable_reason.c_str()));
 			return EVENT_STOP;
@@ -204,7 +225,7 @@ class NSIdentifyLDAP : public Module
 		else if (u->GetExt("m_ldap_authentication_error"))
 		{
 			u->Shrink("m_ldap_authentication_error");
-			return EVENT_CONTINUE;;
+			return EVENT_CONTINUE;
 		}
 
 		IdentifyInfo *ii = new IdentifyInfo(u, c, params, account, password);
@@ -239,6 +260,43 @@ class NSIdentifyLDAP : public Module
 			Log() << "m_ldap_authentication: " << ex.GetReason();
 		}
 	}
+
+	void OnNickRegister(NickAlias *na)
+	{
+		if (this->disable_register || !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(this->object_class);
+
+			attributes[1].name = this->username_attribute;
+			attributes[1].values.push_back(na->nick);
+
+			if (!na->nc->email.empty())
+			{
+				attributes[2].name = email_attribute;
+				attributes[2].values.push_back(na->nc->email);
+			}
+
+			attributes[3].name = this->password_attribute;
+			attributes[3].values.push_back(na->nc->pass);
+
+			Anope::string new_dn = this->username_attribute + "=" + na->nick + "," + this->binddn;
+			this->ldap->Add(&this->orinterface, new_dn, attributes);
+		}
+		catch (const LDAPException &ex)
+		{
+			Log() << "m_ldap_authentication: " << ex.GetReason();
+		}
+	}
 };
 
 MODULE_INIT(NSIdentifyLDAP)
diff --git a/modules/extra/ns_set_misc.cpp b/modules/extra/ns_set_misc.cpp
index 385fb9648..1bb1a95a1 100644
--- a/modules/extra/ns_set_misc.cpp
+++ b/modules/extra/ns_set_misc.cpp
@@ -78,7 +78,7 @@ class NSSetMisc : public Module
 
 	void RemoveAll()
 	{
-		if (Commands.empty())
+		if (!nickserv || Commands.empty())
 			return;
 
 		Command *set = FindCommand(nickserv->Bot(), "SET");
diff --git a/src/main.cpp b/src/main.cpp
index 7ec3cfe0e..380240dbb 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -133,10 +133,12 @@ void do_restart_services()
 	}
 	Log() << "Restarting";
 
-	FOREACH_MOD(I_OnPreRestart, OnPreRestart());
-
 	if (quitmsg.empty())
 		quitmsg = "Restarting";
+
+	FOREACH_MOD(I_OnRestart, OnRestart());
+	ModuleManager::UnloadAll();
+
 	/* Send a quit for all of our bots */
 	for (Anope::insensitive_map<BotInfo *>::const_iterator it = BotListByNick.begin(), it_end = BotListByNick.end(); it != it_end; ++it)
 	{
@@ -150,9 +152,6 @@ void do_restart_services()
 			UserListByUID.erase(bi->GetUID());
 	}
 
-	FOREACH_MOD(I_OnRestart, OnRestart());
-
-	ModuleManager::UnloadAll();
 	ircdproto->SendSquit(Config->ServerName, quitmsg);
 	delete UplinkSock;
 	SocketEngine::Shutdown();
@@ -174,11 +173,13 @@ void do_restart_services()
 
 static void services_shutdown()
 {
-	FOREACH_MOD(I_OnPreShutdown, OnPreShutdown());
-
 	if (quitmsg.empty())
 		quitmsg = "Terminating, reason unknown";
 	Log() << quitmsg;
+
+	FOREACH_MOD(I_OnShutdown, OnShutdown());
+	ModuleManager::UnloadAll();
+
 	if (started && UplinkSock)
 	{
 		/* Send a quit for all of our bots */
@@ -203,9 +204,6 @@ static void services_shutdown()
 			delete u;
 		}
 	}
-	FOREACH_MOD(I_OnShutdown, OnShutdown());
-	ModuleManager::UnloadAll();
-	ircdproto->SendSquit(Config->ServerName, quitmsg);
 	delete UplinkSock;
 	SocketEngine::Shutdown();
 
@@ -225,7 +223,6 @@ void sighandler(int signum)
 #else
 		quitmsg = Anope::string("Services terminating via signal ") + stringify(signum);
 #endif
-	bool fatal = false;
 
 	if (started)
 	{
@@ -267,17 +264,13 @@ void sighandler(int signum)
 #endif
 
 				save_databases();
-				services_shutdown();
+				quitting = true;
 			default:
-				fatal = true;
 				break;
 		}
 	}
 
 	FOREACH_MOD(I_OnSignal, OnSignal(signum, quitmsg));
-
-	if (fatal)
-		throw FatalException(quitmsg);
 }
 
 /*************************************************************************/
diff --git a/src/modulemanager.cpp b/src/modulemanager.cpp
index d9d105953..ac6316728 100644
--- a/src/modulemanager.cpp
+++ b/src/modulemanager.cpp
@@ -472,16 +472,17 @@ void ModuleManager::ClearCallBacks(Module *m)
  */
 void ModuleManager::UnloadAll()
 {
+	std::vector<Anope::string> modules[MT_END];
+	for (std::list<Module *>::iterator it = Modules.begin(), it_end = Modules.end(); it != it_end; ++it)
+		modules[(*it)->type].push_back((*it)->name);
+
 	for (size_t i = MT_BEGIN + 1; i != MT_END; ++i)
-	{
-		for (std::list<Module *>::iterator it = Modules.begin(), it_end = Modules.end(); it != it_end; )
+		for (unsigned j = 0; j < modules[i].size(); ++j)
 		{
-			Module *m = *it++;
-
-			if (static_cast<MODType>(i) == m->type)
+			Module *m = FindModule(modules[i][j]);
+			if (m != NULL)
 				UnloadModule(m, NULL);
 		}
-	}
 }
 
 /** Register a service