Bug 1600309 - P2: Don't use socket porcess when it crashes too many times, r=necko-reviewers,valentin

Differential Revision: https://phabricator.services.mozilla.com/D134904

modules/libpref/init/StaticPrefList.yaml

@@ -10757,6 +10757,13 @@
   value: 5
   mirror: always
 
+# The maximum count that we allow socket prrocess to crash. If this count is
+# reached, we won't use networking over socket process.
+- name: network.max_socket_process_failed_count
+  type: RelaxedAtomicUint32
+  value: 1
+  mirror: always
+
 #---------------------------------------------------------------------------
 # Prefs starting with "nglayout."
 #---------------------------------------------------------------------------

netwerk/base/nsIIOService.idl

@@ -258,6 +258,11 @@ interface nsIIOService : nsISupports
      * Return true if socket process is launched.
      */
     readonly attribute boolean socketProcessLaunched;
+
+    /**
+     * The pid for socket process.
+     */
+    readonly attribute unsigned long long socketProcessId;
 };
 
 %{C++

netwerk/base/nsIOService.cpp

@@ -18,6 +18,7 @@
 #include "nsXPCOM.h"
 #include "nsIProxiedProtocolHandler.h"
 #include "nsIProxyInfo.h"
+#include "nsDNSService2.h"
 #include "nsEscape.h"
 #include "nsNetUtil.h"
 #include "nsNetCID.h"
@@ -196,6 +197,8 @@ static const char kProfileDoChange[] = "profile-do-change";
 uint32_t nsIOService::gDefaultSegmentSize = 4096;
 uint32_t nsIOService::gDefaultSegmentCount = 24;
 
+uint32_t nsIOService::sSocketProcessCrashedCount = 0;
+
 ////////////////////////////////////////////////////////////////////////////////
 
 nsIOService::nsIOService()
@@ -509,6 +512,12 @@ class SocketProcessListenerProxy : public SocketProcessHost::Listener {
   }
 };
 
