Bug 1475382 - Remove debugging crashes added in bug 1461459. r=spohl
authorJed Davis <jld@mozilla.com>
Fri, 13 Jul 2018 15:18:03 -0600
changeset 427221 7c10f1db88f01d23aa1a9406f6a7d9b295feaa42
parent 427220 8149fe4ffa8bbafa66e62f7146e808b04a0140e0
child 427222 99011a1e3da55e8f58054e6646cd10fab9e590b9
push id66557
push userjedavis@mozilla.com
push dateThu, 19 Jul 2018 00:10:06 +0000
treeherderautoland@99011a1e3da5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersspohl
bugs1475382, 1461459
milestone63.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 1475382 - Remove debugging crashes added in bug 1461459. r=spohl These are no longer providing useful information. There are still a noticeable number of failures on Windows, but we've narrowed them down to within SandboxBroker::LaunchApp. MozReview-Commit-ID: 9srWLNZq1Wo
ipc/chromium/src/base/process_util_mac.mm
ipc/glue/GeckoChildProcessHost.cpp
--- a/ipc/chromium/src/base/process_util_mac.mm
+++ b/ipc/chromium/src/base/process_util_mac.mm
@@ -35,19 +35,16 @@ bool LaunchApp(const std::vector<std::st
     argv_copy[i] = const_cast<char*>(argv[i].c_str());
   }
   argv_copy[argv.size()] = NULL;
 
   EnvironmentArray vars = BuildEnvironmentArray(options.env_map);
 
   posix_spawn_file_actions_t file_actions;
   if (posix_spawn_file_actions_init(&file_actions) != 0) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_CRASH("base::LaunchApp: posix_spawn_file_actions_init failed");
-#endif
     return false;
   }
   auto file_actions_guard = mozilla::MakeScopeExit([&file_actions] {
     posix_spawn_file_actions_destroy(&file_actions);
   });
 
   // Turn fds_to_remap array into a set of dup2 calls.
   for (const auto& fd_map : options.fds_to_remap) {
@@ -56,77 +53,54 @@ bool LaunchApp(const std::vector<std::st
 
     if (src_fd == dest_fd) {
       int flags = fcntl(src_fd, F_GETFD);
       if (flags != -1) {
         fcntl(src_fd, F_SETFD, flags & ~FD_CLOEXEC);
       }
     } else {
       if (posix_spawn_file_actions_adddup2(&file_actions, src_fd, dest_fd) != 0) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-        MOZ_CRASH("base::LaunchApp: posix_spawn_file_actions_adddup2 failed");
-#endif
         return false;
       }
     }
   }
 
   // Initialize spawn attributes.
   posix_spawnattr_t spawnattr;
   if (posix_spawnattr_init(&spawnattr) != 0) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_CRASH("base::LaunchApp: posix_spawnattr_init failed");
-#endif
     return false;
   }
   auto spawnattr_guard = mozilla::MakeScopeExit([&spawnattr] {
     posix_spawnattr_destroy(&spawnattr);
   });
 
   // Prevent the child process from inheriting any file descriptors
   // that aren't named in `file_actions`.  (This is an Apple-specific
   // extension to posix_spawn.)
   if (posix_spawnattr_setflags(&spawnattr, POSIX_SPAWN_CLOEXEC_DEFAULT) != 0) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_CRASH("base::LaunchApp: posix_spawnattr_setflags failed");
