Fixed *nix threading issue;

Thread objects now use variable names consistent with MCS convention;
Fixed a few *nix threading cornercases

git-svn-id: http://mc-server.googlecode.com/svn/trunk@392 0a769ca7-a7f5-676a-18bf-c427514a06d6

source/ChunkSender.cpp

@@ -30,7 +30,7 @@ cChunkSender::cChunkSender(void) :
 
 cChunkSender::~cChunkSender()
 {
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
 	m_evtQueue.Set();
 }
 
@@ -100,7 +100,7 @@ void cChunkSender::RemoveClient(cClientHandle * a_Client)
 
 void cChunkSender::Execute(void)
 {
-	while (!mShouldTerminate)
+	while (!m_ShouldTerminate)
 	{
 		cCSLock Lock(m_CS);
 		while (m_ChunksReady.empty() && m_SendChunks.empty())
@@ -108,7 +108,7 @@ void cChunkSender::Execute(void)
 			cCSUnlock Unlock(Lock);
 			m_evtRemoved.Set();  // Notify that the removed clients are safe to be deleted
 			m_evtQueue.Wait();
-			if (mShouldTerminate)
+			if (m_ShouldTerminate)
 			{
 				return;
 			}

source/LightingThread.cpp

@@ -40,7 +40,7 @@ void cLightingThread::Stop(void)
 			delete *itr;
 		}  // for itr - m_Queue[]
 	}
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
 	m_Event.Set();
 	
 	Wait();

source/WorldStorage.cpp

@@ -153,10 +153,11 @@ void cWorldStorage::WaitForFinish(void)
 	}
 	
 	// Wait for the thread to finish:
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
 	m_Event.Set();
 	m_evtRemoved.Set();  // Wake up anybody waiting in the WaitForQueuesEmpty() method
 	super::Wait();
+	LOG("World storage thread finished");
 }
 
 
