Bug 1876423 - Make SocketProcessParent thread-safe, r=necko-reviewers,valentin

Differential Revision: https://phabricator.services.mozilla.com/D217800
2024-08-07 09:43:30 +00:00 · 2024-08-07 09:43:30 +00:00 · 0aa93e371f
--- a/netwerk/base/Dashboard.cpp
+++ b/netwerk/base/Dashboard.cpp
@ -498,7 +498,9 @@ Dashboard::RequestSockets(nsINetDashboardCallback* aCallback) {
    }

    RefPtr<Dashboard> self(this);
-    SocketProcessParent::GetSingleton()->SendGetSocketData()->Then(
+    RefPtr<SocketProcessParent> socketParent =
+        SocketProcessParent::GetSingleton();
+    socketParent->SendGetSocketData()->Then(
        GetMainThreadSerialEventTarget(), __func__,
        [self{std::move(self)},
         socketData{std::move(socketData)}](SocketDataArgs&& args) {
@ -588,7 +590,9 @@ Dashboard::RequestHttpConnections(nsINetDashboardCallback* aCallback) {
    }

    RefPtr<Dashboard> self(this);
-    SocketProcessParent::GetSingleton()->SendGetHttpConnectionData()->Then(
+    RefPtr<SocketProcessParent> socketParent =
+        SocketProcessParent::GetSingleton();
+    socketParent->SendGetHttpConnectionData()->Then(
        GetMainThreadSerialEventTarget(), __func__,
        [self{std::move(self)}, httpData](nsTArray<HttpRetParams>&& params) {
          httpData->mData.Assign(std::move(params));
@ -834,7 +838,9 @@ Dashboard::RequestDNSInfo(nsINetDashboardCallback* aCallback) {
    }

    RefPtr<Dashboard> self(this);
-    SocketProcessParent::GetSingleton()->SendGetDNSCacheEntries()->Then(
+    RefPtr<SocketProcessParent> socketParent =
+        SocketProcessParent::GetSingleton();
+    socketParent->SendGetDNSCacheEntries()->Then(
        GetMainThreadSerialEventTarget(), __func__,
        [self{std::move(self)},
         dnsData{std::move(dnsData)}](nsTArray<DNSCacheEntries>&& entries) {
--- a/netwerk/base/NetworkConnectivityService.cpp
+++ b/netwerk/base/NetworkConnectivityService.cpp
@ -312,7 +312,7 @@ NetworkConnectivityService::RecheckDNS() {
  }

  if (nsIOService::UseSocketProcess()) {
-    SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+    RefPtr<SocketProcessParent> parent = SocketProcessParent::GetSingleton();
    if (parent) {
      Unused << parent->SendRecheckDNS();
    }
@ -482,7 +482,7 @@ NetworkConnectivityService::RecheckIPConnectivity() {
  }

  if (nsIOService::UseSocketProcess()) {
-    SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+    RefPtr<SocketProcessParent> parent = SocketProcessParent::GetSingleton();
    if (parent) {
      Unused << parent->SendRecheckIPConnectivity();
    }
--- a/netwerk/base/ProxyAutoConfig.cpp
+++ b/netwerk/base/ProxyAutoConfig.cpp
@ -878,7 +878,7 @@ RemoteProxyAutoConfig::~RemoteProxyAutoConfig() = default;
 nsresult RemoteProxyAutoConfig::Init(nsIThread* aPACThread) {
  MOZ_ASSERT(NS_IsMainThread());

-  SocketProcessParent* socketProcessParent =
+  RefPtr<SocketProcessParent> socketProcessParent =
      SocketProcessParent::GetSingleton();
  if (!socketProcessParent) {
    return NS_ERROR_NOT_AVAILABLE;
--- a/netwerk/dns/DNSRequestChild.cpp
+++ b/netwerk/dns/DNSRequestChild.cpp
@ -462,7 +462,9 @@ void DNSRequestSender::StartRequest() {
    RefPtr<DNSRequestParent> requestParent = parent;
    RefPtr<DNSRequestSender> self = this;
    auto task = [requestParent, self]() {
-      Unused << SocketProcessParent::GetSingleton()->SendPDNSRequestConstructor(
+      RefPtr<SocketProcessParent> socketParent =
+          SocketProcessParent::GetSingleton();
+      Unused << socketParent->SendPDNSRequestConstructor(
          requestParent, self->mHost, self->mTrrServer, self->mPort,
          self->mType, self->mOriginAttributes, self->mFlags);
    };
--- a/netwerk/dns/NativeDNSResolverOverrideParent.cpp
+++ b/netwerk/dns/NativeDNSResolverOverrideParent.cpp
@ -34,9 +34,10 @@ NativeDNSResolverOverrideParent::GetSingleton() {
  ClearOnShutdown(&gNativeDNSResolverOverrideParent);

  auto initTask = []() {
-    Unused << SocketProcessParent::GetSingleton()
-                  ->SendPNativeDNSResolverOverrideConstructor(
-                      gNativeDNSResolverOverrideParent);
+    RefPtr<SocketProcessParent> socketParent =
+        SocketProcessParent::GetSingleton();
+    Unused << socketParent->SendPNativeDNSResolverOverrideConstructor(
+        gNativeDNSResolverOverrideParent);
  };
  gIOService->CallOrWaitForSocketProcess(initTask);
  return do_AddRef(gNativeDNSResolverOverrideParent);
--- a/netwerk/dns/TRRServiceParent.cpp
+++ b/netwerk/dns/TRRServiceParent.cpp
@ -47,7 +47,8 @@ void TRRServiceParent::Init() {
    return;
  }

-  SocketProcessParent* socketParent = SocketProcessParent::GetSingleton();
+  RefPtr<SocketProcessParent> socketParent =
+      SocketProcessParent::GetSingleton();
  if (!socketParent) {
    return;
  }
--- a/netwerk/ipc/InputChannelThrottleQueueParent.cpp
+++ b/netwerk/ipc/InputChannelThrottleQueueParent.cpp
@ -83,9 +83,10 @@ InputChannelThrottleQueueParent::Init(uint32_t aMeanBytesPerSecond,
  gIOService->CallOrWaitForSocketProcess(
      [self, meanBytesPerSecond(mMeanBytesPerSecond),
       maxBytesPerSecond(mMaxBytesPerSecond)] {
-        Unused << SocketProcessParent::GetSingleton()
-                      ->SendPInputChannelThrottleQueueConstructor(
-                          self, meanBytesPerSecond, maxBytesPerSecond);
+        RefPtr<SocketProcessParent> socketParent =
+            SocketProcessParent::GetSingleton();
+        Unused << socketParent->SendPInputChannelThrottleQueueConstructor(
+            self, meanBytesPerSecond, maxBytesPerSecond);
      });

  return NS_OK;
--- a/netwerk/ipc/NeckoParent.cpp
+++ b/netwerk/ipc/NeckoParent.cpp
@ -736,7 +736,7 @@ mozilla::ipc::IPCResult NeckoParent::RecvInitSocketProcessBridge(
      return;
    }

-    SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+    RefPtr<SocketProcessParent> parent = SocketProcessParent::GetSingleton();
    if (NS_WARN_IF(!parent)) {
      resolver(std::move(invalidEndpoint));
      return;
--- a/netwerk/ipc/SocketProcessParent.cpp
+++ b/netwerk/ipc/SocketProcessParent.cpp
@ -11,6 +11,7 @@
 #include "HttpTransactionParent.h"
 #include "SocketProcessHost.h"
 #include "TLSClientAuthCertSelection.h"
+#include "mozilla/Atomics.h"
 #include "mozilla/Components.h"
 #include "mozilla/dom/MemoryReportRequest.h"
 #include "mozilla/FOGIPC.h"
@ -45,7 +46,7 @@
 namespace mozilla {
 namespace net {

-static SocketProcessParent* sSocketProcessParent;
+static Atomic<SocketProcessParent*> sSocketProcessParent;

 SocketProcessParent::SocketProcessParent(SocketProcessHost* aHost)
    : mHost(aHost) {
@ -57,17 +58,14 @@ SocketProcessParent::SocketProcessParent(SocketProcessHost* aHost)
 }

 SocketProcessParent::~SocketProcessParent() {
-  MOZ_ASSERT(NS_IsMainThread());
-
  MOZ_COUNT_DTOR(SocketProcessParent);
  sSocketProcessParent = nullptr;
 }

 /* static */
-SocketProcessParent* SocketProcessParent::GetSingleton() {
-  MOZ_ASSERT(NS_IsMainThread());
-
-  return sSocketProcessParent;
+already_AddRefed<SocketProcessParent> SocketProcessParent::GetSingleton() {
+  RefPtr<SocketProcessParent> parent(sSocketProcessParent);
+  return parent.forget();
 }

 void SocketProcessParent::ActorDestroy(ActorDestroyReason aWhy) {
--- a/netwerk/ipc/SocketProcessParent.h
+++ b/netwerk/ipc/SocketProcessParent.h
@ -30,11 +30,11 @@ class SocketProcessParent final
 public:
  friend class SocketProcessHost;

-  NS_INLINE_DECL_REFCOUNTING(SocketProcessParent, final)
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SocketProcessParent, final)

  explicit SocketProcessParent(SocketProcessHost* aHost);

-  static SocketProcessParent* GetSingleton();
+  static already_AddRefed<SocketProcessParent> GetSingleton();

  mozilla::ipc::IPCResult RecvAddMemoryReport(const MemoryReport& aReport);
  mozilla::ipc::IPCResult RecvAccumulateChildHistograms(
--- a/netwerk/protocol/http/AltSvcTransactionParent.cpp
+++ b/netwerk/protocol/http/AltSvcTransactionParent.cpp
@ -36,7 +36,7 @@ AltSvcTransactionParent::~AltSvcTransactionParent() {
 }

 bool AltSvcTransactionParent::Init() {
-  SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+  RefPtr<SocketProcessParent> parent = SocketProcessParent::GetSingleton();
  if (!parent) {
    return false;
  }
--- a/netwerk/protocol/http/nsHttpActivityDistributor.cpp
+++ b/netwerk/protocol/http/nsHttpActivityDistributor.cpp
@ -193,7 +193,8 @@ nsHttpActivityDistributor::AddObserver(nsIHttpActivityObserver* aObserver) {
    mActivated = true;
    if (nsIOService::UseSocketProcess()) {
      auto task = []() {
-        SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+        RefPtr<SocketProcessParent> parent =
+            SocketProcessParent::GetSingleton();
        if (parent && parent->CanSend()) {
          Unused << parent->SendOnHttpActivityDistributorActivated(true);
        }
@ -225,7 +226,7 @@ nsHttpActivityDistributor::RemoveObserver(nsIHttpActivityObserver* aObserver) {

  if (nsIOService::UseSocketProcess() && !mActivated) {
    auto task = []() {
-      SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+      RefPtr<SocketProcessParent> parent = SocketProcessParent::GetSingleton();
      if (parent && parent->CanSend()) {
        Unused << parent->SendOnHttpActivityDistributorActivated(false);
      }
@ -254,7 +255,7 @@ nsHttpActivityDistributor::SetObserveProxyResponse(bool aObserveProxyResponse) {
  mObserveProxyResponse = aObserveProxyResponse;
  if (nsIOService::UseSocketProcess()) {
    auto task = [aObserveProxyResponse]() {
-      SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+      RefPtr<SocketProcessParent> parent = SocketProcessParent::GetSingleton();
      if (parent && parent->CanSend()) {
        Unused << parent->SendOnHttpActivityDistributorObserveProxyResponse(
            aObserveProxyResponse);
@ -283,7 +284,7 @@ nsHttpActivityDistributor::SetObserveConnection(bool aObserveConnection) {
  mObserveConnection = aObserveConnection;
  if (nsIOService::UseSocketProcess()) {
    auto task = [aObserveConnection]() {
-      SocketProcessParent* parent = SocketProcessParent::GetSingleton();
+      RefPtr<SocketProcessParent> parent = SocketProcessParent::GetSingleton();
      if (parent && parent->CanSend()) {
        Unused << parent->SendOnHttpActivityDistributorObserveConnection(
            aObserveConnection);
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@ -1635,7 +1635,8 @@ nsresult nsHttpChannel::InitTransaction() {
    if (NS_WARN_IF(!gIOService->SocketProcessReady())) {
      return NS_ERROR_NOT_AVAILABLE;
    }
-    SocketProcessParent* socketProcess = SocketProcessParent::GetSingleton();
+    RefPtr<SocketProcessParent> socketProcess =
+        SocketProcessParent::GetSingleton();
    if (!socketProcess->CanSend()) {
      return NS_ERROR_NOT_AVAILABLE;
    }
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@ -588,15 +588,15 @@ nsresult nsHttpHandler::InitConnectionMgr() {
    auto task = [self]() {
      RefPtr<HttpConnectionMgrParent> parent =
          self->mConnMgr->AsHttpConnectionMgrParent();
-      Unused << SocketProcessParent::GetSingleton()
-                    ->SendPHttpConnectionMgrConstructor(
-                        parent,
-                        HttpHandlerInitArgs(
-                            self->mLegacyAppName, self->mLegacyAppVersion,
-                            self->mPlatform, self->mOscpu, self->mMisc,
-                            self->mProduct, self->mProductSub, self->mAppName,
-                            self->mAppVersion, self->mCompatFirefox,
-                            self->mCompatDevice, self->mDeviceModelId));
+      RefPtr<SocketProcessParent> socketParent =
+          SocketProcessParent::GetSingleton();
+      Unused << socketParent->SendPHttpConnectionMgrConstructor(
+          parent,
+          HttpHandlerInitArgs(self->mLegacyAppName, self->mLegacyAppVersion,
+                              self->mPlatform, self->mOscpu, self->mMisc,
+                              self->mProduct, self->mProductSub, self->mAppName,
+                              self->mAppVersion, self->mCompatFirefox,
+                              self->mCompatDevice, self->mDeviceModelId));
    };
    gIOService->CallOrWaitForSocketProcess(std::move(task));
  } else {
@ -1158,8 +1158,9 @@ void nsHttpHandler::PrefsChanged(const char* pref) {
      if (!XRE_IsSocketProcess()) {
        mDeviceModelId = mozilla::net::GetDeviceModelId();
        if (gIOService->SocketProcessReady()) {
-          Unused << SocketProcessParent::GetSingleton()
-                        ->SendUpdateDeviceModelId(mDeviceModelId);
+          RefPtr<SocketProcessParent> socketParent =
+              SocketProcessParent::GetSingleton();
+          Unused << socketParent->SendUpdateDeviceModelId(mDeviceModelId);
        }
      }
    } else {
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@ -1929,8 +1929,9 @@ nsNSSComponent::ClearSSLExternalAndInternalSessionCache() {
  if (mozilla::net::nsIOService::UseSocketProcess()) {
    if (mozilla::net::gIOService) {
      mozilla::net::gIOService->CallOrWaitForSocketProcess([]() {
-        Unused << mozilla::net::SocketProcessParent::GetSingleton()
-                      ->SendClearSessionCache();
+        RefPtr<mozilla::net::SocketProcessParent> socketParent =
+            mozilla::net::SocketProcessParent::GetSingleton();
+        Unused << socketParent->SendClearSessionCache();
      });
    }
  }
@ -1960,19 +1961,15 @@ nsNSSComponent::AsyncClearSSLExternalAndInternalSessionCache(

  if (mozilla::net::nsIOService::UseSocketProcess() &&
      mozilla::net::gIOService) {
-    mozilla::net::gIOService->CallOrWaitForSocketProcess(
-        [p = RefPtr{promise}]() {
-          Unused << mozilla::net::SocketProcessParent::GetSingleton()
-                        ->SendClearSessionCache()
-                        ->Then(
-                            GetCurrentSerialEventTarget(), __func__,
-                            [promise = RefPtr{p}] {
-                              promise->MaybeResolveWithUndefined();
-                            },
-                            [promise = RefPtr{p}] {
-                              promise->MaybeReject(NS_ERROR_UNEXPECTED);
-                            });
-        });
+    mozilla::net::gIOService->CallOrWaitForSocketProcess([p = RefPtr{
+                                                              promise}]() {
+      RefPtr<mozilla::net::SocketProcessParent> socketParent =
+          mozilla::net::SocketProcessParent::GetSingleton();
+      Unused << socketParent->SendClearSessionCache()->Then(
+          GetCurrentSerialEventTarget(), __func__,
+          [promise = RefPtr{p}] { promise->MaybeResolveWithUndefined(); },
+          [promise = RefPtr{p}] { promise->MaybeReject(NS_ERROR_UNEXPECTED); });
+    });
  } else {
    promise->MaybeResolveWithUndefined();
  }
--- a/security/sandbox/common/test/SandboxTest.cpp
+++ b/security/sandbox/common/test/SandboxTest.cpp
@ -242,10 +242,10 @@ SandboxTest::StartTests(const nsTArray<nsCString>& aProcessesList) {
          // If socket process was previously disabled by env,
          // nsIOService code will take some time before it creates the new
          // process and it triggers this callback
-          net::SocketProcessParent* parent =
+          RefPtr<net::SocketProcessParent> parent =
              net::SocketProcessParent::GetSingleton();
          if (parent) {
-            return InitializeSandboxTestingActors(parent, processPromise);
+            return InitializeSandboxTestingActors(parent.get(), processPromise);
          }
          return processPromise->Reject(NS_ERROR_FAILURE, __func__);
        });
--- a/toolkit/components/glean/ipc/FOGIPC.cpp
+++ b/toolkit/components/glean/ipc/FOGIPC.cpp
@ -480,7 +480,7 @@ void FlushAllChildData(
    }
  }

-  if (net::SocketProcessParent* socketParent =
+  if (RefPtr<net::SocketProcessParent> socketParent =
          net::SocketProcessParent::GetSingleton()) {
    promises.EmplaceBack(socketParent->SendFlushFOGData());
  }
@ -616,14 +616,14 @@ void TestTriggerMetrics(uint32_t aProcessType,
          [promise]() { promise->MaybeResolveWithUndefined(); },
          [promise]() { promise->MaybeRejectWithUndefined(); });
      break;
-    case nsIXULRuntime::PROCESS_TYPE_SOCKET:
-      Unused << net::SocketProcessParent::GetSingleton()
-                    ->SendTestTriggerMetrics()
-                    ->Then(
-                        GetCurrentSerialEventTarget(), __func__,
-                        [promise]() { promise->MaybeResolveWithUndefined(); },
-                        [promise]() { promise->MaybeRejectWithUndefined(); });
-      break;
+    case nsIXULRuntime::PROCESS_TYPE_SOCKET: {
+      RefPtr<net::SocketProcessParent> socketParent(
+          net::SocketProcessParent::GetSingleton());
+      Unused << socketParent->SendTestTriggerMetrics()->Then(
+          GetCurrentSerialEventTarget(), __func__,
+          [promise]() { promise->MaybeResolveWithUndefined(); },
+          [promise]() { promise->MaybeRejectWithUndefined(); });
+    } break;
    case nsIXULRuntime::PROCESS_TYPE_UTILITY:
      Unused << ipc::UtilityProcessManager::GetSingleton()
                    ->GetProcessParent(ipc::SandboxingKind::GENERIC_UTILITY)
--- a/toolkit/components/telemetry/other/UntrustedModules.cpp
+++ b/toolkit/components/telemetry/other/UntrustedModules.cpp
@ -133,7 +133,8 @@ MultiGetUntrustedModulesData::GetUntrustedModuleLoadEvents() {
    AddPending(contentParent->SendGetUntrustedModulesData());
  }

-  if (auto* socketActor = net::SocketProcessParent::GetSingleton()) {
+  if (RefPtr<net::SocketProcessParent> socketActor =
+          net::SocketProcessParent::GetSingleton()) {
    AddPending(socketActor->SendGetUntrustedModulesData());
  }

--- a/toolkit/xre/dllservices/UntrustedModulesProcessor.cpp
+++ b/toolkit/xre/dllservices/UntrustedModulesProcessor.cpp
@ -193,7 +193,8 @@ NS_IMETHODIMP UntrustedModulesProcessor::Observe(nsISupports* aSubject,
      for (auto* proc : contentProcesses) {
        Unused << proc->SendUnblockUntrustedModulesThread();
      }
-      if (auto* proc = net::SocketProcessParent::GetSingleton()) {
+      if (RefPtr<net::SocketProcessParent> proc =
+              net::SocketProcessParent::GetSingleton()) {
        Unused << proc->SendUnblockUntrustedModulesThread();
      }
      if (auto* rddMgr = RDDProcessManager::Get()) {