+// static
+bool nsIOService::TooManySocketProcessCrash() {
+  return sSocketProcessCrashedCount >=
+         StaticPrefs::network_max_socket_process_failed_count();
+}
+
 nsresult nsIOService::LaunchSocketProcess() {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -588,6 +597,11 @@ bool nsIOService::UseSocketProcess(bool aCheckAgain) {
     return sUseSocketProcess;
   }
 
+  if (TooManySocketProcessCrash()) {
+    LOG(("TooManySocketProcessCrash"));
+    return sUseSocketProcess;
+  }
+
   if (PR_GetEnv("MOZ_FORCE_USE_SOCKET_PROCESS")) {
     sUseSocketProcess = true;
     return sUseSocketProcess;
@@ -677,6 +691,11 @@ void nsIOService::OnProcessUnexpectedShutdown(SocketProcessHost* aHost) {
 
   LOG(("nsIOService::OnProcessUnexpectedShutdown\n"));
   DestroySocketProcess();
+  sSocketProcessCrashedCount++;
+  if (TooManySocketProcessCrash()) {
+    sUseSocketProcessChecked = false;
+    DNSServiceWrapper::SwitchToBackupDNSService();
+  }
 
   nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
   if (observerService) {
@@ -2046,5 +2065,21 @@ nsIOService::GetSocketProcessLaunched(bool* aResult) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsIOService::GetSocketProcessId(uint64_t* aPid) {
+  NS_ENSURE_ARG_POINTER(aPid);
+
+  *aPid = 0;
+  if (!mSocketProcess) {
+    return NS_OK;
+  }
+
+  if (SocketProcessParent* actor = mSocketProcess->GetActor()) {
+    *aPid = (uint64_t)actor->OtherPid();
+  }
+
+  return NS_OK;
+}
+
 }  // namespace net
 }  // namespace mozilla

netwerk/base/nsIOService.h

@@ -146,6 +146,8 @@ class nsIOService final : public nsIIOService,
 
   nsresult LaunchSocketProcess();
 
+  static bool TooManySocketProcessCrash();
+
  private:
   // These shouldn't be called directly:
   // - construct using GetInstance
@@ -235,6 +237,7 @@ class nsIOService final : public nsIIOService,
   uint32_t mTotalRequests{0};
   uint32_t mCacheWon{0};
   uint32_t mNetWon{0};
+  static uint32_t sSocketProcessCrashedCount;
 
   // These timestamps are needed for collecting telemetry on PR_Connect,
   // PR_ConnectContinue and PR_Close blocking time.  If we spend very long

netwerk/dns/nsDNSService2.cpp

@@ -610,6 +610,50 @@ class NotifyDNSResolution : public Runnable {
 
 //-----------------------------------------------------------------------------
 
+static StaticRefPtr<DNSServiceWrapper> gDNSServiceWrapper;
+
+NS_IMPL_ISUPPORTS(DNSServiceWrapper, nsIDNSService, nsPIDNSService)
+
+// static
+already_AddRefed<nsIDNSService> DNSServiceWrapper::GetSingleton() {
+  if (!gDNSServiceWrapper) {
+    gDNSServiceWrapper = new DNSServiceWrapper();
+    gDNSServiceWrapper->mDNSServiceInUse = ChildDNSService::GetSingleton();
+    if (gDNSServiceWrapper->mDNSServiceInUse) {
+      ClearOnShutdown(&gDNSServiceWrapper);
+      nsDNSPrefetch::Initialize(gDNSServiceWrapper);
+    } else {
+      gDNSServiceWrapper = nullptr;
+    }
+  }
+
+  return do_AddRef(gDNSServiceWrapper);
+}
+
+// static
+void DNSServiceWrapper::SwitchToBackupDNSService() {
+  gDNSServiceWrapper->mBackupDNSService = nsDNSService::GetSingleton();
+
+  MutexAutoLock lock(gDNSServiceWrapper->mLock);
+  gDNSServiceWrapper->mBackupDNSService.swap(
+      gDNSServiceWrapper->mDNSServiceInUse);
+}
+
+nsIDNSService* DNSServiceWrapper::DNSService() {
+  MOZ_ASSERT(XRE_IsParentProcess());
+
+  MutexAutoLock lock(mLock);
+  return mDNSServiceInUse.get();
+}
+
+nsPIDNSService* DNSServiceWrapper::PIDNSService() {
+  MOZ_ASSERT(XRE_IsParentProcess());
+
+  nsCOMPtr<nsPIDNSService> service = do_QueryInterface(DNSService());
+  return service.get();
+}
+
+//-----------------------------------------------------------------------------
 NS_IMPL_ISUPPORTS_INHERITED(nsDNSService, DNSServiceBase, nsIDNSService,
                             nsPIDNSService, nsIMemoryReporter)
 
@@ -651,7 +695,11 @@ already_AddRefed<nsIDNSService> nsDNSService::GetXPCOMSingleton() {
         return GetSingleton();
       }
 
-      if (XRE_IsContentProcess() || XRE_IsParentProcess()) {
+      if (XRE_IsParentProcess()) {
+        return DNSServiceWrapper::GetSingleton();
+      }
+
+      if (XRE_IsContentProcess()) {
         return ChildDNSService::GetSingleton();
       }
 

netwerk/dns/nsDNSService2.h

@@ -23,6 +23,27 @@
 
 class nsAuthSSPI;
 
+class DNSServiceWrapper final : public nsPIDNSService {
+ public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_FORWARD_NSPIDNSSERVICE(PIDNSService()->)
+  NS_FORWARD_NSIDNSSERVICE(DNSService()->)
+
+  DNSServiceWrapper() = default;
+
+  static already_AddRefed<nsIDNSService> GetSingleton();
+  static void SwitchToBackupDNSService();
+
+ private:
+  ~DNSServiceWrapper() = default;
+  nsIDNSService* DNSService();
+  nsPIDNSService* PIDNSService();
+
+  mozilla::Mutex mLock{"DNSServiceWrapper.mLock"};
+  nsCOMPtr<nsIDNSService> mDNSServiceInUse;
+  nsCOMPtr<nsIDNSService> mBackupDNSService;
+};
+
 class nsDNSService final : public mozilla::net::DNSServiceBase,
                            public nsPIDNSService,
                            public nsIMemoryReporter {
@@ -43,6 +64,7 @@ class nsDNSService final : public mozilla::net::DNSServiceBase,
 
  protected:
   friend class nsAuthSSPI;
+  friend class DNSServiceWrapper;
 
   nsresult DeprecatedSyncResolve(
       const nsACString& aHostname, uint32_t flags,

netwerk/protocol/http/nsHttpChannel.cpp

@@ -1253,8 +1253,9 @@ nsresult nsHttpChannel::SetupTransaction() {
 
   // create the transaction object
   if (nsIOService::UseSocketProcess()) {
-    MOZ_ASSERT(gIOService->SocketProcessReady(),
-               "Socket process should be ready.");
+    if (NS_WARN_IF(!gIOService->SocketProcessReady())) {
+      return NS_ERROR_NOT_AVAILABLE;
+    }
 
     nsCOMPtr<nsIParentChannel> parentChannel;
     NS_QueryNotificationCallbacks(this, parentChannel);

netwerk/test/unit/test_networking_over_socket_process.js

@@ -6,6 +6,9 @@
 
 ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 var { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm");
+const { TestUtils } = ChromeUtils.import(
+  "resource://testing-common/TestUtils.jsm"
+);
 
 let h2Port;
 let trrServer;
@@ -15,6 +18,7 @@ const certOverrideService = Cc[
 ].getService(Ci.nsICertOverrideService);
 
 function setup() {
+  Services.prefs.setIntPref("network.max_socket_process_failed_count", 2);
   trr_test_setup();
 
   let env = Cc["@mozilla.org/process/environment;1"].getService(
@@ -29,6 +33,7 @@ function setup() {
 
 setup();
 registerCleanupFunction(async () => {
+  Services.prefs.clearUserPref("network.max_socket_process_failed_count");
   trr_clear_prefs();
   if (trrServer) {
     await trrServer.stop();
@@ -104,10 +109,10 @@ add_task(async function setupTRRServer() {
   });
 });
 
-async function doTestSimpleRequest() {
+async function doTestSimpleRequest(fromSocketProcess) {
   let { inRecord } = await new TRRDNSListener("test.example.com", "127.0.0.1");
   inRecord.QueryInterface(Ci.nsIDNSAddrRecord);
-  Assert.ok(inRecord.resolvedInSocketProcess());
+  Assert.equal(inRecord.resolvedInSocketProcess(), fromSocketProcess);
 
   let chan = makeChan(`https://test.example.com/server-timing`);
   let [req] = await channelOpenPromise(chan);
@@ -115,35 +120,49 @@ async function doTestSimpleRequest() {
   Assert.equal(req.getResponseHeader("x-connection-http2"), "yes");
 
   let internal = chan.QueryInterface(Ci.nsIHttpChannelInternal);
-  Assert.ok(internal.isLoadedBySocketProcess);
+  Assert.equal(internal.isLoadedBySocketProcess, fromSocketProcess);
 }
 
 // Test if the data is loaded from socket process.
 add_task(async function testSimpleRequest() {
-  await doTestSimpleRequest();
+  await doTestSimpleRequest(true);
 });
 
-add_task(async function testSimpleRequestAfterCrash() {
-  let main = await ChromeUtils.requestProcInfo();
-  let socketProcessId = 0;
-  for (let child of main.children) {
-    info(`child.type=${child.type}, child.pid=${child.pid}`);
-    if (child.type == "socket") {
-      socketProcessId = child.pid;
-    }
-  }
-
-  Assert.ok(socketProcessId != 0);
-
+function killSocketProcess(pid) {
   const ProcessTools = Cc["@mozilla.org/processtools-service;1"].getService(
     Ci.nsIProcessToolsService
   );
+  ProcessTools.kill(pid);
+}
+
+// Test if socket process is restarted.
+add_task(async function testSimpleRequestAfterCrash() {
+  let socketProcessId = Services.io.socketProcessId;
+  info(`socket process pid is ${socketProcessId}`);
+  Assert.ok(socketProcessId != 0);
+
+  killSocketProcess(socketProcessId);
 
-  ProcessTools.kill(socketProcessId);
   info("wait socket process restart...");
   await new Promise(resolve => setTimeout(resolve, 1000));
+  await TestUtils.waitForCondition(() => Services.io.socketProcessLaunched);
 
-  Assert.ok(Services.io.socketProcessLaunched);
-
-  await doTestSimpleRequest();
+  await doTestSimpleRequest(true);
+});
+
+// Test if data is loaded from parent process.
+add_task(async function testTooManyCrashes() {
+  let socketProcessId = Services.io.socketProcessId;
+  info(`socket process pid is ${socketProcessId}`);
+  Assert.ok(socketProcessId != 0);
+
+  let socketProcessCrashed = false;
+  Services.obs.addObserver(function observe(subject, topic, data) {
+    Services.obs.removeObserver(observe, topic);
+    socketProcessCrashed = true;
+  }, "network:socket-process-crashed");
+
+  killSocketProcess(socketProcessId);
+  await TestUtils.waitForCondition(() => socketProcessCrashed);
+  await doTestSimpleRequest(false);
 });