Bug 1292181 - Let each dispatch event sends data to the socket. r=mcmanus, a=lizzard
authorDragana Damjanovic <dd.mozilla@gmail.com>
Thu, 11 Aug 2016 16:30:23 -0700
changeset 347725 3784a68ebabe70682746270b4cd7b705c9a60694
parent 347724 ae48669d6b0711b296c1a208bb1511584ab6be5c
child 347726 2893f7dad7dfc81aa7d9771e117008b0e3cdafd6
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus, lizzard
bugs1292181
milestone50.0a2
Bug 1292181 - Let each dispatch event sends data to the socket. r=mcmanus, a=lizzard
netwerk/base/PollableEvent.cpp
netwerk/base/nsSocketTransportService2.cpp
--- a/netwerk/base/PollableEvent.cpp
+++ b/netwerk/base/PollableEvent.cpp
@@ -212,23 +212,37 @@ bool
 PollableEvent::Signal()
 {
   SOCKET_LOG(("PollableEvent::Signal\n"));
 
   if (!mWriteFD) {
     SOCKET_LOG(("PollableEvent::Signal Failed on no FD\n"));
     return false;
   }
+#ifndef XP_WIN
+  // On windows poll can hang and this became worse when we introduced the
+  // patch for bug 698882 (see also bug 1292181), therefore we reverted the
+  // behavior on windows to be as before bug 698882, e.g. write to the socket
+  // also if an event dispatch is on the socket thread and writing to the
+  // socket for each event. See bug 1292181.
   if (PR_GetCurrentThread() == gSocketThread) {
     SOCKET_LOG(("PollableEvent::Signal OnSocketThread nop\n"));
     return true;
   }
+#endif
+
+#ifndef XP_WIN
+  // To wake up the poll writing once is enough, but for Windows that can cause
+  // hangs so we will write for every event.
+  // For non-Windows systems it is enough to write just once.
   if (mSignaled) {
     return true;
   }
+#endif
+
   mSignaled = true;
   int32_t status = PR_Write(mWriteFD, "M", 1);
   SOCKET_LOG(("PollableEvent::Signal PR_Write %d\n", status));
   if (status != 1) {
     NS_WARNING("PollableEvent::Signal Failed\n");
     SOCKET_LOG(("PollableEvent::Signal Failed\n"));
     mSignaled = false;
   }
@@ -243,17 +257,39 @@ PollableEvent::Clear()
 
   SOCKET_LOG(("PollableEvent::Clear\n"));
   mSignaled = false;
   if (!mReadFD) {
     SOCKET_LOG(("PollableEvent::Clear mReadFD is null\n"));
     return false;
   }
   char buf[2048];
-  int32_t status = PR_Read(mReadFD, buf, 2048);
+  int32_t status;
+#ifdef XP_WIN
+  // On Windows we are writing to the socket for each event, to be sure that we
+  // do not have any deadlock read from the socket as much as we can.
+  while (true) {
+    status = PR_Read(mReadFD, buf, 2048);
+    SOCKET_LOG(("PollableEvent::Signal PR_Read %d\n", status));
+    if (status == 0) {
+      SOCKET_LOG(("PollableEvent::Clear EOF!\n"));
+      return false;
+    }
+    if (status < 0) {
+      PRErrorCode code = PR_GetError();
+      if (code == PR_WOULD_BLOCK_ERROR) {
+        return true;
+      } else {
+        SOCKET_LOG(("PollableEvent::Clear unexpected error %d\n", code));
+        return false;
+      }
+    }
+  }
+#else
+  status = PR_Read(mReadFD, buf, 2048);
   SOCKET_LOG(("PollableEvent::Signal PR_Read %d\n", status));
 
   if (status == 1) {
     return true;
   }
   if (status == 0) {
     SOCKET_LOG(("PollableEvent::Clear EOF!\n"));
     return false;
@@ -265,12 +301,13 @@ PollableEvent::Clear()
     return true;
   }
   PRErrorCode code = PR_GetError();
   if (code == PR_WOULD_BLOCK_ERROR) {
     return true;
   }
   SOCKET_LOG(("PollableEvent::Clear unexpected error %d\n", code));
   return false;
+#endif //XP_WIN
+
 }
-
 } // namespace net
 } // namespace mozilla
--- a/netwerk/base/nsSocketTransportService2.cpp
+++ b/netwerk/base/nsSocketTransportService2.cpp
@@ -742,24 +742,31 @@ nsSocketTransportService::CreateUnixDoma
 
     trans.forget(result);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSocketTransportService::OnDispatchedEvent(nsIThreadInternal *thread)
 {
+#ifndef XP_WIN
+    // On windows poll can hang and this became worse when we introduced the
+    // patch for bug 698882 (see also bug 1292181), therefore we reverted the
+    // behavior on windows to be as before bug 698882, e.g. write to the socket
+    // also if an event dispatch is on the socket thread and writing to the
+    // socket for each event.
     if (PR_GetCurrentThread() == gSocketThread) {
         // this check is redundant to one done inside ::Signal(), but
         // we can do it here and skip obtaining the lock - given that
         // this is a relatively common occurance its worth the
         // redundant code
         SOCKET_LOG(("OnDispatchedEvent Same Thread Skip Signal\n"));
         return NS_OK;
     }
+#endif
 
     MutexAutoLock lock(mLock);
     if (mPollableEvent) {
         mPollableEvent->Signal();
     }
     return NS_OK;
 }