[Box Backup-commit] COMMIT r1194 - in box/chris/merge: lib/common test/common
boxbackup-dev@fluffy.co.uk
boxbackup-dev@fluffy.co.uk
Sun, 03 Dec 2006 13:58:14 +0000
Author: chris
Date: 2006-12-03 13:58:14 +0000 (Sun, 03 Dec 2006)
New Revision: 1194
Modified:
box/chris/merge/lib/common/Timer.cpp
box/chris/merge/lib/common/Timer.h
box/chris/merge/test/common/testcommon.cpp
Log:
Fixed a race condition caused by rescheduling in signal handler (refs
#3, refs #9)
Modified: box/chris/merge/lib/common/Timer.cpp
===================================================================
--- box/chris/merge/lib/common/Timer.cpp 2006-12-03 10:52:38 UTC (rev 1193)
+++ box/chris/merge/lib/common/Timer.cpp 2006-12-03 13:58:14 UTC (rev 1194)
@@ -17,23 +17,11 @@
#include "MemLeakFindOn.h"
std::vector<Timer*>* Timers::spTimers = NULL;
+bool Timers::sRescheduleNeeded = false;
// --------------------------------------------------------------------------
//
// Function
-// Name: static void TimerSigHandler(int)
-// Purpose: Signal handler, notifies Timers class
-// Created: 19/3/04
-//
-// --------------------------------------------------------------------------
-static void TimerSigHandler(int iUnused)
-{
- Timers::Signal();
-}
-
-// --------------------------------------------------------------------------
-//
-// Function
// Name: static void Timers::Init()
// Purpose: Initialise timers, prepare signal handler
// Created: 5/11/2006
@@ -43,15 +31,13 @@
{
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 @@
{
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 @@
{
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 @@
// --------------------------------------------------------------------------
//
// 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 @@
}
}
- Reschedule();
+ Timers::RequestReschedule();
}
Timer::Timer(size_t timeoutSecs)
Modified: box/chris/merge/lib/common/Timer.h
===================================================================
--- box/chris/merge/lib/common/Timer.h 2006-12-03 10:52:38 UTC (rev 1193)
+++ box/chris/merge/lib/common/Timer.h 2006-12-03 13:58:14 UTC (rev 1194)
@@ -18,24 +18,8 @@
#include "MemLeakFindOn.h"
#include "BoxTime.h"
-class Timer
-{
-public:
- Timer(size_t timeoutSecs);
- virtual ~Timer();
- Timer(const Timer &);
- Timer &operator=(const Timer &);
+class Timer;
-public:
- box_time_t GetExpiryTime() { return mExpires; }
- bool HasExpired () { return mExpired; }
- virtual void OnExpire ();
-
-private:
- box_time_t mExpires;
- bool mExpired;
-};
-
// --------------------------------------------------------------------------
//
// Class
@@ -50,15 +34,51 @@
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"
#endif // TIMER__H
Modified: box/chris/merge/test/common/testcommon.cpp
===================================================================
--- box/chris/merge/test/common/testcommon.cpp 2006-12-03 10:52:38 UTC (rev 1193)
+++ box/chris/merge/test/common/testcommon.cpp 2006-12-03 13:58:14 UTC (rev 1194)
@@ -238,7 +238,7 @@
CommonException, AssertFailed);
TEST_CHECK_THROWS(Timers::Remove(*(Timer*)NULL),
CommonException, AssertFailed);
- TEST_CHECK_THROWS(Timers::Signal(), CommonException, AssertFailed);
+ // TEST_CHECK_THROWS(Timers::Signal(), CommonException, AssertFailed);
TEST_CHECK_THROWS(Timers::Cleanup(), CommonException, AssertFailed);
// Check that we can initialise the timers