diff options
author | Chris Wilson <chris+github@qwirx.com> | 2006-12-03 13:58:14 +0000 |
---|---|---|
committer | Chris Wilson <chris+github@qwirx.com> | 2006-12-03 13:58:14 +0000 |
commit | cff19bd8c52b58095aaf2aa35f38673c29f1aa88 (patch) | |
tree | db721b29a1f290d228805429d3516a486dc13e81 /lib | |
parent | bc82918ca28737fc934c33b69e13bb2efb28d89e (diff) |
Fixed a race condition caused by rescheduling in signal handler (refs
#3, refs #9)
Diffstat (limited to 'lib')
-rw-r--r-- | lib/common/Timer.cpp | 84 | ||||
-rw-r--r-- | lib/common/Timer.h | 56 |
2 files changed, 89 insertions, 51 deletions
diff --git a/lib/common/Timer.cpp b/lib/common/Timer.cpp index 76b700d3..10365e03 100644 --- a/lib/common/Timer.cpp +++ b/lib/common/Timer.cpp @@ -17,19 +17,7 @@ #include "MemLeakFindOn.h" std::vector<Timer*>* Timers::spTimers = NULL; - -// -------------------------------------------------------------------------- -// -// Function -// Name: static void TimerSigHandler(int) -// Purpose: Signal handler, notifies Timers class -// Created: 19/3/04 -// -// -------------------------------------------------------------------------- -static void TimerSigHandler(int iUnused) -{ - Timers::Signal(); -} +bool Timers::sRescheduleNeeded = false; // -------------------------------------------------------------------------- // @@ -43,15 +31,13 @@ void Timers::Init() { ASSERT(!spTimers); - #ifdef PLATFORM_CYGWIN - ASSERT(::signal(SIGALRM, TimerSigHandler) == 0); - #elif defined WIN32 + #if defined WIN32 && ! defined PLATFORM_CYGWIN // no support for signals at all InitTimer(); - SetTimerHandler(TimerSigHandler); + SetTimerHandler(Timers::SignalHandler); #else - ASSERT(::signal(SIGALRM, TimerSigHandler) == 0); - #endif // PLATFORM_CYGWIN + ASSERT(::signal(SIGALRM, Timers::SignalHandler) == 0); + #endif // WIN32 && !PLATFORM_CYGWIN spTimers = new std::vector<Timer*>; } @@ -68,15 +54,13 @@ void Timers::Cleanup() { ASSERT(spTimers); - #ifdef PLATFORM_CYGWIN - ASSERT(::signal(SIGALRM, NULL) == TimerSigHandler); - #elif defined WIN32 + #if defined WIN32 && ! defined PLATFORM_CYGWIN // no support for signals at all SetTimerHandler(NULL); FiniTimer(); #else - ASSERT(::signal(SIGALRM, NULL) == TimerSigHandler); - #endif // PLATFORM_CYGWIN + ASSERT(::signal(SIGALRM, NULL) == Timers::SignalHandler); + #endif // WIN32 && !PLATFORM_CYGWIN spTimers->clear(); delete spTimers; @@ -142,17 +126,48 @@ void Timers::Reschedule() { ASSERT(spTimers); + // Set the reschedule-needed flag to false before we start. + // If a timer event occurs while we are scheduling, then we + // may or may not need to reschedule again, but this way + // we will do it anyway. + sRescheduleNeeded = false; + + // scan for and remove expired timers, which requires + // us to restart the scan each time we remove one. + bool restart = true; + while (restart) + { + restart = false; + + for (std::vector<Timer*>::iterator i = spTimers->begin(); + i != spTimers->end(); i++) + { + Timer& rTimer = **i; + if (rTimer.HasExpired()) + { + spTimers->erase(i); + restart = true; + break; + } + } + } + + // Now the only remaining timers should all be in the future. + // Scan to find the next one to fire (earliest deadline). + box_time_t timeNow = GetCurrentBoxTime(); int64_t timeToNextEvent = 0; - + for (std::vector<Timer*>::iterator i = spTimers->begin(); i != spTimers->end(); i++) { Timer& rTimer = **i; - ASSERT(!rTimer.HasExpired()); - int64_t timeToExpiry = rTimer.GetExpiryTime() - timeNow; - if (timeToExpiry <= 0) timeToExpiry = 1; + + if (timeToExpiry <= 0) + { + timeToExpiry = 1; + } if (timeToNextEvent == 0 || timeToNextEvent > timeToExpiry) { @@ -183,14 +198,17 @@ void Timers::Reschedule() // -------------------------------------------------------------------------- // // Function -// Name: static void Timers::Signal() -// Purpose: Called by signal handler. Signals any timers which +// Name: static void Timers::SignalHandler(unused) +// Purpose: Called as signal handler. Signals any timers which // are due or overdue, removes them from the set, -// and reschedules next wakeup. +// and requests a reschedule event. The actual +// reschedule will happen next time someone calls +// Timer::HasExpired(), which may be rather late, +// but we can't do it from here, in signal context. // Created: 5/11/2006 // // -------------------------------------------------------------------------- -void Timers::Signal() +void Timers::SignalHandler(int iUnused) { ASSERT(spTimers); @@ -212,7 +230,7 @@ void Timers::Signal() } } - Reschedule(); + Timers::RequestReschedule(); } Timer::Timer(size_t timeoutSecs) diff --git a/lib/common/Timer.h b/lib/common/Timer.h index e0eb34db..390442df 100644 --- a/lib/common/Timer.h +++ b/lib/common/Timer.h @@ -18,23 +18,7 @@ #include "MemLeakFindOn.h" #include "BoxTime.h" -class Timer -{ -public: - Timer(size_t timeoutSecs); - virtual ~Timer(); - Timer(const Timer &); - Timer &operator=(const Timer &); - -public: - box_time_t GetExpiryTime() { return mExpires; } - bool HasExpired () { return mExpired; } - virtual void OnExpire (); - -private: - box_time_t mExpires; - bool mExpired; -}; +class Timer; // -------------------------------------------------------------------------- // @@ -50,13 +34,49 @@ class Timers private: static std::vector<Timer*>* spTimers; static void Reschedule(); + + static bool sRescheduleNeeded; + static void SignalHandler(int iUnused); public: static void Init(); static void Cleanup(); static void Add (Timer& rTimer); static void Remove(Timer& rTimer); - static void Signal(); + static void RequestReschedule() + { + sRescheduleNeeded = true; + } + + static void RescheduleIfNeeded() + { + if (sRescheduleNeeded) + { + Reschedule(); + } + } +}; + +class Timer +{ +public: + Timer(size_t timeoutSecs); + virtual ~Timer(); + Timer(const Timer &); + Timer &operator=(const Timer &); + +public: + box_time_t GetExpiryTime() { return mExpires; } + virtual void OnExpire(); + bool HasExpired() + { + Timers::RescheduleIfNeeded(); + return mExpired; + } + +private: + box_time_t mExpires; + bool mExpired; }; #include "MemLeakFindOff.h" |