-#endif
     return false;
   }
 
   // Exempt std{in,out,err} from being closed by POSIX_SPAWN_CLOEXEC_DEFAULT.
   for (int fd = 0; fd <= STDERR_FILENO; ++fd) {
     if (posix_spawn_file_actions_addinherit_np(&file_actions, fd) != 0) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-      MOZ_CRASH("base::LaunchApp: posix_spawn_file_actions_addinherit_np "
-                "failed");
-#endif
       return false;
     }
   }
 
   int pid = 0;
   int spawn_succeeded = (posix_spawnp(&pid,
                                       argv_copy[0],
                                       &file_actions,
                                       &spawnattr,
                                       argv_copy,
                                       vars.get()) == 0);
 
   bool process_handle_valid = pid > 0;
   if (!spawn_succeeded || !process_handle_valid) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    if (!spawn_succeeded && !process_handle_valid) {
-      MOZ_CRASH("base::LaunchApp: spawn_succeeded is false and "
-                "process_handle_valid is false");
-    } else if (!spawn_succeeded) {
-      MOZ_CRASH("base::LaunchApp: spawn_succeeded is false");
-    } else {
-      MOZ_CRASH("base::LaunchApp: process_handle_valid is false");
-    }
-#endif
     retval = false;
   } else {
     gProcessLog.print("==> process %d launched child process %d\n",
                       GetCurrentProcId(), pid);
     if (options.wait)
       HANDLE_EINTR(waitpid(pid, 0, 0));
 
     if (process_handle)
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -622,38 +622,32 @@ AddAppDirToCommandLine(std::vector<std::
 }
 
 bool
 GeckoChildProcessHost::PerformAsyncLaunchInternal(std::vector<std::string>& aExtraOpts)
 {
   // We rely on the fact that InitializeChannel() has already been processed
   // on the IO thread before this point is reached.
   if (!GetChannel()) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(1 == 2);
-#endif
     return false;
   }
 
   base::ProcessHandle process = 0;
 
   // send the child the PID so that it can open a ProcessHandle back to us.
   // probably don't want to do this in the long run
   char pidstring[32];
   SprintfLiteral(pidstring, "%d", base::GetCurrentProcId());
 
   const char* const childProcessType =
       XRE_ChildProcessTypeToString(mProcessType);
 
   PRFileDesc* crashAnnotationReadPipe;
   PRFileDesc* crashAnnotationWritePipe;
   if (PR_CreatePipe(&crashAnnotationReadPipe, &crashAnnotationWritePipe) != PR_SUCCESS) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(2 == 3);
-#endif
     return false;
   }
 
 //--------------------------------------------------
 #if defined(OS_POSIX)
   // For POSIX, we have to be extremely anal about *not* using
   // std::wstring in code compiled with Mozilla's -fshort-wchar
   // configuration, because chromium is compiled with -fno-short-wchar
@@ -839,36 +833,27 @@ GeckoChildProcessHost::PerformAsyncLaunc
   // Wait for the child process to send us its 'task_t' data.
   const int kTimeoutMs = 10000;
 
   MachReceiveMessage child_message;
   kern_return_t err = parent_recv_port.WaitForMessage(&child_message, kTimeoutMs);
   if (err != KERN_SUCCESS) {
     std::string errString = StringPrintf("0x%x %s", err, mach_error_string(err));
     CHROMIUM_LOG(ERROR) << "parent WaitForMessage() failed: " << errString;
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(3 == 4);
-#endif
     return false;
   }
 
   task_t child_task = child_message.GetTranslatedPort(0);
   if (child_task == MACH_PORT_NULL) {
     CHROMIUM_LOG(ERROR) << "parent GetTranslatedPort(0) failed.";
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(4 == 5);
-#endif
     return false;
   }
 
   if (child_message.GetTranslatedPort(1) == MACH_PORT_NULL) {
     CHROMIUM_LOG(ERROR) << "parent GetTranslatedPort(1) failed.";
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(5 == 6);
-#endif
     return false;
   }
   MachPortSender parent_sender(child_message.GetTranslatedPort(1));
 
   if (child_message.GetTranslatedPort(2) == MACH_PORT_NULL) {
     CHROMIUM_LOG(ERROR) << "parent GetTranslatedPort(2) failed.";
   }
   auto* parent_recv_port_memory_ack = new MachPortSender(child_message.GetTranslatedPort(2));
