Block signals while forking so they aren't run in the child process.

base/process/launch_posix.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/process/launch.h"
 
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <signal.h>
@@ skipping 59 matching lines @@
 // Set the process's "environment" (i.e. the thing that setenv/getenv
 // work with).
 void SetEnvironment(char** env) {
 #if defined(OS_MACOSX)
   *_NSGetEnviron() = env;
 #else
   environ = env;
 #endif
 }
 
+// List of signals that we want to block while forking and then reset
+// to SIG_DFL in the child process.
+// The previous signal handlers are likely to be meaningless in the child's
+// context so we reset them to the defaults for now. http://crbug.com/44953
+// These signal handlers are set up at least in browser_main_posix.cc:
+// BrowserMainPartsPosix::PreEarlyInitialization and stack_trace_posix.cc:
+// EnableInProcessStackDumping.
+const int kSignalsToReset[] = {

[jln (very slow on Chromium), 2013/07/31 23:54:55]

This ignores the other implementation of ResetChildSignalHandlersToDefaults()
(where all signals get reset).

Maybe the solution is to write a function that iterates through all signal
numbers and blocks any signal with a custom signal handler and returns a mask
that will be restored in the parent after fork().

[mdempsky_google, 2013/08/01 00:31:57]

Uploaded a new patch that's slightly simpler and just blocks all signals while
fork()ing.  This seems okay to me since fork()ing isn't that common (I think?),
so it shouldn't cause any problems if other signals get delayed until after the
fork() completes.

(I also realized I need to restore the original sigmask in the child process
after calling ResetChildSignalHandlersToDefaults.)

+  SIGHUP,
+  SIGINT,
+  SIGILL,
+  SIGABRT,
+  SIGFPE,
+  SIGBUS,
+  SIGSEGV,
+  SIGSYS,
+  SIGTERM,
+};
+
 #if !defined(OS_LINUX) || \
     (!defined(__i386__) && !defined(__x86_64__) && !defined(__arm__))
 void ResetChildSignalHandlersToDefaults() {
-  // The previous signal handlers are likely to be meaningless in the child's
-  // context so we reset them to the defaults for now. http://crbug.com/44953
-  // These signal handlers are set up at least in browser_main_posix.cc:
-  // BrowserMainPartsPosix::PreEarlyInitialization and stack_trace_posix.cc:
-  // EnableInProcessStackDumping.
-  signal(SIGHUP, SIG_DFL);
-  signal(SIGINT, SIG_DFL);
-  signal(SIGILL, SIG_DFL);
-  signal(SIGABRT, SIG_DFL);
-  signal(SIGFPE, SIG_DFL);
-  signal(SIGBUS, SIG_DFL);
-  signal(SIGSEGV, SIG_DFL);
-  signal(SIGSYS, SIG_DFL);
-  signal(SIGTERM, SIG_DFL);
+  for (size_t i = 0; i < arraysize(kSignalsToReset); i++) {
+    signal(kSignalsToReset[i], SIG_DFL);
+  }
 }
 
 #else
 
 // TODO(jln): remove the Linux special case once kernels are fixed.
 
 // Internally the kernel makes sigset_t an array of long large enough to have
 // one bit per signal.
 typedef uint64_t kernel_sigset_t;
 
@@ skipping 280 matching lines @@
   InjectiveMultimap fd_shuffle1;
   InjectiveMultimap fd_shuffle2;
   fd_shuffle1.reserve(fd_shuffle_size);
   fd_shuffle2.reserve(fd_shuffle_size);
 
   scoped_ptr<char*[]> argv_cstr(new char*[argv.size() + 1]);
   scoped_ptr<char*[]> new_environ;
   if (options.environ)
     new_environ.reset(AlterEnvironment(*options.environ, GetEnvironment()));
 
+  sigset_t sigs_to_block;
+  sigemptyset(&sigs_to_block);
+  for (size_t i = 0; i < arraysize(kSignalsToReset); i++) {
+    sigaddset(&sigs_to_block, kSignalsToReset[i]);
+  }
+
+  sigset_t old_sigset;
+  if (pthread_sigmask(SIG_BLOCK, &sigs_to_block, &old_sigset) != 0) {
+    DPLOG(ERROR) << "pthread_sigmask";
+    return false;
+  }
+
   pid_t pid;
 #if defined(OS_LINUX)
   if (options.clone_flags) {
     pid = syscall(__NR_clone, options.clone_flags, 0, 0, 0);
   } else
 #endif
   {
     pid = fork();
   }
 
+  if (pid != 0) {
+    if (pthread_sigmask(SIG_SETMASK, &old_sigset, NULL) != 0) {
+      NOTREACHED();
+    }
+  }
+
   if (pid < 0) {
     DPLOG(ERROR) << "fork";
     return false;
   } else if (pid == 0) {
     // Child process
 
     // DANGER: fork() rule: in the child, if you don't end up doing exec*(),
     // you call _exit() instead of exit(). This is because _exit() does not
     // call any previously-registered (in the parent) exit handlers, which
     // might do things like block waiting for threads that don't even exist
@@ skipping 303 matching lines @@
                               std::string* output,
                               int* exit_code) {
   // Run |execve()| with the current environment and store "unlimited" data.
   GetAppOutputInternalResult result = GetAppOutputInternal(
       cl.argv(), NULL, output, std::numeric_limits<std::size_t>::max(), true,
       exit_code);
   return result == EXECUTE_SUCCESS;
 }
 
 }  // namespace base