Bug 1475382 - Remove debugging crashes added in bug 1461459. r?spohl draft
authorJed Davis <jld@mozilla.com>
Fri, 13 Jul 2018 15:18:03 -0600
changeset 819077 dbc24fdab7767eb94d4518bfad68b0193221b248
parent 819019 f6df6a982ee9510ca32dd3afa52dfe9f8c3586a3
child 819078 dce6171c3fd4066f115f74f56a0f50772dbb2878
push id116432
push userbmo:jld@mozilla.com
push dateTue, 17 Jul 2018 04:26:12 +0000
reviewersspohl
bugs1475382, 1461459
milestone63.0a1
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)