From 82472d09acacba9fcf31765e4abbfddd3ffbbcd1 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Thu, 23 Oct 2014 11:20:25 +0200 Subject: Reimplemented cEvent using C++11 primitives. Fixes #1523. --- src/OSSupport/Event.cpp | 154 +++++++++++------------------------------------- src/OSSupport/Event.h | 33 ++++++----- 2 files changed, 51 insertions(+), 136 deletions(-) diff --git a/src/OSSupport/Event.cpp b/src/OSSupport/Event.cpp index 87bc110ce..e05de8e15 100644 --- a/src/OSSupport/Event.cpp +++ b/src/OSSupport/Event.cpp @@ -12,90 +12,23 @@ -cEvent::cEvent(void) +cEvent::cEvent(void) : + m_ShouldWait(true) { -#ifdef _WIN32 - m_Event = CreateEvent(nullptr, FALSE, FALSE, nullptr); - if (m_Event == nullptr) - { - LOGERROR("cEvent: cannot create event, GLE = %u. Aborting server.", (unsigned)GetLastError()); - abort(); - } -#else // *nix - m_bIsNamed = false; - m_Event = new sem_t; - if (sem_init(m_Event, 0, 0)) - { - // This path is used by MacOS, because it doesn't support unnamed semaphores. - delete m_Event; - m_bIsNamed = true; - - AString EventName; - Printf(EventName, "cEvent%p", this); - m_Event = sem_open(EventName.c_str(), O_CREAT, 777, 0); - if (m_Event == SEM_FAILED) - { - AString error = GetOSErrorString(errno); - LOGERROR("cEvent: Cannot create event, err = %s. Aborting server.", error.c_str()); - abort(); - } - // Unlink the semaphore immediately - it will continue to function but will not pollute the namespace - // We don't store the name, so can't call this in the destructor - if (sem_unlink(EventName.c_str()) != 0) - { - AString error = GetOSErrorString(errno); - LOGWARN("ERROR: Could not unlink cEvent. (%s)", error.c_str()); - } - } -#endif // *nix } -cEvent::~cEvent() +void cEvent::Wait(void) { -#ifdef _WIN32 - CloseHandle(m_Event); -#else - if (m_bIsNamed) - { - if (sem_close(m_Event) != 0) - { - AString error = GetOSErrorString(errno); - LOGERROR("ERROR: Could not close cEvent. (%s)", error.c_str()); - } - } - else + std::unique_lock Lock(m_Mutex); + while (m_ShouldWait) { - sem_destroy(m_Event); - delete m_Event; - m_Event = nullptr; + m_CondVar.wait(Lock); } -#endif -} - - - - - -void cEvent::Wait(void) -{ - #ifdef _WIN32 - DWORD res = WaitForSingleObject(m_Event, INFINITE); - if (res != WAIT_OBJECT_0) - { - LOGWARN("cEvent: waiting for the event failed: %u, GLE = %u. Continuing, but server may be unstable.", (unsigned)res, (unsigned)GetLastError()); - } - #else - int res = sem_wait(m_Event); - if (res != 0) - { - AString error = GetOSErrorString(errno); - LOGWARN("cEvent: waiting for the event failed: %i, err = %s. Continuing, but server may be unstable.", res, error.c_str()); - } - #endif + m_ShouldWait = true; } @@ -104,45 +37,34 @@ void cEvent::Wait(void) bool cEvent::Wait(int a_TimeoutMSec) { - #ifdef _WIN32 - DWORD res = WaitForSingleObject(m_Event, (DWORD)a_TimeoutMSec); - switch (res) + std::chrono::steady_clock::time_point dst = std::chrono::steady_clock::now() + std::chrono::milliseconds(a_TimeoutMSec); + std::unique_lock Lock(m_Mutex); // We assume that this lock is acquired without much delay - we are the only user of the mutex + while (m_ShouldWait && (std::chrono::steady_clock::now() < dst)) + { + switch (m_CondVar.wait_until(Lock, dst)) { - case WAIT_OBJECT_0: return true; // Regular event signalled - case WAIT_TIMEOUT: return false; // Regular event timeout - default: + case std::cv_status::no_timeout: { - LOGWARN("cEvent: waiting for the event failed: %u, GLE = %u. Continuing, but server may be unstable.", (unsigned)res, (unsigned)GetLastError()); - return false; + // The wait was successful, check for spurious wakeup: + if (!m_ShouldWait) + { + m_ShouldWait = true; + return true; + } + // This was a spurious wakeup, wait again: + continue; } - } - #else - // Get the current time: - timespec timeout; - if (clock_gettime(CLOCK_REALTIME, &timeout) == -1) - { - LOGWARN("cEvent: Getting current time failed: %i, err = %s. Continuing, but the server may be unstable.", errno, GetOSErrorString(errno).c_str()); - return false; - } - - // Add the specified timeout: - timeout.tv_sec += a_TimeoutMSec / 1000; - timeout.tv_nsec += (a_TimeoutMSec % 1000) * 1000000; // 1 msec = 1000000 usec - - // Wait with timeout: - int res = sem_timedwait(m_Event, &timeout); - switch (res) - { - case 0: return true; // Regular event signalled - case ETIMEDOUT: return false; // Regular even timeout - default: + + case std::cv_status::timeout: { - AString error = GetOSErrorString(errno); - LOGWARN("cEvent: waiting for the event failed: %i, err = %s. Continuing, but server may be unstable.", res, error.c_str()); + // The wait timed out, return failure: return false; } - } - #endif + } // switch (wait_until()) + } // while (m_ShouldWait && not timeout) + + // The wait timed out in the while() condition: + return false; } @@ -151,19 +73,11 @@ bool cEvent::Wait(int a_TimeoutMSec) void cEvent::Set(void) { - #ifdef _WIN32 - if (!SetEvent(m_Event)) - { - LOGWARN("cEvent: Could not set cEvent: GLE = %u", (unsigned)GetLastError()); - } - #else - int res = sem_post(m_Event); - if (res != 0) - { - AString error = GetOSErrorString(errno); - LOGWARN("cEvent: Could not set cEvent: %i, err = %s", res, error.c_str()); - } - #endif + { + std::unique_lock Lock(m_Mutex); + m_ShouldWait = false; + } + m_CondVar.notify_one(); } diff --git a/src/OSSupport/Event.h b/src/OSSupport/Event.h index e2fa65a05..5818115be 100644 --- a/src/OSSupport/Event.h +++ b/src/OSSupport/Event.h @@ -1,16 +1,17 @@ // Event.h -// Interfaces to the cEvent object representing an OS-specific synchronization primitive that can be waited-for -// Implemented as an Event on Win and as a 1-semaphore on *nix +// Interfaces to the cEvent object representing a synchronization primitive that can be waited-for +// Implemented using C++11 condition variable and mutex #pragma once -#ifndef CEVENT_H_INCLUDED -#define CEVENT_H_INCLUDED + +#include +#include @@ -20,9 +21,13 @@ class cEvent { public: cEvent(void); - ~cEvent(); + /** Waits until the event has been set. + If the event has been set before it has been waited for, Wait() returns immediately. */ void Wait(void); + + /** Sets the event - releases one thread that has been waiting in Wait(). + If there was no thread waiting, the next call to Wait() will not block. */ void Set (void); /** Waits for the event until either it is signalled, or the (relative) timeout is passed. @@ -31,20 +36,16 @@ public: private: - #ifdef _WIN32 - HANDLE m_Event; - #else - sem_t * m_Event; - bool m_bIsNamed; - #endif -} ; - - - + /** Used for checking for spurious wakeups. */ + bool m_ShouldWait; + /** Mutex protecting m_ShouldWait from multithreaded access. */ + std::mutex m_Mutex; + /** The condition variable used as the Event. */ + std::condition_variable m_CondVar; +} ; -#endif // CEVENT_H_INCLUDED -- cgit v1.2.3 From 4b32c00f667eb37fbf325110e68fbfe62b089b98 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Fri, 24 Oct 2014 10:22:17 +0200 Subject: Added a missing chrono include. --- src/OSSupport/Event.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OSSupport/Event.cpp b/src/OSSupport/Event.cpp index e05de8e15..fbf04b47c 100644 --- a/src/OSSupport/Event.cpp +++ b/src/OSSupport/Event.cpp @@ -7,6 +7,7 @@ #include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules #include "Event.h" +#include #include "Errors.h" -- cgit v1.2.3 From b9777287ca79fbf9abae57e0ab36acf561995f10 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Fri, 24 Oct 2014 11:01:45 +0200 Subject: Moved the chrono include into Globals. --- src/Globals.h | 2 ++ src/OSSupport/Event.cpp | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Globals.h b/src/Globals.h index 582f5fdaa..eb59f499e 100644 --- a/src/Globals.h +++ b/src/Globals.h @@ -251,6 +251,8 @@ template class SizeChecker; #include #include #include +#include + diff --git a/src/OSSupport/Event.cpp b/src/OSSupport/Event.cpp index fbf04b47c..e05de8e15 100644 --- a/src/OSSupport/Event.cpp +++ b/src/OSSupport/Event.cpp @@ -7,7 +7,6 @@ #include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules #include "Event.h" -#include #include "Errors.h" -- cgit v1.2.3 From 0d15261601317efa062d2573ec8d074a89ad4846 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Tue, 4 Nov 2014 15:47:55 +0100 Subject: cEvent: Changed steady_clock to system_clock. --- src/OSSupport/Event.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OSSupport/Event.cpp b/src/OSSupport/Event.cpp index e05de8e15..7007b5dd2 100644 --- a/src/OSSupport/Event.cpp +++ b/src/OSSupport/Event.cpp @@ -37,9 +37,9 @@ void cEvent::Wait(void) bool cEvent::Wait(int a_TimeoutMSec) { - std::chrono::steady_clock::time_point dst = std::chrono::steady_clock::now() + std::chrono::milliseconds(a_TimeoutMSec); + std::chrono::system_clock::time_point dst = std::chrono::system_clock::now() + std::chrono::milliseconds(a_TimeoutMSec); std::unique_lock Lock(m_Mutex); // We assume that this lock is acquired without much delay - we are the only user of the mutex - while (m_ShouldWait && (std::chrono::steady_clock::now() < dst)) + while (m_ShouldWait && (std::chrono::system_clock::now() < dst)) { switch (m_CondVar.wait_until(Lock, dst)) { -- cgit v1.2.3 From 5dbf6018249101a7ef459dffdb2abfbc4984a442 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Tue, 4 Nov 2014 15:56:27 +0100 Subject: cEvent: Changed chrono duration resolution. --- src/OSSupport/Event.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OSSupport/Event.cpp b/src/OSSupport/Event.cpp index 7007b5dd2..d519ad63f 100644 --- a/src/OSSupport/Event.cpp +++ b/src/OSSupport/Event.cpp @@ -37,7 +37,7 @@ void cEvent::Wait(void) bool cEvent::Wait(int a_TimeoutMSec) { - std::chrono::system_clock::time_point dst = std::chrono::system_clock::now() + std::chrono::milliseconds(a_TimeoutMSec); + std::chrono::system_clock::time_point dst = std::chrono::system_clock::now() + std::chrono::microseconds(a_TimeoutMSec * 1000); std::unique_lock Lock(m_Mutex); // We assume that this lock is acquired without much delay - we are the only user of the mutex while (m_ShouldWait && (std::chrono::system_clock::now() < dst)) { -- cgit v1.2.3 From c65bb6341dfc25ae937bd12c9e41855fb27fdccb Mon Sep 17 00:00:00 2001 From: Mattes D Date: Sun, 7 Dec 2014 21:37:47 +0100 Subject: Fixed integer overflow problems. The event would overflow when requesting a 60 minute timeout. --- src/OSSupport/Event.cpp | 6 +++--- src/OSSupport/Event.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OSSupport/Event.cpp b/src/OSSupport/Event.cpp index d519ad63f..d6ba937f9 100644 --- a/src/OSSupport/Event.cpp +++ b/src/OSSupport/Event.cpp @@ -35,11 +35,11 @@ void cEvent::Wait(void) -bool cEvent::Wait(int a_TimeoutMSec) +bool cEvent::Wait(unsigned a_TimeoutMSec) { - std::chrono::system_clock::time_point dst = std::chrono::system_clock::now() + std::chrono::microseconds(a_TimeoutMSec * 1000); + auto dst = std::chrono::system_clock::now() + std::chrono::milliseconds(a_TimeoutMSec); std::unique_lock Lock(m_Mutex); // We assume that this lock is acquired without much delay - we are the only user of the mutex - while (m_ShouldWait && (std::chrono::system_clock::now() < dst)) + while (m_ShouldWait && (std::chrono::system_clock::now() <= dst)) { switch (m_CondVar.wait_until(Lock, dst)) { diff --git a/src/OSSupport/Event.h b/src/OSSupport/Event.h index 5818115be..572388a3f 100644 --- a/src/OSSupport/Event.h +++ b/src/OSSupport/Event.h @@ -32,7 +32,7 @@ public: /** Waits for the event until either it is signalled, or the (relative) timeout is passed. Returns true if the event was signalled, false if the timeout was hit or there was an error. */ - bool Wait(int a_TimeoutMSec); + bool Wait(unsigned a_TimeoutMSec); private: -- cgit v1.2.3 From 2bd03ee1f90f90cf99b0c41600511670235bafb2 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Sun, 7 Dec 2014 21:38:28 +0100 Subject: cMojangAPI: Fixed a possible problem with thread termination order. --- src/Protocol/MojangAPI.cpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Protocol/MojangAPI.cpp b/src/Protocol/MojangAPI.cpp index 67f513e44..570754204 100644 --- a/src/Protocol/MojangAPI.cpp +++ b/src/Protocol/MojangAPI.cpp @@ -161,26 +161,38 @@ class cMojangAPI::cUpdateThread : { typedef cIsThread super; public: - cUpdateThread() : - super("cMojangAPI::cUpdateThread") + cUpdateThread(cMojangAPI & a_MojangAPI) : + super("cMojangAPI::cUpdateThread"), + m_MojangAPI(a_MojangAPI) { } ~cUpdateThread() { + // Notify the thread that it should stop: + m_ShouldTerminate = true; m_evtNotify.Set(); + + // Wait for the thread to actually finish work: Stop(); } protected: + + /** The cMojangAPI instance to update. */ + cMojangAPI & m_MojangAPI; + + /** The event used for notifying that the thread should terminate, as well as timing. */ cEvent m_evtNotify; + + // cIsThread override: virtual void Execute(void) override { do { - cRoot::Get()->GetMojangAPI().Update(); - } while (!m_evtNotify.Wait(60 * 60 * 1000)); // Repeat every 60 minutes + m_MojangAPI.Update(); + } while (!m_ShouldTerminate && !m_evtNotify.Wait(60 * 60 * 1000)); // Repeat every 60 minutes until termination request } } ; @@ -197,7 +209,7 @@ cMojangAPI::cMojangAPI(void) : m_UUIDToProfileServer(DEFAULT_UUID_TO_PROFILE_SERVER), m_UUIDToProfileAddress(DEFAULT_UUID_TO_PROFILE_ADDRESS), m_RankMgr(nullptr), - m_UpdateThread(new cUpdateThread()) + m_UpdateThread(new cUpdateThread(*this)) { } -- cgit v1.2.3