ThreadEngine: remove excessive mutex use on thread creation

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

9 files changed, 59 insertions, 152 deletions

diff --git a/src/commands/cmd_rehash.cpp b/src/commands/cmd_rehash.cpp
index 1d3d5afbc..4182de830 100644
--- a/src/commands/cmd_rehash.cpp
+++ b/src/commands/cmd_rehash.cpp
@@ -68,7 +68,7 @@ CmdResult CommandRehash::Handle (const std::vector<std::string>& parameters, Use
 		ServerInstance->Config->RehashParameter = parameters.size() ? parameters[0] : "";
 
 		ServerInstance->ConfigThread = new ConfigReaderThread(ServerInstance, false, ServerInstance->Config->RehashUserUID);
-		ServerInstance->Threads->Create(ServerInstance->ConfigThread);
+		ServerInstance->Threads->Start(ServerInstance->ConfigThread);
 	}
 	else
 	{
diff --git a/src/configreader.cpp b/src/configreader.cpp
index eb5edbb09..6444ee211 100644
--- a/src/configreader.cpp
+++ b/src/configreader.cpp
@@ -1006,7 +1006,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 			if (!Values[Index].validation_function(this, Values[Index].tag, Values[Index].value, vi))
 				throw CoreException("One or more values in your configuration file failed to validate. Please see your ircd.log for more information.");
 
-			ServerInstance->Threads->Lock();
+			// XXX: ServerInstance->Threads->Lock();
 			switch (dt)
 			{
 				case DT_NOSPACES:
@@ -1035,7 +1035,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 					ValueContainerChar* vcc = (ValueContainerChar*)Values[Index].val;
 					if (*(vi.GetString()) && !ServerInstance->IsChannel(vi.GetString(), MAXBUF))
 					{
-						ServerInstance->Threads->Unlock();
+						// XXX: ServerInstance->Threads->Unlock();
 						throw CoreException("The value of <"+std::string(Values[Index].tag)+":"+Values[Index].value+"> is not a valid channel name");
 					}
 					vcc->Set(vi.GetString(), strlen(vi.GetString()) + 1);
@@ -1068,7 +1068,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 			}
 			/* We're done with this now */
 			delete Values[Index].val;
-			ServerInstance->Threads->Unlock();
+			// XXX: ServerInstance->Threads->Unlock();
 		}
 
 		/* Read the multiple-tag items (class tags, connect tags, etc)
@@ -1077,9 +1077,9 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 		 */
 		for (int Index = 0; MultiValues[Index].tag; ++Index)
 		{
-			ServerInstance->Threads->Lock();
+			// XXX: ServerInstance->Threads->Lock();
 			MultiValues[Index].init_function(this, MultiValues[Index].tag);
-			ServerInstance->Threads->Unlock();
+			// XXX: ServerInstance->Threads->Unlock();
 
 			int number_of_tags = ConfValueEnum(newconfig, MultiValues[Index].tag);
 
@@ -1094,7 +1094,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 					dt &= ~DT_ALLOW_NEWLINE;
 					dt &= ~DT_ALLOW_WILD;
 
-					ServerInstance->Threads->Lock();
+					// XXX: ServerInstance->Threads->Lock();
 					/* We catch and rethrow any exception here just so we can free our mutex
 					 */
 					try
@@ -1173,10 +1173,10 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 					}
 					catch (CoreException &e)
 					{
-						ServerInstance->Threads->Unlock();
+						// XXX: ServerInstance->Threads->Unlock();
 						throw e;
 					}
-					ServerInstance->Threads->Unlock();
+					// XXX: ServerInstance->Threads->Unlock();
 				}
 				MultiValues[Index].validation_function(this, MultiValues[Index].tag, (char**)MultiValues[Index].items, vl, MultiValues[Index].datatype);
 			}
@@ -1196,7 +1196,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 		return;
 	}
 
