Add Module::init() for correct exception handling during hook registration

git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@12278 e03df62e-2008-0410-955e-edbf42e46eb7

11 files changed, 73 insertions, 35 deletions

diff --git a/include/inspircd.h b/include/inspircd.h
index ac3d45a8d..3d1abdee4 100644
--- a/include/inspircd.h
+++ b/include/inspircd.h
@@ -598,7 +598,10 @@ class CoreExport InspIRCd
 	 * @param f A Command command handler object to add
 	 * @throw ModuleException Will throw ModuleExcption if the command already exists
 	 */
-	void AddCommand(Command *f);
+	inline void AddCommand(Command *f)
+	{
+		Modules->AddService(*f);
+	}
 
 	/** Send a modechange.
 	 * The parameters provided are identical to that sent to the
diff --git a/include/modules.h b/include/modules.h
index 13e3dc28c..582e74275 100644
--- a/include/modules.h
+++ b/include/modules.h
@@ -347,12 +347,16 @@ class CoreExport Module : public classbase, public usecountbase
 	DLLManager* ModuleDLLManager;
 
 	/** Default constructor.
-	 * Creates a module class.
-	 * @param Me An instance of the InspIRCd class which will be saved into ServerInstance for your use
-	 * \exception ModuleException Throwing this class, or any class derived from ModuleException, causes loading of the module to abort.
+	 * Creates a module class. Don't do any type of hook registration or checks
+	 * for other modules here; do that in init().
 	 */
 	Module();
 
+	/** Module setup
+	 * \exception ModuleException Throwing this class, or any class derived from ModuleException, causes loading of the module to abort.
+	 */
+	virtual void init() {}
+
 	/** Clean up prior to destruction
 	 * If you override, you must call this AFTER your module's cleanup
 	 */
diff --git a/src/modmanager_dynamic.cpp b/src/modmanager_dynamic.cpp
index 900562260..1fcf3aa56 100644
--- a/src/modmanager_dynamic.cpp
+++ b/src/modmanager_dynamic.cpp
@@ -89,11 +89,12 @@ bool ModuleManager::Load(const char* filename)
 			newmod->ModuleSourceFile = filename_str;
 			newmod->ModuleDLLManager = newhandle;
 			Version v = newmod->GetVersion();
+			Modules[filename_str] = newmod;
+
+			newmod->init();
 
 			ServerInstance->Logs->Log("MODULE", DEFAULT,"New module introduced: %s (Module version %s)%s",
 				filename, newhandle->GetVersion().c_str(), (!(v.Flags & VF_VENDOR) ? " [3rd Party]" : " [Vendor]"));
