From e2074c41cdf08fc82117a752b2d0f0758f400e20 Mon Sep 17 00:00:00 2001
From: Benjamin Smedberg <benjamin@smedbergs.us>
Date: Thu, 8 Dec 2016 09:05:39 -1000
Subject: [PATCH] Bug 1322377 - frequent intermittent timeouts in
 TestNamedPipeService - there was a race between notifying the monitor and
 waiting for it, so instead of a monitor use an event, r=bagder

MozReview-Commit-ID: 6xgSH6shSm6

--HG--
extra : rebase_source : 36554d14857eb441b52f386a2984d02bd683cdd4
---
 netwerk/test/TestNamedPipeService.cpp | 54 +++++++++++++++++++++------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/netwerk/test/TestNamedPipeService.cpp b/netwerk/test/TestNamedPipeService.cpp
index 45f67e3d8492..71254c23db4a 100644
--- a/netwerk/test/TestNamedPipeService.cpp
+++ b/netwerk/test/TestNamedPipeService.cpp
@@ -18,6 +18,41 @@
 
 using namespace mozilla;
 
+namespace {
+
+/**
+ * Unlike a monitor, an event allows a thread to wait on another thread
+ * completing an action without regard to ordering of the wait and the notify.
+ */
+class Event
+{
+public:
+  explicit Event(const char* aName)
+    : mMonitor(aName) { }
+
+  ~Event() = default;
+
+  void Set() {
+    MonitorAutoLock lock(mMonitor);
+    MOZ_ASSERT(!mSignaled);
+    mSignaled = true;
+    mMonitor.Notify();
+  }
+  void Wait() {
+    MonitorAutoLock lock(mMonitor);
+    while (!mSignaled) {
+      lock.Wait();
+    }
+    mSignaled = false;
+  }
+
+private:
+  Monitor mMonitor;
+  bool mSignaled = false;
+};
+
+} // anonymous namespace
+
 class nsNamedPipeDataObserver : public nsINamedPipeDataObserver
 {
 public:
@@ -37,7 +72,7 @@ private:
   HANDLE mPipe;
   OVERLAPPED mOverlapped;
   Atomic<uint32_t> mBytesTransferred;
-  Monitor mMonitor;
+  Event mEvent;
 };
 
 NS_IMPL_ISUPPORTS(nsNamedPipeDataObserver, nsINamedPipeDataObserver)
@@ -46,7 +81,7 @@ nsNamedPipeDataObserver::nsNamedPipeDataObserver(HANDLE aPipe)
   : mPipe(aPipe)
   , mOverlapped()
   , mBytesTransferred(0)
-  , mMonitor("named-pipe")
+  , mEvent("named-pipe")
 {
   mOverlapped.hEvent = CreateEventA(nullptr, TRUE, TRUE, "named-pipe");
 }
@@ -59,8 +94,7 @@ nsNamedPipeDataObserver::Read(void* aBuffer, uint32_t aSize)
     switch(GetLastError()) {
       case ERROR_IO_PENDING:
         {
-          MonitorAutoLock lock(mMonitor);
-          mMonitor.Wait();
+          mEvent.Wait();
         }
         if (!GetOverlappedResult(mPipe, &mOverlapped, &bytesRead, FALSE)) {
           ADD_FAILURE() << "GetOverlappedResult failed";
@@ -77,8 +111,7 @@ nsNamedPipeDataObserver::Read(void* aBuffer, uint32_t aSize)
         return -1;
     }
   } else {
-    MonitorAutoLock lock(mMonitor);
-    mMonitor.Wait();
+    mEvent.Wait();
 
     if (mBytesTransferred != bytesRead) {
       ADD_FAILURE() << "GetOverlappedResult mismatch";
@@ -98,8 +131,7 @@ nsNamedPipeDataObserver::Write(const void* aBuffer, uint32_t aSize)
     switch(GetLastError()) {
       case ERROR_IO_PENDING:
         {
-          MonitorAutoLock lock(mMonitor);
-          mMonitor.Wait();
+          mEvent.Wait();
         }
         if (!GetOverlappedResult(mPipe, &mOverlapped, &bytesWritten, FALSE)) {
           ADD_FAILURE() << "GetOverlappedResult failed";
@@ -116,8 +148,7 @@ nsNamedPipeDataObserver::Write(const void* aBuffer, uint32_t aSize)
         return -1;
     }
   } else {
-    MonitorAutoLock lock(mMonitor);
-    mMonitor.Wait();
+    mEvent.Wait();
 
     if (mBytesTransferred != bytesWritten) {
       ADD_FAILURE() << "GetOverlappedResult mismatch";
@@ -155,8 +186,7 @@ nsNamedPipeDataObserver::OnDataAvailable(uint32_t aBytesTransferred,
   }
 
   mBytesTransferred += aBytesTransferred;
-  MonitorAutoLock lock(mMonitor);
-  mMonitor.Notify();
+  mEvent.Set();
 
   return NS_OK;
 }