From 1a5ebb44aa4ed3aecc84a8d6f0422d504db1ec44 Mon Sep 17 00:00:00 2001 From: "madmaxoft@gmail.com" Date: Sat, 10 Mar 2012 17:37:00 +0000 Subject: 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 | 6 ++--- source/LightingThread.cpp | 2 +- source/WorldStorage.cpp | 9 +++---- source/cAuthenticator.cpp | 58 +++++++++++++++++++++++----------------------- source/cAuthenticator.h | 12 +++++----- source/cChunkGenerator.cpp | 8 +++---- source/cIsThread.cpp | 53 ++++++++++++++++++++++-------------------- source/cIsThread.h | 18 +++++++------- source/cServer.cpp | 6 ++--- source/cSocketThreads.cpp | 7 +++--- source/cWorld.cpp | 2 +- 11 files changed, 93 insertions(+), 88 deletions(-) diff --git a/source/ChunkSender.cpp b/source/ChunkSender.cpp index 661e0e931..ed650e1f8 100644 --- a/source/ChunkSender.cpp +++ b/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; } diff --git a/source/LightingThread.cpp b/source/LightingThread.cpp index e70dc252e..1bddb81bf 100644 --- a/source/LightingThread.cpp +++ b/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(); diff --git a/source/WorldStorage.cpp b/source/WorldStorage.cpp index 484b87691..791d640c4 100644 --- a/source/WorldStorage.cpp +++ b/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; } diff --git a/source/cAuthenticator.cpp b/source/cAuthenticator.cpp index 7441c6ccd..3865e46c3 100644 --- a/source/cAuthenticator.cpp +++ b/source/cAuthenticator.cpp @@ -24,9 +24,9 @@ cAuthenticator::cAuthenticator(void) : super("cAuthenticator"), - mServer(DEFAULT_AUTH_SERVER), - mAddress(DEFAULT_AUTH_ADDRESS), - mShouldAuthenticate(true) + m_Server(DEFAULT_AUTH_SERVER), + m_Address(DEFAULT_AUTH_ADDRESS), + m_ShouldAuthenticate(true) { ReadINI(); } @@ -53,27 +53,27 @@ void cAuthenticator::ReadINI(void) return; } - mServer = IniFile.GetValue("Authentication", "Server"); - mAddress = IniFile.GetValue("Authentication", "Address"); - mShouldAuthenticate = IniFile.GetValueB("Authentication", "Authenticate", true); + m_Server = IniFile.GetValue("Authentication", "Server"); + m_Address = IniFile.GetValue("Authentication", "Address"); + m_ShouldAuthenticate = IniFile.GetValueB("Authentication", "Authenticate", true); bool bSave = false; - if (mServer.length() == 0) + if (m_Server.length() == 0) { - mServer = DEFAULT_AUTH_SERVER; - IniFile.SetValue("Authentication", "Server", mServer); + m_Server = DEFAULT_AUTH_SERVER; + IniFile.SetValue("Authentication", "Server", m_Server); bSave = true; } - if (mAddress.length() == 0) + if (m_Address.length() == 0) { - mAddress = DEFAULT_AUTH_ADDRESS; - IniFile.SetValue("Authentication", "Address", mAddress); + m_Address = DEFAULT_AUTH_ADDRESS; + 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); - mQueue.push_back(cUser(a_ClientID, a_UserName, a_ServerHash)); - mQueueNonempty.Set(); + cCSLock Lock(m_CS); + m_Queue.push_back(cUser(a_ClientID, a_UserName, a_ServerHash)); + m_QueueNonempty.Set(); } @@ -102,8 +102,8 @@ void cAuthenticator::Authenticate(int a_ClientID, const AString & a_UserName, co void cAuthenticator::Stop(void) { - mShouldTerminate = true; - mQueueNonempty.Set(); + m_ShouldTerminate = true; + m_QueueNonempty.Set(); Wait(); } @@ -115,27 +115,27 @@ void cAuthenticator::Execute(void) { for (;;) { - cCSLock Lock(mCS); - while (!mShouldTerminate && (mQueue.size() == 0)) + cCSLock Lock(m_CS); + 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; - AString UserName = mQueue.front().mName; - AString ActualAddress = mAddress; + int ClientID = m_Queue.front().mClientID; + AString UserName = m_Queue.front().mName; + 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!"); } diff --git a/source/cAuthenticator.h b/source/cAuthenticator.h index 4f33ac036..ba75e8eb3 100644 --- a/source/cAuthenticator.h +++ b/source/cAuthenticator.h @@ -59,13 +59,13 @@ private: typedef std::deque cUserList; - cCriticalSection mCS; - cUserList mQueue; - cEvent mQueueNonempty; + cCriticalSection m_CS; + cUserList m_Queue; + cEvent m_QueueNonempty; - AString mServer; - AString mAddress; - bool mShouldAuthenticate; + AString m_Server; + AString m_Address; + bool m_ShouldAuthenticate; // cIsThread override: virtual void Execute(void) override; diff --git a/source/cChunkGenerator.cpp b/source/cChunkGenerator.cpp index 83375fc5b..e6296baa6 100644 --- a/source/cChunkGenerator.cpp +++ b/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; } diff --git a/source/cIsThread.cpp b/source/cIsThread.cpp index 61dd23b2a..edde68767 100644 --- a/source/cIsThread.cpp +++ b/source/cIsThread.cpp @@ -51,12 +51,12 @@ static void SetThreadName( DWORD dwThreadID, LPCSTR szThreadName) // cIsThread: cIsThread::cIsThread(const AString & iThreadName) : - mThreadName(iThreadName), - mShouldTerminate(false), + m_ThreadName(iThreadName), + 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); - if (mHandle == NULL) + m_Handle = CreateThread(NULL, 0, thrExecute, this, CREATE_SUSPENDED, &ThreadID); + 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()); - int res = WaitForSingleObject(mHandle, INFINITE); - mHandle = NULL; + // LOG("Waiting for thread \"%s\" to terminate.", m_ThreadName.c_str()); + int res = WaitForSingleObject(m_Handle, INFINITE); + 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()); - int res = pthread_join(mHandle, NULL); - mHasStarted = false; - LOG("Thread \"%s\" %s terminated, errno = %d", mThreadName.c_str(), (res == 0) ? "" : "not", errno); + // Cannot log, logger may already be stopped: + // LOG("Waiting for thread \"%s\" to terminate.", m_ThreadName.c_str()); + int res = pthread_join(m_Handle, NULL); + 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 diff --git a/source/cIsThread.h b/source/cIsThread.h index 55bb62a11..20084dfad 100644 --- a/source/cIsThread.h +++ b/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; - bool mHasStarted; + pthread_t m_Handle; + bool m_HasStarted; - static void * thrExecute(void * iParam) + static void * thrExecute(void * a_Param) { - ((cIsThread *)iParam)->Execute(); + ((cIsThread *)a_Param)->Execute(); return NULL; } diff --git a/source/cServer.cpp b/source/cServer.cpp index 51d60d1d4..5412e4be5 100644 --- a/source/cServer.cpp +++ b/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; } diff --git a/source/cSocketThreads.cpp b/source/cSocketThreads.cpp index 74324f909..1aa802a23 100644 --- a/source/cSocketThreads.cpp +++ b/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; diff --git a/source/cWorld.cpp b/source/cWorld.cpp index 4f03ff87e..aab874b7e 100644 --- a/source/cWorld.cpp +++ b/source/cWorld.cpp @@ -113,7 +113,7 @@ protected: for (int i = 0; i < 20; i++) { cSleep::MilliSleep(100); - if (mShouldTerminate) + if (m_ShouldTerminate) { return; } -- cgit v1.2.3