GTTF: Fix a few cases of very likely broken code between fork() and exec()

I took another look at http://crbug.com/60426, and decided to also
review the LaunchApp code in process_util. It turns out that
the child code after fork() contained things like "return false",
rather not what was intended (I replaced it with _exit(127)).

There are some other cases where we possibly retained some
signal handlers. The new code also resets them to defaults.

I don't expect it to fix the referenced bug, but those
"obvious" problems can only make it worse.

BUG=60426
Review URL: http://codereview.chromium.org/6801047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80927 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 28 insertions, 21 deletions

diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 26301d2..1ea90c3 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -156,10 +156,17 @@ void StackDumpSignalHandler(int signal, siginfo_t* info, ucontext_t* context) {
 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 setup in browser_main.cc:BrowserMain
-  signal(SIGTERM, SIG_DFL);
+  // These signal handlers are set up at least in browser_main.cc:BrowserMain
+  // and process_util_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);
 }
 
 }  // anonymous namespace
@@ -296,7 +303,7 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
   if (getrlimit(RLIMIT_NOFILE, &nofile)) {
     // getrlimit failed. Take a best guess.
     max_fds = kSystemDefaultMaxFds;
-    DLOG(ERROR) << "getrlimit(RLIMIT_NOFILE) failed: " << errno;
+    RAW_LOG(ERROR, "getrlimit(RLIMIT_NOFILE) failed");
   } else {
     max_fds = nofile.rlim_cur;
   }
@@ -322,7 +329,7 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
 
       // Since we're just trying to close anything we can find,
       // ignore any error return values of close().
-      int unused ALLOW_UNUSED = HANDLE_EINTR(close(fd));
+      ignore_result(HANDLE_EINTR(close(fd)));
     }
     return;
   }
@@ -505,28 +512,34 @@ bool LaunchAppImpl(
   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
+    // in the child.
+
     // If a child process uses the readline library, the process block forever.
     // In BSD like OSes including OS X it is safe to assign /dev/null as stdin.
     // See http://crbug.com/56596.
     int null_fd = HANDLE_EINTR(open("/dev/null", O_RDONLY));
     if (null_fd < 0) {
-      PLOG(ERROR) << "Failed to open /dev/null";
-      return false;
-    } else {
-      file_util::ScopedFD null_fd_closer(&null_fd);
-      int new_fd = HANDLE_EINTR(dup2(null_fd, STDIN_FILENO));
-      if (new_fd != STDIN_FILENO) {
-        PLOG(ERROR) << "Failed to dup /dev/null for stdin";
-        return false;
-      }
+      RAW_LOG(ERROR, "Failed to open /dev/null");
+      _exit(127);
+    }
+
+    file_util::ScopedFD null_fd_closer(&null_fd);
+    int new_fd = HANDLE_EINTR(dup2(null_fd, STDIN_FILENO));
+    if (new_fd != STDIN_FILENO) {
+      RAW_LOG(ERROR, "Failed to dup /dev/null for stdin");
+      _exit(127);
     }
 
     if (start_new_process_group) {
       // Instead of inheriting the process group ID of the parent, the child
       // starts off a new process group with pgid equal to its process ID.
       if (setpgid(0, 0) < 0) {
-        PLOG(ERROR) << "setpgid";
-        return false;
+        RAW_LOG(ERROR, "setpgid failed");
+        _exit(127);
       }
     }
 #if defined(OS_MACOSX)
@@ -555,12 +568,6 @@ bool LaunchAppImpl(
 
     environ = new_environ.get();
 
-    // Obscure 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
-    // in the child.
-
     // fd_shuffle1 is mutated by this call because it cannot malloc.
     if (!ShuffleFileDescriptors(&fd_shuffle1))
       _exit(127);