Bug 1555322 - Make sure WebSocketImpl is deleted on the right thread. r=smaug, a=RyanVM
authorKershaw Chang <kershaw@mozilla.com>
Tue, 25 Jun 2019 20:35:38 +0000
changeset 537167 1df87d06a827fb2989f3467da5ce59e155b34d18
parent 537166 12c9d3b85c14cefe91e938b72c63e79f4f722e5d
child 537168 f8c40b71fcb16f6d1713563182c456b6201f7939
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, RyanVM
bugs1555322
milestone68.0
Bug 1555322 - Make sure WebSocketImpl is deleted on the right thread. r=smaug, a=RyanVM I think the root cause of the crash is that WebSocketImpl is not deleted on the target thread. When this happens, there is a race between setting WebSocket::mImpl to nullptr and accessubg mImpl. If WebSocketImpl is always deleted on the target thread, WebSocketImpl::Disconnect should be called in ~WebSocketImpl when mDisconnectingOrDisconnected is false. So, this patch checks the ref counter in WebSocketImpl::Release and make sure to delete WebSocketImpl on the right thread. Differential Revision: https://phabricator.services.mozilla.com/D34320
dom/websocket/WebSocket.cpp
--- a/dom/websocket/WebSocket.cpp
+++ b/dom/websocket/WebSocket.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebSocket.h"
 #include "mozilla/dom/WebSocketBinding.h"
 #include "mozilla/net/WebSocketChannel.h"
 
 #include "jsapi.h"
 #include "jsfriendapi.h"
+#include "mozilla/Atomics.h"
 #include "mozilla/DOMEventTargetHelper.h"
 #include "mozilla/net/WebSocketChannel.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/MessageEvent.h"
 #include "mozilla/dom/MessageEventBinding.h"
 #include "mozilla/dom/nsCSPContext.h"
 #include "mozilla/dom/nsCSPUtils.h"
 #include "mozilla/dom/ScriptSettings.h"
@@ -172,17 +173,17 @@ class WebSocketImpl final : public nsIIn
   bool mIsServerSide;  // True if we're implementing the server side of a
                        // websocket connection
 
   bool mSecure;  // if true it is using SSL and the wss scheme,
                  // otherwise it is using the ws scheme with no SSL
 
   bool mOnCloseScheduled;
   bool mFailed;
-  bool mDisconnectingOrDisconnected;
+  Atomic<bool> mDisconnectingOrDisconnected;
 
   // Set attributes of DOM 'onclose' message
   bool mCloseEventWasClean;
   nsString mCloseEventReason;
   uint16_t mCloseEventCode;
 
   nsCString mAsciiHost;  // hostname
   uint32_t mPort;
@@ -227,19 +228,52 @@ class WebSocketImpl final : public nsIIn
   ~WebSocketImpl() {
     // If we threw during Init we never called disconnect
     if (!mDisconnectingOrDisconnected) {
       Disconnect();
     }
   }
 };
 
-NS_IMPL_ISUPPORTS(WebSocketImpl, nsIInterfaceRequestor, nsIWebSocketListener,
-                  nsIObserver, nsISupportsWeakReference, nsIRequest,
-                  nsIEventTarget)
+NS_IMPL_ADDREF(WebSocketImpl)
+NS_IMPL_QUERY_INTERFACE(WebSocketImpl, nsIInterfaceRequestor,
+                        nsIWebSocketListener, nsIObserver,
+                        nsISupportsWeakReference, nsIRequest, nsIEventTarget)
+
+NS_IMETHODIMP_(MozExternalRefCountType) WebSocketImpl::Release(void) {
+  MOZ_ASSERT(int32_t(mRefCnt) > 0, "dup release");
+
+  if (!mRefCnt.isThreadSafe) {
+    NS_ASSERT_OWNINGTHREAD(WebSocketImpl);
+  }
+
+  nsrefcnt count = mRefCnt - 1;
+  // If WebSocketImpl::Disconnect is not called, the last release of
+  // WebSocketImpl should be on the right thread.
+  if (count == 0 && !IsTargetThread() && !mDisconnectingOrDisconnected) {
+    DebugOnly<nsresult> rv = Dispatch(NewNonOwningRunnableMethod(
+        "dom::WebSocketImpl::Release", this, &WebSocketImpl::Release));
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+    return count;
+  }
+
+  count = --mRefCnt;
+  NS_LOG_RELEASE(this, count, "WebSocketImpl");
+
+  if (count == 0) {
+    if (!mRefCnt.isThreadSafe) {
+      NS_ASSERT_OWNINGTHREAD(WebSocketImpl);
+    }
+
+    mRefCnt = 1; /* stabilize */
+    delete (this);
+    return 0;
+  }
+  return count;
+}
 
 class CallDispatchConnectionCloseEvents final : public CancelableRunnable {
  public:
   explicit CallDispatchConnectionCloseEvents(WebSocketImpl* aWebSocketImpl)
       : CancelableRunnable("dom::CallDispatchConnectionCloseEvents"),
         mWebSocketImpl(aWebSocketImpl) {
     aWebSocketImpl->AssertIsOnTargetThread();
   }