-	ServerInstance->Threads->Lock();
+	// XXX: ServerInstance->Threads->Lock();
 	for (int i = 0; i < ConfValueEnum(newconfig, "type"); ++i)
 	{
 		char item[MAXBUF], classn[MAXBUF], classes[MAXBUF];
@@ -1239,7 +1239,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 	/* If we succeeded, set the ircd config to the new one */
 	this->config_data = newconfig;
 
-	ServerInstance->Threads->Unlock();
+	// XXX: ServerInstance->Threads->Unlock();
 
 	// write once here, to try it out and make sure its ok
 	ServerInstance->WritePID(this->PID);
@@ -1254,7 +1254,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 
 		if (pl.size() && !useruid.empty())
 		{
-			ServerInstance->Threads->Lock();
+			// XXX: ServerInstance->Threads->Lock();
 			User* user = ServerInstance->FindNick(useruid);
 			if (user)
 			{
@@ -1266,10 +1266,10 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 					user->WriteServ("NOTICE %s :*** %d.   Address: %s        Reason: %s", user->nick.c_str(), j, i->first.empty() ? "<all>" : i->first.c_str(), i->second.c_str());
 				}
 			}
-			ServerInstance->Threads->Unlock();
+			// XXX: ServerInstance->Threads->Unlock();
 		}
 
-		ServerInstance->Threads->Lock();
+		// XXX: ServerInstance->Threads->Lock();
 		if (!removed_modules.empty())
 		{
 			for (std::vector<std::string>::iterator removing = removed_modules.begin(); removing != removed_modules.end(); removing++)
@@ -1334,7 +1334,7 @@ void ServerConfig::Read(bool bail, const std::string &useruid)
 				}
 			}
 		}
-		ServerInstance->Threads->Unlock();
+		// XXX: ServerInstance->Threads->Unlock();
 
 	}
 
@@ -2372,7 +2372,4 @@ bool DoneELine(ServerConfig* conf, const char* tag)
 void ConfigReaderThread::Run()
 {
 	ServerInstance->Config->Read(do_bail, TheUserUID);
-	ServerInstance->Threads->Lock();
-	this->SetExitFlag();
-	ServerInstance->Threads->Unlock();
 }
diff --git a/src/modules/extra/m_mssql.cpp b/src/modules/extra/m_mssql.cpp
index 441876401..7515570a9 100644
--- a/src/modules/extra/m_mssql.cpp
+++ b/src/modules/extra/m_mssql.cpp
@@ -779,7 +779,7 @@ class ModuleMsSQL : public Module
 		ReadConf();
 
 		queryDispatcher = new QueryThread(ServerInstance, this);
-		ServerInstance->Threads->Create(queryDispatcher);
+		ServerInstance->Threads->Start(queryDispatcher);
 
 		ServerInstance->Modules->PublishInterface("SQL", this);
 		Implementation eventlist[] = { I_OnRequest, I_OnRehash };
diff --git a/src/modules/extra/m_mysql.cpp b/src/modules/extra/m_mysql.cpp
index f105bdab9..fe4e08965 100644
--- a/src/modules/extra/m_mysql.cpp
+++ b/src/modules/extra/m_mysql.cpp
@@ -826,7 +826,7 @@ ModuleSQL::ModuleSQL(InspIRCd* Me) : Module(Me), rehashing(false)
 	}
 
 	Dispatcher = new DispatcherThread(ServerInstance, this);
-	ServerInstance->Threads->Create(Dispatcher);
+	ServerInstance->Threads->Start(Dispatcher);
 
 	ResultsMutex = ServerInstance->Mutexes->CreateMutex();
 	QueueMutex = ServerInstance->Mutexes->CreateMutex();
diff --git a/src/server.cpp b/src/server.cpp
index 56970054d..3e607e032 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -58,7 +58,7 @@ void RehashHandler::Call(const std::string &reason)
 		Server->Config->RehashParameter = "";
 
 		Server->ConfigThread = new ConfigReaderThread(Server, false, "");
-		Server->Threads->Create(Server->ConfigThread);
+		Server->Threads->Start(Server->ConfigThread);
 	}
 	Server->RehashFinishMutex->Unlock();
 }
