Issue #1767 - Prevent incorrect calling of network change detection function.

This rewrites the websocket channel network change detection function to not
skip part of its logic in a situation that has already been checked, preventing
a thread race. See analysis of the problem in the issue.

2 files changed, 16 insertions, 13 deletions

diff --git a/netwerk/protocol/websocket/WebSocketChannel.cpp b/netwerk/protocol/websocket/WebSocketChannel.cpp
index 76be1c9dd0..952c0d5ee5 100644
--- a/netwerk/protocol/websocket/WebSocketChannel.cpp
+++ b/netwerk/protocol/websocket/WebSocketChannel.cpp
@@ -1247,10 +1247,10 @@ WebSocketChannel::Observe(nsISupports *subject,
         // Next we check mDataStarted, which we need to do on mTargetThread.
         if (!IsOnTargetThread()) {
           mTargetThread->Dispatch(
-            NewRunnableMethod(this, &WebSocketChannel::OnNetworkChanged),
+            NewRunnableMethod(this, &WebSocketChannel::OnNetworkChangedTargetThread),
             NS_DISPATCH_NORMAL);
         } else {
-          OnNetworkChanged();
+          OnNetworkChangedTargetThread();
         }
       }
     }
@@ -1260,21 +1260,23 @@ WebSocketChannel::Observe(nsISupports *subject,
 }
 
 nsresult
-WebSocketChannel::OnNetworkChanged()
+WebSocketChannel::OnNetworkChangedTargetThread()
 {
-  if (IsOnTargetThread()) {
-    LOG(("WebSocketChannel::OnNetworkChanged() - on target thread %p", this));
-
-    if (!mDataStarted) {
-      LOG(("WebSocket: data not started yet, no ping needed"));
-      return NS_OK;
-    }
+  LOG(("WebSocketChannel::OnNetworkChangedTargetThread() - on target thread %p", this));
 
-    return mSocketThread->Dispatch(
-      NewRunnableMethod(this, &WebSocketChannel::OnNetworkChanged),
-      NS_DISPATCH_NORMAL);
+  if (!mDataStarted) {
+    LOG(("WebSocket: data not started yet, no ping needed"));
+    return NS_OK;
   }
 
+  return mSocketThread->Dispatch(
+    NewRunnableMethod(this, &WebSocketChannel::OnNetworkChanged),
+    NS_DISPATCH_NORMAL);
+}
+
+nsresult
+WebSocketChannel::OnNetworkChanged()
+{
   MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not socket thread");
 
   LOG(("WebSocketChannel::OnNetworkChanged() - on socket thread %p", this));
diff --git a/netwerk/protocol/websocket/WebSocketChannel.h b/netwerk/protocol/websocket/WebSocketChannel.h
index 631edebfaf..f60bc4fbbe 100644
--- a/netwerk/protocol/websocket/WebSocketChannel.h
+++ b/netwerk/protocol/websocket/WebSocketChannel.h
@@ -146,6 +146,7 @@ private:
   void GeneratePong(uint8_t *payload, uint32_t len);
   void GeneratePing();
 
+  nsresult OnNetworkChangedTargetThread();

   nsresult OnNetworkChanged();
   nsresult StartPinging();