Backed out changeset a0cf88b1fe5b (bug 1487287)
authorJed Davis <jld@mozilla.com>
Thu, 10 Jan 2019 13:55:31 -0700
changeset 510472 191057a8eccf5db9b6bf18cbcb9dbbd5f82e10d1
parent 510471 0140cfbc71db4e1c546448a97cbf536539997949
child 510473 4ef4c81995f62e908208a3c99f2e34664ae5adab
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1487287
milestone66.0a1
backs outa0cf88b1fe5b2b34d29b621e454d43fff788dbc7
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
Backed out changeset a0cf88b1fe5b (bug 1487287)
ipc/glue/GeckoChildProcessHost.cpp
ipc/glue/GeckoChildProcessHost.h
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -28,26 +28,22 @@
 #include "nsAppDirectoryServiceDefs.h"
 #endif
 
 #include "nsExceptionHandler.h"
 
 #include "nsDirectoryServiceDefs.h"
 #include "nsIFile.h"
 #include "nsPrintfCString.h"
-#include "nsIObserverService.h"
 
+#include "mozilla/ClearOnShutdown.h"
 #include "mozilla/ipc/BrowserProcessSubThread.h"
 #include "mozilla/ipc/EnvironmentMap.h"
 #include "mozilla/Omnijar.h"
-#include "mozilla/RecordReplay.h"
 #include "mozilla/Scoped.h"
-#include "mozilla/Services.h"
-#include "mozilla/SharedThreadPool.h"
-#include "mozilla/StaticMutex.h"
 #include "mozilla/Telemetry.h"
 #include "ProtocolUtils.h"
 #include <sys/stat.h>
 
 #ifdef XP_WIN
 #include "nsIWinTaskbar.h"
 #include <stdlib.h>
 #define NS_TASKBAR_CONTRACTID "@mozilla.org/windows-taskbar;1"
@@ -66,17 +62,16 @@
 #include "nsTArray.h"
 #include "nsClassHashtable.h"
 #include "nsHashKeys.h"
 #include "nsNativeCharsetUtils.h"
 #include "nscore.h"  // for NS_FREE_PERMANENT_DATA
 #include "private/pprio.h"
 
 using mozilla::MonitorAutoLock;
-using mozilla::StaticMutexAutoLock;
 using mozilla::ipc::GeckoChildProcessHost;
 
 namespace mozilla {
 MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedPRFileDesc, PRFileDesc,
                                           PR_Close)
 }
 
 using mozilla::ScopedPRFileDesc;
@@ -465,140 +460,36 @@ void GeckoChildProcessHost::GetChildLogN
     buffer.Append(origLogName);
   }
 
   // Append child-specific postfix to name
   buffer.AppendLiteral(".child-");
   buffer.AppendInt(mChildCounter);
 }
 