@@ -166,7 +167,7 @@ void cWorldStorage::WaitForFinish(void)
 void cWorldStorage::WaitForQueuesEmpty(void)
 {
 	cCSLock Lock(m_CSQueues);
-	while (!mShouldTerminate && (!m_LoadQueue.empty() || !m_SaveQueue.empty()))
+	while (!m_ShouldTerminate && (!m_LoadQueue.empty() || !m_SaveQueue.empty()))
 	{
 		cCSUnlock Unlock(Lock);
 		m_evtRemoved.Wait();
@@ -305,7 +306,7 @@ void cWorldStorage::InitSchemas(void)
 
 void cWorldStorage::Execute(void)
 {
-	while (!mShouldTerminate)
+	while (!m_ShouldTerminate)
 	{
 		m_Event.Wait();
 		
@@ -314,7 +315,7 @@ void cWorldStorage::Execute(void)
 		do
 		{
 			HasMore = false;
-			if (mShouldTerminate)
+			if (m_ShouldTerminate)
 			{
 				return;
 			}

source/cAuthenticator.cpp

@@ -24,9 +24,9 @@
 
 cAuthenticator::cAuthenticator(void) :
 	super("cAuthenticator"),
-	mServer(DEFAULT_AUTH_SERVER),
+	m_Server(DEFAULT_AUTH_SERVER),
-	mAddress(DEFAULT_AUTH_ADDRESS),
+	m_Address(DEFAULT_AUTH_ADDRESS),
-	mShouldAuthenticate(true)
+	m_ShouldAuthenticate(true)
 {
 	ReadINI();
 }
@@ -53,27 +53,27 @@ void cAuthenticator::ReadINI(void)
 		return;
 	}
 	
-	mServer  = IniFile.GetValue("Authentication", "Server");
+	m_Server  = IniFile.GetValue("Authentication", "Server");
-	mAddress = IniFile.GetValue("Authentication", "Address");
+	m_Address = IniFile.GetValue("Authentication", "Address");
-	mShouldAuthenticate = IniFile.GetValueB("Authentication", "Authenticate", true);
+	m_ShouldAuthenticate = IniFile.GetValueB("Authentication", "Authenticate", true);
 	bool bSave = false;
 	
-	if (mServer.length() == 0)
+	if (m_Server.length() == 0)
 	{
-		mServer = DEFAULT_AUTH_SERVER;
+		m_Server = DEFAULT_AUTH_SERVER;
-		IniFile.SetValue("Authentication", "Server", mServer);
+		IniFile.SetValue("Authentication", "Server", m_Server);
 		bSave = true;
 	}
-	if (mAddress.length() == 0)
+	if (m_Address.length() == 0)
 	{
-		mAddress = DEFAULT_AUTH_ADDRESS;
+		m_Address = DEFAULT_AUTH_ADDRESS;
-		IniFile.SetValue("Authentication", "Address", mAddress);
+		IniFile.SetValue("Authentication", "Address", m_Address);
 		bSave = true;
 	}
 
 	if (bSave)
 	{
-		IniFile.SetValueB("Authentication", "Authenticate", mShouldAuthenticate);
+		IniFile.SetValueB("Authentication", "Authenticate", m_ShouldAuthenticate);
 		IniFile.WriteFile();
 	}
 }
@@ -85,15 +85,15 @@ void cAuthenticator::ReadINI(void)
 /// Queues a request for authenticating a user. If the auth fails, the user is kicked
 void cAuthenticator::Authenticate(int a_ClientID, const AString & a_UserName, const AString & a_ServerHash)
 {
-	if (!mShouldAuthenticate)
+	if (!m_ShouldAuthenticate)
 	{
 		cRoot::Get()->AuthenticateUser(a_ClientID);
 		return;
 	}
 
-	cCSLock Lock(mCS);
+	cCSLock Lock(m_CS);
-	mQueue.push_back(cUser(a_ClientID, a_UserName, a_ServerHash));
+	m_Queue.push_back(cUser(a_ClientID, a_UserName, a_ServerHash));
-	mQueueNonempty.Set();
+	m_QueueNonempty.Set();
 }
 
 
@@ -102,8 +102,8 @@ void cAuthenticator::Authenticate(int a_ClientID, const AString & a_UserName, co
 
 void cAuthenticator::Stop(void)
 {
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
-	mQueueNonempty.Set();
+	m_QueueNonempty.Set();
 	Wait();
 }
 
@@ -115,27 +115,27 @@ void cAuthenticator::Execute(void)
 {
 	for (;;)
 	{
-		cCSLock Lock(mCS);
+		cCSLock Lock(m_CS);
-		while (!mShouldTerminate && (mQueue.size() == 0))
+		while (!m_ShouldTerminate && (m_Queue.size() == 0))
 		{
 			cCSUnlock Unlock(Lock);
-			mQueueNonempty.Wait();
+			m_QueueNonempty.Wait();
 		}
-		if (mShouldTerminate)
+		if (m_ShouldTerminate)
 		{
 			return;
 		}
-		ASSERT(mQueue.size() > 0);
+		ASSERT(m_Queue.size() > 0);
 		
-		int ClientID = mQueue.front().mClientID;
+		int ClientID = m_Queue.front().mClientID;
-		AString UserName = mQueue.front().mName;
+		AString UserName = m_Queue.front().mName;
-		AString ActualAddress = mAddress;
+		AString ActualAddress = m_Address;
 		ReplaceString(ActualAddress, "%USERNAME%", UserName);
 		ReplaceString(ActualAddress, "%SERVERID%", cRoot::Get()->GetServer()->GetServerID());
-		mQueue.pop_front();
+		m_Queue.pop_front();
 		Lock.Unlock();
 
-		if (!AuthFromAddress(mServer, ActualAddress, UserName))
+		if (!AuthFromAddress(m_Server, ActualAddress, UserName))
 		{
 			cRoot::Get()->KickUser(ClientID, "Failed to authenticate account!");
 		}

source/cAuthenticator.h

@@ -59,13 +59,13 @@ private:
 	
 	typedef std::deque<cUser> cUserList;
 	
-	cCriticalSection mCS;
+	cCriticalSection m_CS;
-	cUserList        mQueue;
+	cUserList        m_Queue;
-	cEvent           mQueueNonempty;
+	cEvent           m_QueueNonempty;
 	
-	AString mServer;
+	AString m_Server;
-	AString mAddress;
+	AString m_Address;
-	bool    mShouldAuthenticate;
+	bool    m_ShouldAuthenticate;
 	
 	// cIsThread override:
 	virtual void Execute(void) override;

source/cChunkGenerator.cpp

@@ -69,7 +69,7 @@ bool cChunkGenerator::Start(cWorld * a_World, const AString & a_WorldGeneratorNa
 
 void cChunkGenerator::Stop(void)
 {
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
 	m_Event.Set();
 	m_evtRemoved.Set();  // Wake up anybody waiting for empty queue
 	Wait();
@@ -115,7 +115,7 @@ void cChunkGenerator::GenerateChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ)
 void cChunkGenerator::WaitForQueueEmpty(void)
 {
 	cCSLock Lock(m_CS);
-	while (!mShouldTerminate && !m_Queue.empty())
+	while (!m_ShouldTerminate && !m_Queue.empty())
 	{
 		cCSUnlock Unlock(Lock);
 		m_evtRemoved.Wait();
@@ -138,14 +138,14 @@ int cChunkGenerator::GetQueueLength(void)
 
 void cChunkGenerator::Execute(void)
 {
-	while (!mShouldTerminate)
+	while (!m_ShouldTerminate)
 	{
 		cCSLock Lock(m_CS);
 		while (m_Queue.size() == 0)
 		{
 			cCSUnlock Unlock(Lock);
 			m_Event.Wait();
-			if (mShouldTerminate)
+			if (m_ShouldTerminate)
 			{
 				return;
 			}

source/cIsThread.cpp

@@ -51,12 +51,12 @@ static void SetThreadName( DWORD dwThreadID, LPCSTR szThreadName)
 // cIsThread:
 
 cIsThread::cIsThread(const AString & iThreadName) :
-	mThreadName(iThreadName),
+	m_ThreadName(iThreadName),
-	mShouldTerminate(false),
+	m_ShouldTerminate(false),
 	#ifdef _WIN32
-	mHandle(NULL)
+	m_Handle(NULL)
 	#else  // _WIN32
-	mHasStarted(false)
+	m_HasStarted(false)
 	#endif  // else _WIN32
 {
 }
@@ -67,7 +67,7 @@ cIsThread::cIsThread(const AString & iThreadName) :
 
 cIsThread::~cIsThread()
 {
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
 	Wait();
 }
 
@@ -78,34 +78,35 @@ cIsThread::~cIsThread()
 bool cIsThread::Start(void)
 {
 	#ifdef _WIN32
-		ASSERT(mHandle == NULL);  // Has already started one thread?
+		ASSERT(m_Handle == NULL);  // Has already started one thread?
 		
 		// Create the thread suspended, so that the mHandle variable is valid in the thread procedure
 		DWORD ThreadID = 0;
-		mHandle = CreateThread(NULL, 0, thrExecute, this, CREATE_SUSPENDED, &ThreadID);
+		m_Handle = CreateThread(NULL, 0, thrExecute, this, CREATE_SUSPENDED, &ThreadID);
-		if (mHandle == NULL)
+		if (m_Handle == NULL)
 		{
-			LOGERROR("ERROR: Could not create thread \"%s\", GLE = %d!", mThreadName.c_str(), GetLastError());
+			LOGERROR("ERROR: Could not create thread \"%s\", GLE = %d!", m_ThreadName.c_str(), GetLastError());
 			return false;
 		}
-		ResumeThread(mHandle);
+		ResumeThread(m_Handle);
 
 		#if defined(_DEBUG) && defined(_MSC_VER)
 			// Thread naming is available only in MSVC
-			if (!mThreadName.empty())
+			if (!m_ThreadName.empty())
 			{
-				SetThreadName(ThreadID, mThreadName.c_str());
+				SetThreadName(ThreadID, m_ThreadName.c_str());
 			}
 		#endif  // _DEBUG and _MSC_VER
 		
 	#else  // _WIN32
-		ASSERT(!mHasStarted);
+		ASSERT(!m_HasStarted);
 		
-		if (pthread_create(&mHandle, NULL, thrExecute, this))
+		if (pthread_create(&m_Handle, NULL, thrExecute, this))
 		{
-			LOGERROR("ERROR: Could not create thread \"%s\", !", mThreadName.c_str());
+			LOGERROR("ERROR: Could not create thread \"%s\", !", m_ThreadName.c_str());
 			return false;
 		}
+		m_HasStarted = true;
 	#endif  // else _WIN32
 
 	return true;
@@ -119,28 +120,30 @@ bool cIsThread::Wait(void)
 {
 	#ifdef _WIN32
 	
-		if (mHandle == NULL)
+		if (m_Handle == NULL)
 		{
 			return true;
 		}
 		// Cannot log, logger may already be stopped:
-		// LOG("Waiting for thread \"%s\" to terminate.", mThreadName.c_str());
+		// LOG("Waiting for thread \"%s\" to terminate.", m_ThreadName.c_str());
-		int res = WaitForSingleObject(mHandle, INFINITE);
+		int res = WaitForSingleObject(m_Handle, INFINITE);
-		mHandle = NULL;
+		m_Handle = NULL;
 		// Cannot log, logger may already be stopped:
-		// LOG("Thread \"%s\" %s terminated, GLE = %d", mThreadName.c_str(), (res == WAIT_OBJECT_0) ? "" : "not", GetLastError());
+		// LOG("Thread \"%s\" %s terminated, GLE = %d", m_ThreadName.c_str(), (res == WAIT_OBJECT_0) ? "" : "not", GetLastError());
 		return (res == WAIT_OBJECT_0);
 		
 	#else  // _WIN32
 	
-		if (!mHasStarted)
+		if (!m_HasStarted)
 		{
 			return true;
 		}
-		LOG("Waiting for thread \"%s\" to terminate.", mThreadName.c_str());
+		// Cannot log, logger may already be stopped:
-		int res = pthread_join(mHandle, NULL);
+		// LOG("Waiting for thread \"%s\" to terminate.", m_ThreadName.c_str());
-		mHasStarted = false;
+		int res = pthread_join(m_Handle, NULL);
-		LOG("Thread \"%s\" %s terminated, errno = %d", mThreadName.c_str(), (res == 0) ? "" : "not", errno);
+		m_HasStarted = false;
+		// Cannot log, logger may already be stopped:
+		// LOG("Thread \"%s\" %s terminated, errno = %d", m_ThreadName.c_str(), (res == 0) ? "" : "not", errno);
 		return (res == 0);
 		
 	#endif  // else _WIN32

source/cIsThread.h

@@ -28,7 +28,7 @@ class cIsThread
 protected:
 	virtual void Execute(void) = 0;  // This function is called in the new thread's context
 
-	volatile bool mShouldTerminate;  // The overriden Execute() method should check this periodically and terminate if this is true
+	volatile bool m_ShouldTerminate;  // The overriden Execute() method should check this periodically and terminate if this is true
 	
 public:
 	cIsThread(const AString & iThreadName);
@@ -40,26 +40,26 @@ public:
 	static unsigned long GetCurrentID(void);  // Returns the OS-dependent thread ID for the caller's thread
 
 private:
-	AString mThreadName;
+	AString m_ThreadName;
 	
 	#ifdef _WIN32
 	
-		HANDLE mHandle;
+		HANDLE m_Handle;
 		
-		static DWORD_PTR __stdcall thrExecute(LPVOID iParam)
+		static DWORD_PTR __stdcall thrExecute(LPVOID a_Param)
 		{
-			((cIsThread *)iParam)->Execute();
+			((cIsThread *)a_Param)->Execute();
 			return 0;
 		}
 		
 	#else  // _WIN32
 	
-		pthread_t mHandle;
+		pthread_t m_Handle;
-		bool      mHasStarted;
+		bool      m_HasStarted;
 		
-		static void * thrExecute(void * iParam)
+		static void * thrExecute(void * a_Param)
 		{
-			((cIsThread *)iParam)->Execute();
+			((cIsThread *)a_Param)->Execute();
 			return NULL;
 		}
 		

source/cServer.cpp

@@ -666,7 +666,7 @@ cServer::cNotifyWriteThread::cNotifyWriteThread(void) :
 
 cServer::cNotifyWriteThread::~cNotifyWriteThread()
 {
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
 	m_Event.Set();
 	Wait();
 }
@@ -688,14 +688,14 @@ bool cServer::cNotifyWriteThread::Start(cServer * a_Server)
 void cServer::cNotifyWriteThread::Execute(void)
 {
 	cClientHandleList Clients;
-	while (!mShouldTerminate)
+	while (!m_ShouldTerminate)
 	{
 		cCSLock Lock(m_CS);
 		while (m_Clients.size() == 0)
 		{
 			cCSUnlock Unlock(Lock);
 			m_Event.Wait();
-			if (mShouldTerminate)
+			if (m_ShouldTerminate)
 			{
 				return;
 			}

source/cSocketThreads.cpp

@@ -155,7 +155,8 @@ void cSocketThreads::Write(const cSocket * a_Socket, const AString & a_Data)
 		}
 	}  // for itr - m_Threads[]
 	
-	ASSERT(!"Writing to an unknown socket");
+	// This may be perfectly legal, if the socket has been destroyed and the client is finishing up
+	// ASSERT(!"Writing to an unknown socket");
 }
 
 
@@ -224,7 +225,7 @@ cSocketThreads::cSocketThread::cSocketThread(cSocketThreads * a_Parent) :
 
 cSocketThreads::cSocketThread::~cSocketThread()
 {
-	mShouldTerminate = true;
+	m_ShouldTerminate = true;
 	m_ControlSocket1.CloseSocket();
 	m_ControlSocket2.CloseSocket();
 }
@@ -506,7 +507,7 @@ void cSocketThreads::cSocketThread::Execute(void)
 	}
 	
 	// The main thread loop:
-	while (!mShouldTerminate)
+	while (!m_ShouldTerminate)
 	{
 		// Put all sockets into the Read set:
 		fd_set fdRead;

source/cWorld.cpp

@@ -113,7 +113,7 @@ protected:
 			for (int i = 0; i < 20; i++)
 			{
 				cSleep::MilliSleep(100);
-				if (mShouldTerminate)
+				if (m_ShouldTerminate)
 				{
 					return;
 				}