-
-			Modules[filename_str] = newmod;
 		}
 		else
 		{
@@ -106,6 +107,8 @@ bool ModuleManager::Load(const char* filename)
 	catch (CoreException& modexcept)
 	{
 		// failure in module constructor
+		if (newmod)
+			DoSafeUnload(newmod);
 		delete newmod;
 		delete newhandle;
 		LastModuleError = "Unable to load " + filename_str + ": " + modexcept.GetReason();
diff --git a/src/modmanager_static.cpp b/src/modmanager_static.cpp
index 322703679..e267a2c90 100644
--- a/src/modmanager_static.cpp
+++ b/src/modmanager_static.cpp
@@ -64,15 +64,20 @@ bool ModuleManager::Load(const char* name)
 	{
 		if ((**i).name == name)
 		{
+			Module* c = NULL;
 			try
 			{
-				Module* c = (*(**i).init)();
+				c = (*(**i).init)();
 				Modules[name] = c;
+				c->init();
 				FOREACH_MOD(I_OnLoadModule,OnLoadModule(c));
 				return true;
 			}
 			catch (CoreException& modexcept)
 			{
+				if (c)
+					DoSafeUnload(c);
+				delete c;
 				ServerInstance->Logs->Log("MODULE", DEFAULT, "Unable to load " + (**i).name + ": " + modexcept.GetReason());
 			}
 		}
@@ -117,15 +122,20 @@ void ModuleManager::LoadAll()
 	ModCount = 0;
 	for(std::vector<AllModuleList*>::iterator i = modlist->begin(); i != modlist->end(); ++i)
 	{
+		Module* c = NULL;
 		try
 		{
-			Module* c = (*(**i).init)();
+			c = (*(**i).init)();
 			c->ModuleSourceFile = (**i).name;
 			Modules[(**i).name] = c;
+			c->init();
 			FOREACH_MOD(I_OnLoadModule,OnLoadModule(c));
 		}
 		catch (CoreException& modexcept)
 		{
+			if (c)
+				DoSafeUnload(c);
+			delete c;
 			ServerInstance->Logs->Log("MODULE", DEFAULT, "Unable to load " + (**i).name + ": " + modexcept.GetReason());
 		}
 	}
diff --git a/src/modules.cpp b/src/modules.cpp
index b25469c30..22628ff0f 100644
--- a/src/modules.cpp
+++ b/src/modules.cpp
@@ -174,6 +174,9 @@ ModuleManager::~ModuleManager()
 
 bool ModuleManager::Attach(Implementation i, Module* mod)
 {
+	if (Modules.find(mod->ModuleSourceFile) == Modules.end())
+		ServerInstance->Logs->Log("MODULE", ERROR, "Module %s is attaching to hook %d in constructor; this does not handle exceptions correctly!", mod->ModuleSourceFile.c_str(), i);
+
 	if (std::find(EventHandlers[i].begin(), EventHandlers[i].end(), mod) != EventHandlers[i].end())
 		return false;
 
@@ -417,16 +420,12 @@ bool InspIRCd::IsValidModuleCommand(const std::string &commandname, int pcnt, Us
 	return this->Parser->IsValidCommand(commandname, pcnt, user);
 }
 
-void InspIRCd::AddCommand(Command *f)
-{
-	if (!this->Parser->AddCommand(f))
-	{
-		throw ModuleException("Command "+std::string(f->name)+" already exists.");
-	}
-}
-
 void ModuleManager::AddService(ServiceProvider& item)
 {
+	Module* owner = item.creator;
+	if (Modules.find(owner->ModuleSourceFile) == Modules.end())
+		ServerInstance->Logs->Log("MODULE", ERROR, "Module %s is registering item %s in constructor; this does not handle exceptions correctly!", owner->ModuleSourceFile.c_str(), item.name.c_str());
+
 	switch (item.service)
 	{
 		case SERVICE_COMMAND:
diff --git a/src/modules/extra/m_ssl_gnutls.cpp b/src/modules/extra/m_ssl_gnutls.cpp
index 26fa58898..1ebec075b 100644
--- a/src/modules/extra/m_ssl_gnutls.cpp
+++ b/src/modules/extra/m_ssl_gnutls.cpp
@@ -149,6 +149,10 @@ class ModuleSSLGnuTLS : public Module
 		gnutls_x509_privkey_init(&x509_key);
 
 		cred_alloc = false;
+	}
+
+	void init()
+	{
 		// Needs the flag as it ignores a plain /rehash
 		OnModuleRehash(NULL,"ssl");
 
@@ -282,8 +286,11 @@ class ModuleSSLGnuTLS : public Module
 	{
 		gnutls_x509_crt_deinit(x509_cert);
 		gnutls_x509_privkey_deinit(x509_key);
-		gnutls_dh_params_deinit(dh_params);
-		gnutls_certificate_free_credentials(x509_cred);
+		if (cred_alloc)
+		{
+			gnutls_dh_params_deinit(dh_params);
+			gnutls_certificate_free_credentials(x509_cred);
+		}
 		gnutls_global_deinit();
 		delete[] sessions;
 	}
diff --git a/src/modules/extra/m_ssl_openssl.cpp b/src/modules/extra/m_ssl_openssl.cpp
index 1d4ebd7fd..c46b93117 100644
--- a/src/modules/extra/m_ssl_openssl.cpp
+++ b/src/modules/extra/m_ssl_openssl.cpp
@@ -101,7 +101,6 @@ class ModuleSSLOpenSSL : public Module
 
 	ModuleSSLOpenSSL() : iohook(this, "ssl/openssl", SERVICE_IOHOOK)
 	{
-
 		sessions = new issl_session[ServerInstance->SE->GetMaxFds()];
 
 		// Not rehashable...because I cba to reduce all the sizes of existing buffers.
@@ -122,7 +121,10 @@ class ModuleSSLOpenSSL : public Module
 
 		SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, OnVerify);
 		SSL_CTX_set_verify(clictx, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, OnVerify);
+	}
 
+	void init()
+	{
 		// Needs the flag as it ignores a plain /rehash
 		OnModuleRehash(NULL,"ssl");
 		Implementation eventlist[] = { I_On005Numeric, I_OnRehash, I_OnModuleRehash, I_OnHookIO, I_OnUserConnect };
diff --git a/src/modules/m_censor.cpp b/src/modules/m_censor.cpp
index 03d591fa8..e356ee9a7 100644
--- a/src/modules/m_censor.cpp
+++ b/src/modules/m_censor.cpp
@@ -44,14 +44,15 @@ class ModuleCensor : public Module
 	CensorChannel cc;
 
  public:
-	ModuleCensor()
-		: cu(this), cc(this)
+	ModuleCensor() : cu(this), cc(this) { }
+
+	void init()
 	{
 		/* Read the configuration file on startup.
 		 */
 		OnRehash(NULL);
-		if (!ServerInstance->Modes->AddMode(&cu) || !ServerInstance->Modes->AddMode(&cc))
-			throw ModuleException("Could not add new modes!");
+		ServerInstance->Modules->AddService(cu);
+		ServerInstance->Modules->AddService(cc);
 		Implementation eventlist[] = { I_OnRehash, I_OnUserPreMessage, I_OnUserPreNotice };
 		ServerInstance->Modules->Attach(eventlist, this, 3);
 	}
diff --git a/src/modules/m_chanprotect.cpp b/src/modules/m_chanprotect.cpp
index 61ffa3ad2..62e41416b 100644
--- a/src/modules/m_chanprotect.cpp
+++ b/src/modules/m_chanprotect.cpp
@@ -231,14 +231,16 @@ class ModuleChanProtect : public Module
  public:
 	ModuleChanProtect() : cp(this), cf(this)
 	{
+	}
+
+	void init()
+	{
 		/* Load config stuff */
 		LoadSettings();
 		settings.booting = false;
 
-		if (!ServerInstance->Modes->AddMode(&cp) || !ServerInstance->Modes->AddMode(&cf))
-		{
-			throw ModuleException("Could not add new modes!");
-		}
+		ServerInstance->Modules->AddService(cf);
+		ServerInstance->Modules->AddService(cp);
 
 		Implementation eventlist[] = { I_OnUserPreJoin };
 		ServerInstance->Modules->Attach(eventlist, this, 1);
diff --git a/src/modules/m_services_account.cpp b/src/modules/m_services_account.cpp
index 7073d58b0..069bcc837 100644
--- a/src/modules/m_services_account.cpp
+++ b/src/modules/m_services_account.cpp
@@ -109,13 +109,16 @@ class ModuleServicesAccount : public Module
 	ModuleServicesAccount() : m1(this), m2(this), m3(this), m4(this), m5(this),
 		accountname("accountname", this)
 	{
+	}
 
-		if (!ServerInstance->Modes->AddMode(&m1) || !ServerInstance->Modes->AddMode(&m2) ||
-			!ServerInstance->Modes->AddMode(&m3) || !ServerInstance->Modes->AddMode(&m4) ||
-			!ServerInstance->Modes->AddMode(&m5))
-			throw ModuleException("Some other module has claimed our modes!");
-
-		ServerInstance->Extensions.Register(&accountname);
+	void init()
+	{
+		ServerInstance->Modules->AddService(m1);
+		ServerInstance->Modules->AddService(m2);
+		ServerInstance->Modules->AddService(m3);
+		ServerInstance->Modules->AddService(m4);
+		ServerInstance->Modules->AddService(m5);
+		ServerInstance->Modules->AddService(accountname);
 		Implementation eventlist[] = { I_OnWhois, I_OnUserPreMessage, I_OnUserPreNotice, I_OnUserPreJoin, I_OnCheckBan,
 			I_OnDecodeMetaData, I_On005Numeric, I_OnUserPostNick };
 
diff --git a/src/modules/m_stripcolor.cpp b/src/modules/m_stripcolor.cpp
index adfc8d565..8034f56ee 100644
--- a/src/modules/m_stripcolor.cpp
+++ b/src/modules/m_stripcolor.cpp
@@ -41,8 +41,12 @@ class ModuleStripColor : public Module
  public:
 	ModuleStripColor() : csc(this), usc(this)
 	{
-		if (!ServerInstance->Modes->AddMode(&usc) || !ServerInstance->Modes->AddMode(&csc))
-			throw ModuleException("Could not add new modes!");
+	}
+
+	void init()
+	{
+		ServerInstance->Modules->AddService(usc);
+		ServerInstance->Modules->AddService(csc);
 		Implementation eventlist[] = { I_OnUserPreMessage, I_OnUserPreNotice, I_On005Numeric };
 		ServerInstance->Modules->Attach(eventlist, this, 3);
 	}