diff --git a/src/testsuite.cpp b/src/testsuite.cpp
index b7a9835e0..5de5601cb 100644
--- a/src/testsuite.cpp
+++ b/src/testsuite.cpp
@@ -205,7 +205,7 @@ bool TestSuite::DoThreadTests()
 	{
 		try
 		{
-			te->Create(tst);
+			te->Start(tst);
 		}
 		catch (CoreException &ce)
 		{
diff --git a/src/threadengine.cpp b/src/threadengine.cpp
index 124102f0c..d15ae2f70 100644
--- a/src/threadengine.cpp
+++ b/src/threadengine.cpp
@@ -28,6 +28,6 @@ ThreadEngine::~ThreadEngine()
 {
 }
 
-Mutex::Mutex(InspIRCd* Instance) : ServerInstance(Instance)
+Mutex::Mutex()
 {
 }
diff --git a/src/threadengines/threadengine_pthread.cpp b/src/threadengines/threadengine_pthread.cpp
index 21f55108c..9fc9cc4b4 100644
--- a/src/threadengines/threadengine_pthread.cpp
+++ b/src/threadengines/threadengine_pthread.cpp
@@ -16,101 +16,45 @@
 #include <pthread.h>
 #include <signal.h>
 
-pthread_mutex_t MyMutex = PTHREAD_MUTEX_INITIALIZER;
-
 PThreadEngine::PThreadEngine(InspIRCd* Instance) : ThreadEngine(Instance)
 {
 }
 
-void PThreadEngine::Create(Thread* thread_to_init)
+static void* entry_point(void* parameter)
 {
-	pthread_attr_t attribs;
-	pthread_attr_init(&attribs);
-	pthread_attr_setdetachstate(&attribs, PTHREAD_CREATE_JOINABLE);
-	pthread_t* MyPThread = new pthread_t;
-
-	/* Create a thread in a mutex. This prevents whacking the member value NewThread,
-	 * and also prevents recursive creation of threads by mistake (instead, the thread
-	 * will just deadlock itself)
-	 */
-	Mutex(true);
-
-	if (pthread_create(MyPThread, &attribs, PThreadEngine::Entry, (void*)this) != 0)
-	{
-		delete MyPThread;
-		Mutex(false);
-		throw CoreException("Unable to create new PThreadEngine: " + std::string(strerror(errno)));
-	}
-
-	pthread_attr_destroy(&attribs);
-
-	NewThread = thread_to_init;
-	NewThread->Creator = this;
-	NewThread->Extend("pthread", MyPThread);
-
-	/* Always unset a mutex if you set it */
-	Mutex(false);
-
-	/* Wait for the PThreadEngine::Run method to take a copy of the
-	 * pointer and clear this member value
-	 */
-	while (NewThread)
-		usleep(1000);
-}
+	/* Recommended by nenolod, signal safety on a per-thread basis */
+	sigset_t set;
+	sigemptyset(&set);
+	sigaddset(&set, SIGPIPE);
+	pthread_sigmask(SIG_BLOCK, &set, NULL);
 
-PThreadEngine::~PThreadEngine()
-{
+	Thread* pt = reinterpret_cast<Thread*>(parameter);
+	pt->Run();
+	return parameter;
 }
 
-void PThreadEngine::Run()
-{
-	/* Take a copy of the member value, then clear it. Do this
-	 * in a mutex so that we can be sure nothing else is looking
-	 * at it.
-	 */
-	Mutex(true);
-	Thread* nt = NewThread;
-	NewThread = NULL;
-	Mutex(false);
-	/* Now we have our own safe copy, call the object on it */
-	nt->Run();
-}
 
-bool PThreadEngine::Mutex(bool enable)
+void PThreadEngine::Start(Thread* thread)
 {
-	if (enable)
-		pthread_mutex_lock(&MyMutex);
-	else
-		pthread_mutex_unlock(&MyMutex);
+	PThreadData* data = new PThreadData;
+	thread->state = data;
 
-	return false;
+	if (pthread_create(&data->pthread_id, NULL, entry_point, thread) != 0)
+	{
+		thread->state = NULL;
+		delete data;
+		throw CoreException("Unable to create new PThreadEngine: " + std::string(strerror(errno)));
+	}
 }
 
-void* PThreadEngine::Entry(void* parameter)
+PThreadEngine::~PThreadEngine()
 {
-	/* Recommended by nenolod, signal safety on a per-thread basis */
-	sigset_t set;
-	sigemptyset(&set);
-	sigaddset(&set, SIGPIPE);
-	if(pthread_sigmask(SIG_BLOCK, &set, NULL))
-		signal(SIGPIPE, SIG_IGN);
-
-	ThreadEngine * pt = (ThreadEngine*)parameter;
-	pt->Run();
-	return NULL;
 }
 
-void PThreadEngine::FreeThread(Thread* thread)
+void PThreadData::FreeThread(Thread* thread)
 {
-	pthread_t* pthread = NULL;
-	if (thread->GetExt("pthread", pthread))
-	{
-		thread->SetExitFlag();
-		int rc;
-		void* status;
-		rc = pthread_join(*pthread, &status);
-		delete pthread;
-	}
+	thread->SetExitFlag(true);
+	pthread_join(pthread_id, NULL);
 }
 
 MutexFactory::MutexFactory(InspIRCd* Instance) : ServerInstance(Instance)
@@ -119,10 +63,10 @@ MutexFactory::MutexFactory(InspIRCd* Instance) : ServerInstance(Instance)
 
 Mutex* MutexFactory::CreateMutex()
 {
-	return new PosixMutex(this->ServerInstance);
+	return new PosixMutex();
 }
 
-PosixMutex::PosixMutex(InspIRCd* Instance) : Mutex(Instance)
+PosixMutex::PosixMutex() : Mutex()
 {
 	pthread_mutex_init(&putex, NULL);
 }
diff --git a/src/threadengines/threadengine_win32.cpp b/src/threadengines/threadengine_win32.cpp
index 1a1755f15..b32dd2441 100644
--- a/src/threadengines/threadengine_win32.cpp
+++ b/src/threadengines/threadengine_win32.cpp
@@ -14,75 +14,41 @@
 #include "inspircd.h"
 #include "threadengines/threadengine_win32.h"
 
-CRITICAL_SECTION MyMutex;
-
 Win32ThreadEngine::Win32ThreadEngine(InspIRCd* Instance) : ThreadEngine(Instance)
 {
-	InitializeCriticalSection(&MyMutex);
 }
 
-void Win32ThreadEngine::Create(Thread* thread_to_init)
+void Win32ThreadEngine::Create(Thread* thread)
 {
-	Mutex(true);
-	HANDLE* MyThread = new HANDLE;
+	Win32ThreadData* data = new Win32ThreadData;
+	thread->state = data;
+
 	DWORD ThreadId = 0;
+	data->handle = CreateThread(NULL,0,Win32ThreadEngine::Entry,thread,0,&ThreadId);
 
-	if (NULL == (*MyThread = CreateThread(NULL,0,Win32ThreadEngine::Entry,this,0,&ThreadId)))
+	if (data->handle == NULL)
 	{
-		delete MyThread;
-		Mutex(false);
+		thread->state = NULL;
+		delete data;
 		throw CoreException(std::string("Unable to create new Win32ThreadEngine: ") + dlerror());
 	}
-
-	NewThread = thread_to_init;
-	NewThread->Creator = this;
-	NewThread->Extend("winthread", MyThread);
-	Mutex(false);
-
-	while (NewThread)
-		SleepEx(100, false);
 }
 
 Win32ThreadEngine::~Win32ThreadEngine()
 {
-	DeleteCriticalSection(&MyMutex);
-}
-
-void Win32ThreadEngine::Run()
-{
-	Mutex(true);
-	Thread* nt = NewThread;
-	NewThread = NULL;
-	Mutex(false);
-	nt->Run();
-}
-
-bool Win32ThreadEngine::Mutex(bool enable)
-{
-	if (enable)
-		EnterCriticalSection(&MyMutex);
-	else
-		LeaveCriticalSection(&MyMutex);
-
-	return false;
 }
 
 DWORD WINAPI Win32ThreadEngine::Entry(void* parameter)
 {
-	ThreadEngine * pt = (ThreadEngine*)parameter;
+	Thread* pt = reinterpret_cast<Thread*>(parameter);
 	pt->Run();
 	return 0;
 }
 
-void Win32ThreadEngine::FreeThread(Thread* thread)
+void Win32ThreadData::FreeThread(Thread* thread)
 {
-	HANDLE* winthread = NULL;
-	if (thread->GetExt("winthread", winthread))
-	{
-		thread->SetExitFlag();
-		WaitForSingleObject(*winthread,INFINITE);
-		delete winthread;
-	}
+	thread->SetExitFlag();
+	WaitForSingleObject(handle,INFINITE);
 }
 
 
@@ -92,10 +58,10 @@ MutexFactory::MutexFactory(InspIRCd* Instance) : ServerInstance(Instance)
 
 Mutex* MutexFactory::CreateMutex()
 {
-	return new Win32Mutex(this->ServerInstance);
+	return new Win32Mutex();
 }
 
-Win32Mutex::Win32Mutex(InspIRCd* Instance) : Mutex(Instance)
+Win32Mutex::Win32Mutex() : Mutex()
 {
 	InitializeCriticalSection(&wutex);
 }
@@ -111,4 +77,4 @@ void Win32Mutex::Enable(bool enable)
 		EnterCriticalSection(&wutex);
 	else
 		LeaveCriticalSection(&wutex);
-}
\ No newline at end of file
+}