-namespace {
-// Windows needs a single dedicated thread for process launching,
-// because of thread-safety restrictions/assertions in the sandbox
-// code.  (This implementation isn't itself Windows-specific, so
-// the ifdef can be changed to test on other platforms.)
-#ifdef XP_WIN
-
-static mozilla::StaticMutex gIPCLaunchThreadMutex;
-static mozilla::StaticRefPtr<nsIThread> gIPCLaunchThread;
-
-class IPCLaunchThreadObserver final : public nsIObserver {
- public:
-  NS_DECL_ISUPPORTS
-  NS_DECL_NSIOBSERVER
- protected:
-  virtual ~IPCLaunchThreadObserver() = default;
-};
-
-NS_IMPL_ISUPPORTS(IPCLaunchThreadObserver, nsIObserver, nsISupports)
-
-NS_IMETHODIMP
-IPCLaunchThreadObserver::Observe(nsISupports* aSubject, const char* aTopic,
-                                 const char16_t* aData) {
-  MOZ_RELEASE_ASSERT(strcmp(aTopic, "xpcom-shutdown-threads") == 0);
-  StaticMutexAutoLock lock(gIPCLaunchThreadMutex);
-
-  nsresult rv = NS_OK;
-  if (gIPCLaunchThread) {
-    rv = gIPCLaunchThread->Shutdown();
-    gIPCLaunchThread = nullptr;
-  }
-  mozilla::Unused << NS_WARN_IF(NS_FAILED(rv));
-  return rv;
-}
+bool GeckoChildProcessHost::RunPerformAsyncLaunch(
+    std::vector<std::string> aExtraOpts) {
+  InitializeChannel();
 
-static nsCOMPtr<nsIEventTarget> GetIPCLauncher() {
-  StaticMutexAutoLock lock(gIPCLaunchThreadMutex);
-  if (!gIPCLaunchThread) {
-    nsCOMPtr<nsIThread> thread;
-    nsresult rv = NS_NewNamedThread(NS_LITERAL_CSTRING("IPC Launch"),
-                                    getter_AddRefs(thread));
-    if (!NS_WARN_IF(NS_FAILED(rv))) {
-      NS_DispatchToMainThread(
-          NS_NewRunnableFunction("GeckoChildProcessHost::GetIPCLauncher", [] {
-            nsCOMPtr<nsIObserverService> obsService =
-                mozilla::services::GetObserverService();
-            nsCOMPtr<nsIObserver> obs = new IPCLaunchThreadObserver();
-            obsService->AddObserver(obs, "xpcom-shutdown-threads", false);
-          }));
-      gIPCLaunchThread = thread.forget();
-    }
-  }
-
-  nsCOMPtr<nsIEventTarget> thread = gIPCLaunchThread.get();
-  return thread;
-}
-
-#else  // XP_WIN
-
-// Non-Windows platforms can use an on-demand thread pool.
-
-static nsCOMPtr<nsIEventTarget> GetIPCLauncher() {
-  nsCOMPtr<nsIEventTarget> pool =
-      mozilla::SharedThreadPool::Get(NS_LITERAL_CSTRING("IPC Launch"));
-  return pool;
-}
-
-#endif  // XP_WIN
-}  // anonymous namespace
-
-void GeckoChildProcessHost::RunPerformAsyncLaunch(
-    std::vector<std::string> aExtraOpts) {
-  auto fail = [this] {
+  bool ok = PerformAsyncLaunch(aExtraOpts);
+  if (!ok) {
+    // WaitUntilConnected might be waiting for us to signal.
+    // If something failed let's set the error state and notify.
     MonitorAutoLock lock(mMonitor);
     mProcessState = PROCESS_ERROR;
     mHandlePromise->Reject(LaunchError{}, __func__);
     lock.Notify();
-  };
-
-  // This (probably?) needs to happen on the I/O thread.
-  InitializeChannel();
-
-  // But the rest of this doesn't, and shouldn't block IPC messages:
-  auto launchWrapper = [this, fail, aExtraOpts = std::move(aExtraOpts)]() {
-    bool ok = PerformAsyncLaunch(aExtraOpts);
-
-    if (!ok) {
-      // WaitUntilConnected might be waiting for us to signal.
-      // If something failed let's set the error state and notify.
-      fail();
-      CHROMIUM_LOG(ERROR) << "Failed to launch "
-                          << XRE_ChildProcessTypeToString(mProcessType)
-                          << " subprocess";
-      Telemetry::Accumulate(
-          Telemetry::SUBPROCESS_LAUNCH_FAILURE,
-          nsDependentCString(XRE_ChildProcessTypeToString(mProcessType)));
-    }
-  };
-
-  // The Web Replay middleman process launches the actual content
-  // processes, and doesn't initialize enough of XPCOM to use thread
-  // pools.
-  if (!mozilla::recordreplay::IsMiddleman()) {
-    auto launcher = GetIPCLauncher();
-    MOZ_DIAGNOSTIC_ASSERT(launcher != nullptr);
-    // Creating a thread pool shouldn't normally fail, but in case it
-    // does, use the fallback we already have for the middleman case.
-    if (launcher != nullptr) {
-      nsresult rv = launcher->Dispatch(
-          NS_NewRunnableFunction(
-              "ipc::GeckoChildProcessHost::PerformAsyncLaunch", launchWrapper),
-          NS_DISPATCH_NORMAL);
-      if (NS_FAILED(rv)) {
-        fail();
-        CHROMIUM_LOG(ERROR) << "Failed to dispatch launch task for "
-                            << XRE_ChildProcessTypeToString(mProcessType)
-                            << " process; launching during shutdown?";
-      }
-      return;
-    }
+    CHROMIUM_LOG(ERROR) << "Failed to launch "
+                        << XRE_ChildProcessTypeToString(mProcessType)
+                        << " subprocess";
+    Telemetry::Accumulate(
+        Telemetry::SUBPROCESS_LAUNCH_FAILURE,
+        nsDependentCString(XRE_ChildProcessTypeToString(mProcessType)));
   }
-
-  // Fall back to launching on the I/O thread.
-  launchWrapper();
+  return ok;
 }
 
 void
 #if defined(XP_WIN)
 AddAppDirToCommandLine(CommandLine& aCmdLine)
 #else
 AddAppDirToCommandLine(std::vector<std::string>& aCmdLine)
 #endif
@@ -1219,22 +1110,17 @@ bool GeckoChildProcessHost::PerformAsync
   ) {
     MOZ_CRASH("cannot open handle to child process");
   }
 
   CrashReporter::RegisterChildCrashAnnotationFileDescriptor(
       base::GetProcId(process), crashAnnotationReadPipe.forget());
 
   MonitorAutoLock lock(mMonitor);
-  // This runs on a launch thread, but the OnChannel{Connected,Error} callbacks
-  // run on the I/O thread, so it's possible that the state already advanced
-  // beyond PROCESS_CREATED.
-  if (mProcessState < PROCESS_CREATED) {
-    mProcessState = PROCESS_CREATED;
-  }
+  mProcessState = PROCESS_CREATED;
   mHandlePromise->Resolve(process, __func__);
   lock.Notify();
 
   mLaunchOptions = nullptr;
 
   Telemetry::AccumulateTimeDelta(Telemetry::CHILD_PROCESS_LAUNCH_MS, startTS);
 
   return true;
@@ -1249,16 +1135,17 @@ bool GeckoChildProcessHost::OpenPrivileg
   return base::OpenPrivilegedProcessHandle(aPid, &mChildProcessHandle);
 }
 
 void GeckoChildProcessHost::OnChannelConnected(int32_t peer_pid) {
   if (!OpenPrivilegedHandle(peer_pid)) {
     MOZ_CRASH("can't open handle to child process");
   }
   MonitorAutoLock lock(mMonitor);
+  MOZ_DIAGNOSTIC_ASSERT(mProcessState == PROCESS_CREATED);
   mProcessState = PROCESS_CONNECTED;
   lock.Notify();
 }
 
 void GeckoChildProcessHost::OnMessageReceived(IPC::Message&& aMsg) {
   // We never process messages ourself, just save them up for the next
   // listener.
   mQueue.push(std::move(aMsg));
@@ -1266,16 +1153,17 @@ void GeckoChildProcessHost::OnMessageRec
 
 void GeckoChildProcessHost::OnChannelError() {
   // Update the process state to an error state if we have a channel
   // error before we're connected. This fixes certain failures,
   // but does not address the full range of possible issues described
   // in the FIXME comment below.
   MonitorAutoLock lock(mMonitor);
   if (mProcessState < PROCESS_CONNECTED) {
+    MOZ_DIAGNOSTIC_ASSERT(mProcessState == PROCESS_CREATED);
     mProcessState = PROCESS_ERROR;
     lock.Notify();
   }
   // FIXME/bug 773925: save up this error for the next listener.
 }
 
 RefPtr<GeckoChildProcessHost::HandlePromise>
 GeckoChildProcessHost::WhenProcessHandleReady() {
--- a/ipc/glue/GeckoChildProcessHost.h
+++ b/ipc/glue/GeckoChildProcessHost.h
@@ -176,23 +176,23 @@ class GeckoChildProcessHost : public Chi
 #endif
   RefPtr<HandlePromise::Private> mHandlePromise;
 
   bool OpenPrivilegedHandle(base::ProcessId aPid);
 
  private:
   DISALLOW_EVIL_CONSTRUCTORS(GeckoChildProcessHost);
 
-  // Does the actual work for AsyncLaunch; run in a thread pool
-  // (or, on Windows, a dedicated thread).
+  // Does the actual work for AsyncLaunch, on the IO thread.
+  // (TODO, bug 1487287: move this to its own thread(s).)
   bool PerformAsyncLaunch(StringVector aExtraOpts);
 
-  // Called on the I/O thread; creates channel, dispatches
-  // PerformAsyncLaunch, and consolidates error handling.
-  void RunPerformAsyncLaunch(StringVector aExtraOpts);
+  // Also called on the I/O thread; creates channel, launches, and
+  // consolidates error handling.
+  bool RunPerformAsyncLaunch(StringVector aExtraOpts);
 
   enum class BinaryPathType { Self, PluginContainer };
 
   static BinaryPathType GetPathToBinary(FilePath& exePath,
                                         GeckoProcessType processType);
 
   // The buffer is passed to preserve its lifetime until we are done
   // with launching the sub-process.