@@ -876,47 +861,35 @@ GeckoChildProcessHost::PerformAsyncLaunc
   if (child_message.GetTranslatedPort(3) == MACH_PORT_NULL) {
     CHROMIUM_LOG(ERROR) << "parent GetTranslatedPort(3) failed.";
   }
   auto* parent_send_port_memory = new MachPortSender(child_message.GetTranslatedPort(3));
 
   MachSendMessage parent_message(/* id= */0);
   if (!parent_message.AddDescriptor(MachMsgPortDescriptor(bootstrap_port))) {
     CHROMIUM_LOG(ERROR) << "parent AddDescriptor(" << bootstrap_port << ") failed.";
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(6 == 7);
-#endif
     return false;
   }
 
   auto* parent_recv_port_memory = new ReceivePort();
   if (!parent_message.AddDescriptor(MachMsgPortDescriptor(parent_recv_port_memory->GetPort()))) {
     CHROMIUM_LOG(ERROR) << "parent AddDescriptor(" << parent_recv_port_memory->GetPort() << ") failed.";
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(7 == 8);
-#endif
     return false;
   }
 
   auto* parent_send_port_memory_ack = new ReceivePort();
   if (!parent_message.AddDescriptor(MachMsgPortDescriptor(parent_send_port_memory_ack->GetPort()))) {
     CHROMIUM_LOG(ERROR) << "parent AddDescriptor(" << parent_send_port_memory_ack->GetPort() << ") failed.";
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(8 == 9);
-#endif
     return false;
   }
 
   err = parent_sender.SendMessage(parent_message, kTimeoutMs);
   if (err != KERN_SUCCESS) {
     std::string errString = StringPrintf("0x%x %s", err, mach_error_string(err));
     CHROMIUM_LOG(ERROR) << "parent SendMessage() failed: " << errString;
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(9 == 10);
-#endif
     return false;
   }
 
   SharedMemoryBasic::SetupMachMemory(process, parent_recv_port_memory, parent_recv_port_memory_ack,
                                      parent_send_port_memory, parent_send_port_memory_ack, false);
 
 # endif // MOZ_WIDGET_COCOA
 
@@ -975,32 +948,26 @@ GeckoChildProcessHost::PerformAsyncLaunc
       }
 #  endif // defined(MOZ_CONTENT_SANDBOX)
       break;
     case GeckoProcessType_Plugin:
       if (mSandboxLevel > 0 &&
           !PR_GetEnv("MOZ_DISABLE_NPAPI_SANDBOX")) {
         bool ok = mSandboxBroker.SetSecurityLevelForPluginProcess(mSandboxLevel);
         if (!ok) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-          MOZ_RELEASE_ASSERT(10 == 11);
-#endif
           return false;
         }
         shouldSandboxCurrentProcess = true;
       }
       break;
 #ifdef MOZ_ENABLE_SKIA_PDF
     case GeckoProcessType_PDFium:
       if (!PR_GetEnv("MOZ_DISABLE_PDFIUM_SANDBOX")) {
         bool ok = mSandboxBroker.SetSecurityLevelForPDFiumProcess();
         if (!ok) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-          MOZ_RELEASE_ASSERT(11 == 12);
-#endif
           return false;
         }
         shouldSandboxCurrentProcess = true;
       }
       break;
 #endif
     case GeckoProcessType_IPDLUnitTest:
       // XXX: We don't sandbox this process type yet
@@ -1011,19 +978,16 @@ GeckoChildProcessHost::PerformAsyncLaunc
         // not at USER_LOCKDOWN. So look in the command line arguments
         // to see if we're loading the path to the Widevine CDM, and if
         // so use sandbox level USER_RESTRICTED instead of USER_LOCKDOWN.
         bool isWidevine = std::any_of(aExtraOpts.begin(), aExtraOpts.end(),
           [](const std::string arg) { return arg.find("gmp-widevinecdm") != std::string::npos; });
         auto level = isWidevine ? SandboxBroker::Restricted : SandboxBroker::LockDown;
         bool ok = mSandboxBroker.SetSecurityLevelForGMPlugin(level);
         if (!ok) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-          MOZ_RELEASE_ASSERT(12 == 13);
-#endif
           return false;
         }
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_GPU:
       if (mSandboxLevel > 0 && !PR_GetEnv("MOZ_DISABLE_GPU_SANDBOX")) {
         // For now we treat every failure as fatal in SetSecurityLevelForGPUProcess
@@ -1123,19 +1087,16 @@ GeckoChildProcessHost::PerformAsyncLaunc
 # endif // MOZ_SANDBOX
   }
 
 #else // goes with defined(OS_POSIX)
 #  error Sorry
 #endif // defined(OS_POSIX)
 
   if (!process) {
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    MOZ_RELEASE_ASSERT(13 == 14);
-#endif
     return false;
   }
   // NB: on OS X, we block much longer than we need to in order to
   // reach this call, waiting for the child process's task_t.  The
   // best way to fix that is to refactor this file, hard.
 #if defined(MOZ_WIDGET_COCOA)
   mChildTask = child_task;
 #endif // defined(MOZ_WIDGET_COCOA)