Bug 1132537 - Make WebSocketImpl::Dispatch less racy, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 17 Feb 2015 18:40:09 +0100
changeset 256634 b87c58ef866756086ef843a0cb0beddcaef9b2c0
parent 256633 a9a074f02927f4661d68297b139df2fb77e6b2c0
child 256635 bd746b89c47cebb9a08642d0ea6f6aca8cb28739
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1132537
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1132537 - Make WebSocketImpl::Dispatch less racy, r=smaug
dom/base/WebSocket.cpp
--- a/dom/base/WebSocket.cpp
+++ b/dom/base/WebSocket.cpp
@@ -85,16 +85,17 @@ public:
   , mCloseEventCode(nsIWebSocketChannel::CLOSE_ABNORMAL)
   , mScriptLine(0)
   , mInnerWindowID(0)
   , mWorkerPrivate(nullptr)
 #ifdef DEBUG
   , mHasFeatureRegistered(false)
 #endif
   , mIsMainThread(true)
+  , mMutex("WebSocketImpl::mMutex")
   , mWorkerShuttingDown(false)
   {
     if (!NS_IsMainThread()) {
       mWorkerPrivate = GetCurrentThreadWorkerPrivate();
       MOZ_ASSERT(mWorkerPrivate);
       mIsMainThread = false;
     }
   }
@@ -217,16 +218,19 @@ public:
     MutexAutoLock lock(mWebSocket->mMutex);
     mHasFeatureRegistered = aValue;
   }
 #endif
 
   nsWeakPtr mWeakLoadGroup;
 
   bool mIsMainThread;
+
+  // This mutex protects mWorkerShuttingDown.
+  mozilla::Mutex mMutex;
   bool mWorkerShuttingDown;
 
 private:
   ~WebSocketImpl()
   {
     // If we threw during Init we never called disconnect
     if (!mDisconnectingOrDisconnected) {
       Disconnect();
@@ -417,17 +421,24 @@ class MOZ_STACK_CLASS MaybeDisconnect
 public:
   explicit MaybeDisconnect(WebSocketImpl* aImpl)
     : mImpl(aImpl)
   {
   }
 
   ~MaybeDisconnect()
   {
-    if (mImpl->mWorkerShuttingDown) {
+    bool toDisconnect = false;
+
+    {
+      MutexAutoLock lock(mImpl->mMutex);
+      toDisconnect = mImpl->mWorkerShuttingDown;
+    }
+
+    if (toDisconnect) {
       mImpl->Disconnect();
     }
   }
 
 private:
   WebSocketImpl* mImpl;
 };
 
@@ -881,17 +892,17 @@ WebSocketImpl::GetInterface(const nsIID&
 
 WebSocket::WebSocket(nsPIDOMWindow* aOwnerWindow)
   : DOMEventTargetHelper(aOwnerWindow)
   , mIsMainThread(true)
   , mKeepingAlive(false)
   , mCheckMustKeepAlive(true)
   , mOutgoingBufferedAmount(0)
   , mBinaryType(dom::BinaryType::Blob)
-  , mMutex("WebSocketImpl::mMutex")
+  , mMutex("WebSocket::mMutex")
   , mReadyState(CONNECTING)
 {
   mImpl = new WebSocketImpl(this);
   mIsMainThread = mImpl->mIsMainThread;
 }
 
 WebSocket::~WebSocket()
 {
@@ -1977,26 +1988,34 @@ public:
   {
   }
 
   bool Notify(JSContext* aCx, Status aStatus) MOZ_OVERRIDE
   {
     MOZ_ASSERT(aStatus > workers::Running);
 
     if (aStatus >= Canceling) {
-      mWebSocketImpl->mWorkerShuttingDown = true;
+      {
+        MutexAutoLock lock(mWebSocketImpl->mMutex);
+        mWebSocketImpl->mWorkerShuttingDown = true;
+      }
+
       mWebSocketImpl->CloseConnection(nsIWebSocketChannel::CLOSE_GOING_AWAY);
     }
 
     return true;
   }
 
   bool Suspend(JSContext* aCx) MOZ_OVERRIDE
   {
-    mWebSocketImpl->mWorkerShuttingDown = true;
+    {
+      MutexAutoLock lock(mWebSocketImpl->mMutex);
+      mWebSocketImpl->mWorkerShuttingDown = true;
+    }
+
     mWebSocketImpl->CloseConnection(nsIWebSocketChannel::CLOSE_GOING_AWAY);
     return true;
   }
 
 private:
   WebSocketImpl* mWebSocketImpl;
 };
 
@@ -2546,26 +2565,37 @@ WebSocketImpl::SetLoadFlags(nsLoadFlags 
   // we won't change the load flags at all.
   return NS_OK;
 }
 
 namespace {
 
 class WorkerRunnableDispatcher MOZ_FINAL : public WorkerRunnable
 {
+  nsRefPtr<WebSocketImpl> mWebSocketImpl;
+
 public:
-  WorkerRunnableDispatcher(WorkerPrivate* aWorkerPrivate, nsIRunnable* aEvent)
+  WorkerRunnableDispatcher(WebSocketImpl* aImpl, WorkerPrivate* aWorkerPrivate,
+                           nsIRunnable* aEvent)
     : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
+    , mWebSocketImpl(aImpl)
     , mEvent(aEvent)
   {
   }
 
   bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
   {
     aWorkerPrivate->AssertIsOnWorkerThread();
+
+    // No messages when disconnected.
+    if (mWebSocketImpl->mDisconnectingOrDisconnected) {
+      NS_WARNING("Dispatching a WebSocket event after the disconnection!");
+      return true;
+    }
+
     aWorkerPrivate->ModifyBusyCountFromWorker(aCx, true);
     return !NS_FAILED(mEvent->Run());
   }
 
   void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult)
   {
     aWorkerPrivate->ModifyBusyCountFromWorker(aCx, false);
   }
@@ -2591,36 +2621,32 @@ private:
 NS_IMETHODIMP
 WebSocketImpl::Dispatch(nsIRunnable* aEvent, uint32_t aFlags)
 {
   // If the target is the main-thread we can just dispatch the runnable.
   if (mIsMainThread) {
     return NS_DispatchToMainThread(aEvent);
   }
 
-  // No messages when disconnected.
-  if (mDisconnectingOrDisconnected) {
-    NS_WARNING("Dispatching a WebSocket event after the disconnection!");
-    return NS_OK;
-  }
-
+  // If the target is a worker, we have to use a custom WorkerRunnableDispatcher
+  // runnable.
+  nsRefPtr<WorkerRunnableDispatcher> event =
+    new WorkerRunnableDispatcher(this, mWorkerPrivate, aEvent);
+
+  MutexAutoLock lock(mMutex);
   if (mWorkerShuttingDown) {
     return NS_OK;
   }
 
   MOZ_ASSERT(mWorkerPrivate);
 
 #ifdef DEBUG
   MOZ_ASSERT(HasFeatureRegistered());
 #endif
 
-  // If the target is a worker, we have to use a custom WorkerRunnableDispatcher
-  // runnable.
-  nsRefPtr<WorkerRunnableDispatcher> event =
-    new WorkerRunnableDispatcher(mWorkerPrivate, aEvent);
   if (!event->Dispatch(nullptr)) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP