full win32 implementation of ConditionVarFull

panda/src/pipeline/conditionVar.I

@@ -72,6 +72,18 @@ operator = (const ConditionVar &copy) {
   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

panda/src/pipeline/conditionVar.h

@@ -58,6 +58,16 @@ private:
   INLINE ConditionVar(const ConditionVar &copy);
   INLINE void operator = (const ConditionVar &copy);
 
+  // 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;
 };

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);
+  // Wait for either event to become signaled due to signal() being
-  nassertv(result == WAIT_OBJECT_0);
+  // 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 <external_mutex>.
   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.
+  bool have_waiters = AtomicAdjust::get(_waiters_count) > 0;
-  nassertv(false);
+
+  if (have_waiters) {
+    SetEvent(_event_broadcast);
+  }
 }

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
+//               We follow the "SetEvent" implementation suggested by
-//               actually implement a full POSIX-style condition
+//               http://www.cs.wustl.edu/~schmidt/win32-cv-1.html .
-//               variable, but the Event primitive does a fair job if
+//               This allows us to implement both signal() and
-//               we disallow POSIX broadcast.  See
+//               signal_all(), but it has more overhead than the
-//               http://www.cs.wustl.edu/~schmidt/win32-cv-1.html for
+//               simpler implementation of ConditionVarWin32Impl.
-//               a full implementation that includes broadcast.  This
+//
-//               class is much simpler than that full implementation,
+//               As described by the above reference, this
-//               so we can avoid the overhead require to support
+//               implementation suffers from a few weaknesses; in
-//               broadcast.
+//               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"

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
+//               we disallow signal_all() (POSIX broadcast).  See
-//               http://www.cs.wustl.edu/~schmidt/win32-cv-1.html for
+//               ConditionVarFullWin32Impl for a full implementation
-//               a full implementation that includes broadcast.  This
+//               that includes signal_all().  This class is much
-//               class is much simpler than that full implementation,
+//               simpler than that full implementation, so we can
-//               so we can avoid the overhead require to support
+//               avoid the overhead required to support broadcast.
-//               broadcast.
 ////////////////////////////////////////////////////////////////////
 class EXPCL_PANDA ConditionVarWin32Impl {
 public: