Bug 1583046: Tighten down threading rules. r=mjf
authorByron Campen [:bwc] <docfaraday@gmail.com>
Thu, 26 Sep 2019 15:15:33 +0000
changeset 495129 44cb42559383ca4a437dc4b1f0fd8852a0fcbefa
parent 495128 1e23b5b444a08e4119d195a6596ec894f708f364
child 495130 da2361310c672ddb217b4e7331600a35d1b6eb06
push id36623
push usershindli@mozilla.com
push dateFri, 27 Sep 2019 04:28:18 +0000
treeherdermozilla-central@dcfdecc355c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjf
bugs1583046
milestone71.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 1583046: Tighten down threading rules. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D47086
media/mtransport/ipc/WebrtcTCPSocket.cpp
media/mtransport/ipc/WebrtcTCPSocket.h
--- a/media/mtransport/ipc/WebrtcTCPSocket.cpp
+++ b/media/mtransport/ipc/WebrtcTCPSocket.cpp
@@ -67,23 +67,25 @@ WebrtcTCPSocket::WebrtcTCPSocket(WebrtcT
 WebrtcTCPSocket::~WebrtcTCPSocket() {
   LOG(("WebrtcTCPSocket::~WebrtcTCPSocket %p\n", this));
 
   NS_ProxyRelease("WebrtcTCPSocket::CleanUpAuthProvider", mMainThread,
                   mAuthProvider.forget());
 }
 
 void WebrtcTCPSocket::SetTabId(dom::TabId aTabId) {
+  MOZ_ASSERT(NS_IsMainThread());
   dom::ContentProcessManager* cpm = dom::ContentProcessManager::GetSingleton();
   dom::ContentParentId cpId = cpm->GetTabProcessId(aTabId);
   mAuthProvider = cpm->GetBrowserParentByProcessAndTabId(cpId, aTabId);
 }
 
 nsresult WebrtcTCPSocket::Write(nsTArray<uint8_t>&& aWriteData) {
   LOG(("WebrtcTCPSocket::Write %p\n", this));
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ALWAYS_SUCCEEDS(
       mSocketThread->Dispatch(NewRunnableMethod<nsTArray<uint8_t>&&>(
           "WebrtcTCPSocket::Write", this, &WebrtcTCPSocket::EnqueueWrite_s,
           std::move(aWriteData))));
 
   return NS_OK;
 }
 
@@ -137,16 +139,17 @@ void WebrtcTCPSocket::CloseWithReason(ns
   InvokeOnClose(aReason);
 }
 
 nsresult WebrtcTCPSocket::Open(
     const nsCString& aHost, const int& aPort, const nsCString& aLocalAddress,
     const int& aLocalPort, bool aUseTls,
     const Maybe<net::WebrtcProxyConfig>& aProxyConfig) {
   LOG(("WebrtcTCPSocket::Open %p\n", this));
+  MOZ_ASSERT(NS_IsMainThread());
 
   if (NS_WARN_IF(mOpened)) {
     LOG(("WebrtcTCPSocket %p: TCP socket already open\n", this));
     CloseWithReason(NS_ERROR_FAILURE);
     return NS_ERROR_FAILURE;
   }
 
   mOpened = true;
@@ -164,32 +167,34 @@ nsresult WebrtcTCPSocket::Open(
     return NS_ERROR_FAILURE;
   }
 
   mTls = aUseTls;
   mLocalAddress = aLocalAddress;
   mLocalPort = aLocalPort;
   mProxyConfig = aProxyConfig;
 
-  if (mProxyConfig.isSome()) {
-    // We need to figure out whether a proxy needs to be used for mURI before
-    // we can start on establishing a connection.
-    rv = DoProxyConfigLookup();
-  } else {
-    rv = OpenWithoutHttpProxy(nullptr);
+  if (!mProxyConfig.isSome()) {
+    OpenWithoutHttpProxy(nullptr);
+    return NS_OK;
   }
 
+  // We need to figure out whether a proxy needs to be used for mURI before
+  // we can start on establishing a connection.
+  rv = DoProxyConfigLookup();
+
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    CloseWithReason(NS_ERROR_FAILURE);
+    CloseWithReason(rv);
   }
 
   return rv;
 }
 
 nsresult WebrtcTCPSocket::DoProxyConfigLookup() {
+  MOZ_ASSERT(NS_IsMainThread());
   nsresult rv;
   nsCOMPtr<nsIProtocolProxyService> pps =
       do_GetService(NS_PROTOCOLPROXYSERVICE_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   nsCOMPtr<nsIChannel> channel;
@@ -216,119 +221,144 @@ nsresult WebrtcTCPSocket::DoProxyConfigL
 
 NS_IMETHODIMP WebrtcTCPSocket::OnProxyAvailable(nsICancelable* aRequest,
                                                 nsIChannel* aChannel,
                                                 nsIProxyInfo* aProxyinfo,
                                                 nsresult aResult) {
   MOZ_ASSERT(NS_IsMainThread());
   mProxyRequest = nullptr;
 
-  nsresult rv;
-
   if (NS_SUCCEEDED(aResult) && aProxyinfo) {
-    rv = aProxyinfo->GetType(mProxyType);
+    nsresult rv = aProxyinfo->GetType(mProxyType);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       CloseWithReason(rv);
       return rv;
     }
 
     if (mProxyType == "http" || mProxyType == "https") {
       rv = OpenWithHttpProxy();
-    } else if (mProxyType == "socks" || mProxyType == "socks4" ||
-               mProxyType == "direct") {
-      rv = OpenWithoutHttpProxy(aProxyinfo);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        CloseWithReason(rv);
+      }
+      return rv;
     }
-  } else {
-    rv = OpenWithoutHttpProxy(nullptr);
+
+    if (mProxyType == "socks" || mProxyType == "socks4" ||
+        mProxyType == "direct") {
+      OpenWithoutHttpProxy(aProxyinfo);
+      return NS_OK;
+    }
   }
 
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    CloseWithReason(rv);
+  OpenWithoutHttpProxy(nullptr);
+
+  return NS_OK;
+}
+
+void WebrtcTCPSocket::OpenWithoutHttpProxy(nsIProxyInfo* aSocksProxyInfo) {
+  if (!OnSocketThread()) {
+    MOZ_ALWAYS_SUCCEEDS(
+        mSocketThread->Dispatch(NewRunnableMethod<nsCOMPtr<nsIProxyInfo>>(
+            "WebrtcTCPSocket::OpenWithoutHttpProxy", this,
+            &WebrtcTCPSocket::OpenWithoutHttpProxy, aSocksProxyInfo)));
+    return;
   }
 
-  return rv;
-}
+  if (mClosed) {
+    return;
+  }
 
-nsresult WebrtcTCPSocket::OpenWithoutHttpProxy(nsIProxyInfo* aSocksProxyInfo) {
   if (NS_WARN_IF(mProxyConfig.isSome() && mProxyConfig->forceProxy() &&
                  !aSocksProxyInfo)) {
-    return NS_ERROR_FAILURE;
+    CloseWithReason(NS_ERROR_FAILURE);
+    return;
   }
 
   nsCString host;
   int32_t port;
 
   nsresult rv = mURI->GetHost(host);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    CloseWithReason(rv);
+    return;
   }
 
   rv = mURI->GetPort(&port);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    CloseWithReason(rv);
+    return;
   }
 
   AutoTArray<nsCString, 1> socketTypes;
   if (mTls) {
     socketTypes.AppendElement(NS_LITERAL_CSTRING("ssl"));
   }
 
   nsCOMPtr<nsISocketTransportService> sts =
       do_GetService("@mozilla.org/network/socket-transport-service;1");
   rv = sts->CreateTransport(socketTypes, host, port, aSocksProxyInfo,
                             getter_AddRefs(mTransport));
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    CloseWithReason(rv);
+    return;
   }
 
   mTransport->SetReuseAddrPort(true);
 
   PRNetAddr prAddr;
   if (NS_WARN_IF(PR_SUCCESS !=
                  PR_InitializeNetAddr(PR_IpAddrAny, mLocalPort, &prAddr))) {
-    return NS_ERROR_FAILURE;
+    CloseWithReason(NS_ERROR_FAILURE);
+    return;
   }
 
   if (NS_WARN_IF(PR_SUCCESS !=
                  PR_StringToNetAddr(mLocalAddress.BeginReading(), &prAddr))) {
-    return NS_ERROR_FAILURE;
+    CloseWithReason(NS_ERROR_FAILURE);
+    return;
   }
 
   mozilla::net::NetAddr addr;
   PRNetAddrToNetAddr(&prAddr, &addr);
   rv = mTransport->Bind(&addr);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    CloseWithReason(rv);
+    return;
   }
 
   nsCOMPtr<nsIInputStream> socketIn;
   rv = mTransport->OpenInputStream(0, 0, 0, getter_AddRefs(socketIn));
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    CloseWithReason(rv);
+    return;
   }
   mSocketIn = do_QueryInterface(socketIn);
   if (NS_WARN_IF(!mSocketIn)) {
-    return NS_ERROR_NULL_POINTER;
+    CloseWithReason(NS_ERROR_NULL_POINTER);
+    return;
   }
 
   nsCOMPtr<nsIOutputStream> socketOut;
   rv = mTransport->OpenOutputStream(nsITransport::OPEN_UNBUFFERED, 0, 0,
                                     getter_AddRefs(socketOut));
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    CloseWithReason(rv);
+    return;
   }
   mSocketOut = do_QueryInterface(socketOut);
   if (NS_WARN_IF(!mSocketOut)) {
-    return NS_ERROR_NULL_POINTER;
+    CloseWithReason(NS_ERROR_NULL_POINTER);
+    return;
   }
 
-  return FinishOpen();
+  FinishOpen();
 }
 
 nsresult WebrtcTCPSocket::OpenWithHttpProxy() {
+  MOZ_ASSERT(NS_IsMainThread(), "not on main thread");
   nsresult rv;
   nsCOMPtr<nsIIOService> ioService;
   ioService = do_GetService(NS_IOSERVICE_CONTRACTID, &rv);
   if (NS_FAILED(rv)) {
     LOG(("WebrtcTCPSocket %p: io service missing\n", this));
     return rv;
   }
 
@@ -403,17 +433,20 @@ nsresult WebrtcTCPSocket::OpenWithHttpPr
   // mode.
 
   return NS_OK;
 }
 
 void WebrtcTCPSocket::EnqueueWrite_s(nsTArray<uint8_t>&& aWriteData) {
   LOG(("WebrtcTCPSocket::EnqueueWrite %p\n", this));
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
-  MOZ_ASSERT(!mClosed, "webrtc TCP socket closed");
+
+  if (mClosed) {
+    return;
+  }
 
   mWriteQueue.emplace_back(std::move(aWriteData));
 
   if (mSocketOut) {
     mSocketOut->AsyncWait(this, 0, 0, nullptr);
   }
 }
 
@@ -507,30 +540,30 @@ WebrtcTCPSocket::OnTransportAvailable(ns
   }
   rv = mTransport->SetRecvBufferSize(minBufferSize);
   if (NS_FAILED(rv)) {
     LOG(("WebrtcProxyChannel::OnTransportAvailable %p recv failed\n", this));
     CloseWithReason(rv);
     return rv;
   }
 
-  return FinishOpen();
+  FinishOpen();
+  return NS_OK;
 }
 
-nsresult WebrtcTCPSocket::FinishOpen() {
+void WebrtcTCPSocket::FinishOpen() {
+  MOZ_ASSERT(OnSocketThread());
   // mTransport, mSocketIn, and mSocketOut are all set. We may have set them in
   // OnTransportAvailable (in the http/https proxy case), or in
   // OpenWithoutHttpProxy. From here on out, this class functions the same for
   // these two cases.
 
   mSocketIn->AsyncWait(this, 0, 0, nullptr);
 
   InvokeOnConnected();
-
-  return NS_OK;
 }
 
 NS_IMETHODIMP
 WebrtcTCPSocket::OnUpgradeFailed(nsresult aErrorCode) {
   LOG(("WebrtcTCPSocket::OnUpgradeFailed %p\n", this));
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   MOZ_ASSERT(!mTransport,
              "already called transport available on webrtc TCP socket");
--- a/media/mtransport/ipc/WebrtcTCPSocket.h
+++ b/media/mtransport/ipc/WebrtcTCPSocket.h
@@ -76,18 +76,18 @@ class WebrtcTCPSocket : public nsIHttpUp
   bool mTls = false;
   Maybe<WebrtcProxyConfig> mProxyConfig;
   nsCString mLocalAddress;
   uint16_t mLocalPort = 0;
   nsCString mProxyType;
 
   nsresult DoProxyConfigLookup();
   nsresult OpenWithHttpProxy();
-  nsresult OpenWithoutHttpProxy(nsIProxyInfo* aSocksProxyInfo);
-  nsresult FinishOpen();
+  void OpenWithoutHttpProxy(nsIProxyInfo* aSocksProxyInfo);
+  void FinishOpen();
   void EnqueueWrite_s(nsTArray<uint8_t>&& aWriteData);
 
   void CloseWithReason(nsresult aReason);
 
   size_t mWriteOffset;
   std::list<WebrtcTCPData> mWriteQueue;
   nsCOMPtr<nsIAuthPromptProvider> mAuthProvider;