diff --git a/panda/src/pipeline/conditionVar.I b/panda/src/pipeline/conditionVar.I index e475ae0816..68377d2002 100644 --- a/panda/src/pipeline/conditionVar.I +++ b/panda/src/pipeline/conditionVar.I @@ -72,6 +72,18 @@ operator = (const ConditionVar ©) { nassertv(false); } +//////////////////////////////////////////////////////////////////// +// Function: ConditionVar::signal_all +// Access: Private +// Description: The signal_all() method is specifically *not* +// provided by ConditionVar. Use ConditionVarFull if +// you need to call this method. +//////////////////////////////////////////////////////////////////// +INLINE void ConditionVar:: +signal_all() { + nassertv(false); +} + //////////////////////////////////////////////////////////////////// // Function: ConditionVar::get_mutex // Access: Public diff --git a/panda/src/pipeline/conditionVar.h b/panda/src/pipeline/conditionVar.h index a218dcff9e..c1758cb288 100644 --- a/panda/src/pipeline/conditionVar.h +++ b/panda/src/pipeline/conditionVar.h @@ -58,6 +58,16 @@ private: INLINE ConditionVar(const ConditionVar ©); INLINE void operator = (const ConditionVar ©); + // These methods are inherited from the base class. + // INLINE void wait(); + // INLINE void signal(); + +private: + // The signal_all() method is specifically *not* provided by + // ConditionVar. Use ConditionVarFull if you need to call this + // method. + INLINE void signal_all(); + public: INLINE Mutex &get_mutex() const; }; diff --git a/panda/src/pipeline/conditionVarFullWin32Impl.I b/panda/src/pipeline/conditionVarFullWin32Impl.I index 8c359809e5..0db939dc6c 100644 --- a/panda/src/pipeline/conditionVarFullWin32Impl.I +++ b/panda/src/pipeline/conditionVarFullWin32Impl.I @@ -26,8 +26,11 @@ INLINE ConditionVarFullWin32Impl:: ConditionVarFullWin32Impl(MutexWin32Impl &mutex) { _external_mutex = &mutex._lock; - // Create an auto-reset event. + // Create an auto-reset event and a manual-reset event. _event_signal = CreateEvent(NULL, false, false, NULL); + _event_broadcast = CreateEvent(NULL, true, false, NULL); + + _waiters_count = 0; } //////////////////////////////////////////////////////////////////// @@ -38,6 +41,7 @@ ConditionVarFullWin32Impl(MutexWin32Impl &mutex) { INLINE ConditionVarFullWin32Impl:: ~ConditionVarFullWin32Impl() { CloseHandle(_event_signal); + CloseHandle(_event_broadcast); } //////////////////////////////////////////////////////////////////// @@ -47,11 +51,28 @@ INLINE ConditionVarFullWin32Impl:: //////////////////////////////////////////////////////////////////// INLINE void ConditionVarFullWin32Impl:: wait() { + AtomicAdjust::inc(_waiters_count); + + // It's ok to release the external_mutex here since Win32 + // manual-reset events maintain state when used with SetEvent(). + // This avoids the "lost wakeup" bug... LeaveCriticalSection(_external_mutex); - DWORD result = WaitForSingleObject(_event_signal, INFINITE); - nassertv(result == WAIT_OBJECT_0); + // Wait for either event to become signaled due to signal() being + // called or signal_all() being called. + int result = WaitForMultipleObjects(2, &_event_signal, FALSE, INFINITE); + bool nonzero = AtomicAdjust::dec(_waiters_count); + bool last_waiter = (result == WAIT_OBJECT_0 + 1 && !nonzero); + + // Some thread called signal_all(). + if (last_waiter) { + // We're the last waiter to be notified or to stop waiting, so + // reset the manual event. + ResetEvent(_event_broadcast); + } + + // Reacquire the . EnterCriticalSection(_external_mutex); } @@ -62,7 +83,11 @@ wait() { //////////////////////////////////////////////////////////////////// INLINE void ConditionVarFullWin32Impl:: signal() { - SetEvent(_event_signal); + bool have_waiters = AtomicAdjust::get(_waiters_count) > 0; + + if (have_waiters) { + SetEvent(_event_signal); + } } //////////////////////////////////////////////////////////////////// @@ -72,6 +97,9 @@ signal() { //////////////////////////////////////////////////////////////////// INLINE void ConditionVarFullWin32Impl:: signal_all() { - // TODO. - nassertv(false); + bool have_waiters = AtomicAdjust::get(_waiters_count) > 0; + + if (have_waiters) { + SetEvent(_event_broadcast); + } } diff --git a/panda/src/pipeline/conditionVarFullWin32Impl.h b/panda/src/pipeline/conditionVarFullWin32Impl.h index e21cacc467..777b7f7dff 100644 --- a/panda/src/pipeline/conditionVarFullWin32Impl.h +++ b/panda/src/pipeline/conditionVarFullWin32Impl.h @@ -26,6 +26,7 @@ #include "mutexWin32Impl.h" #include "pnotify.h" +#include "atomicAdjust.h" class MutexWin32Impl; @@ -34,15 +35,19 @@ class MutexWin32Impl; // Description : Uses Windows native calls to implement a // conditionVarFull. // -// The Windows native synchronization primitives don't -// actually implement a full POSIX-style condition -// variable, but the Event primitive does a fair job if -// we disallow POSIX broadcast. See -// http://www.cs.wustl.edu/~schmidt/win32-cv-1.html for -// a full implementation that includes broadcast. This -// class is much simpler than that full implementation, -// so we can avoid the overhead require to support -// broadcast. +// We follow the "SetEvent" implementation suggested by +// http://www.cs.wustl.edu/~schmidt/win32-cv-1.html . +// This allows us to implement both signal() and +// signal_all(), but it has more overhead than the +// simpler implementation of ConditionVarWin32Impl. +// +// As described by the above reference, this +// implementation suffers from a few weaknesses; in +// particular, it does not necessarily wake up all +// threads fairly; and it may sometimes incorrectly wake +// up a thread that was not waiting at the time signal() +// was called. But we figure it's good enough for our +// purposes. //////////////////////////////////////////////////////////////////// class EXPCL_PANDA ConditionVarFullWin32Impl { public: @@ -56,6 +61,8 @@ public: private: CRITICAL_SECTION *_external_mutex; HANDLE _event_signal; + HANDLE _event_broadcast; + TVOLATILE PN_int32 _waiters_count; }; #include "conditionVarFullWin32Impl.I" diff --git a/panda/src/pipeline/conditionVarWin32Impl.h b/panda/src/pipeline/conditionVarWin32Impl.h index 66a39a4e20..cd1f3678e3 100644 --- a/panda/src/pipeline/conditionVarWin32Impl.h +++ b/panda/src/pipeline/conditionVarWin32Impl.h @@ -37,12 +37,11 @@ class MutexWin32Impl; // The Windows native synchronization primitives don't // actually implement a full POSIX-style condition // variable, but the Event primitive does a fair job if -// we disallow POSIX broadcast. See -// http://www.cs.wustl.edu/~schmidt/win32-cv-1.html for -// a full implementation that includes broadcast. This -// class is much simpler than that full implementation, -// so we can avoid the overhead require to support -// broadcast. +// we disallow signal_all() (POSIX broadcast). See +// ConditionVarFullWin32Impl for a full implementation +// that includes signal_all(). This class is much +// simpler than that full implementation, so we can +// avoid the overhead required to support broadcast. //////////////////////////////////////////////////////////////////// class EXPCL_PANDA ConditionVarWin32Impl { public: