posix LaunchProcess: Don't use STL iterators after fork.

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 189 matching lines @@
 static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_FREEBSD)
 static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_OPENBSD)
 static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_ANDROID)
 static const char kFDDir[] = "/proc/self/fd";
 #endif
 
 void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
-  // DANGER: no calls to malloc are allowed from now on:
+  // DANGER: no calls to malloc or locks are allowed from now on:
   // http://crbug.com/36678
 
   // Get the maximum number of FDs possible.
   size_t max_fds = GetMaxFds();
 
   DirReaderPosix fd_dir(kFDDir);
   if (!fd_dir.IsValid()) {
     // Fallback case: Try every possible fd.
     for (size_t i = 0; i < max_fds; ++i) {
       const int fd = static_cast<int>(i);
       if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
         continue;
-      InjectiveMultimap::const_iterator j;
-      for (j = saved_mapping.begin(); j != saved_mapping.end(); j++) {
-        if (fd == j->dest)
+      // Cannot use STL iterators here, since debug iterators use locks.
+      size_t j;
+      for (j = 0; j < saved_mapping.size(); j++) {
+        if (fd == saved_mapping[j].dest)
           break;
       }
-      if (j != saved_mapping.end())
+      if (j < saved_mapping.size())
         continue;
 
       // Since we're just trying to close anything we can find,
       // ignore any error return values of close().
       close(fd);
     }
     return;
   }
 
   const int dir_fd = fd_dir.fd();
 
   for ( ; fd_dir.Next(); ) {
     // Skip . and .. entries.
     if (fd_dir.name()[0] == '.')
       continue;
 
     char *endptr;
     errno = 0;
     const long int fd = strtol(fd_dir.name(), &endptr, 10);
     if (fd_dir.name()[0] == 0 || *endptr || fd < 0 || errno)
       continue;
     if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
       continue;
-    InjectiveMultimap::const_iterator i;
-    for (i = saved_mapping.begin(); i != saved_mapping.end(); i++) {
-      if (fd == i->dest)
+    // Cannot use STL iterators here, since debug iterators use locks.
+    size_t i;
+    for (i = 0; i < saved_mapping.size(); i++) {
+      if (fd == saved_mapping[i].dest)
         break;
     }
-    if (i != saved_mapping.end())
+    if (i < saved_mapping.size())
       continue;
     if (fd == dir_fd)
       continue;
 
     // When running under Valgrind, Valgrind opens several FDs for its
     // own use and will complain if we try to close them.  All of
     // these FDs are >= |max_fds|, so we can check against that here
     // before closing.  See https://bugs.kde.org/show_bug.cgi?id=191758
     if (fd < static_cast<int>(max_fds)) {
       int ret = IGNORE_EINTR(close(fd));
@@ skipping 113 matching lines @@
 
 #if 0
     // When debugging it can be helpful to check that we really aren't making
     // any hidden calls to malloc.
     void *malloc_thunk =
         reinterpret_cast<void*>(reinterpret_cast<intptr_t>(malloc) & ~4095);
     mprotect(malloc_thunk, 4096, PROT_READ | PROT_WRITE | PROT_EXEC);
     memset(reinterpret_cast<void*>(malloc), 0xff, 8);
 #endif  // 0
 
-    // DANGER: no calls to malloc are allowed from now on:
+    // DANGER: no calls to malloc or locks are allowed from now on:
     // http://crbug.com/36678
 
 #if defined(OS_CHROMEOS)
     if (options.ctrl_terminal_fd >= 0) {
       // Set process' controlling terminal.
       if (HANDLE_EINTR(setsid()) != -1) {
         if (HANDLE_EINTR(
                 ioctl(options.ctrl_terminal_fd, TIOCSCTTY, NULL)) == -1) {
           RAW_LOG(WARNING, "ioctl(TIOCSCTTY), ctrl terminal not set");
         }
       } else {
         RAW_LOG(WARNING, "setsid failed, ctrl terminal not set");
       }
     }
 #endif  // defined(OS_CHROMEOS)
 
     if (options.fds_to_remap) {
-      for (FileHandleMappingVector::const_iterator
-               it = options.fds_to_remap->begin();
-           it != options.fds_to_remap->end(); ++it) {
-        fd_shuffle1.push_back(InjectionArc(it->first, it->second, false));
-        fd_shuffle2.push_back(InjectionArc(it->first, it->second, false));
+      // Cannot use STL iterators here, since debug iterators use locks.
+      for (size_t i = 0; i < options.fds_to_remap->size(); ++i) {
+        const FileHandleMappingVector::value_type& value =
+            (*options.fds_to_remap)[i];
+        fd_shuffle1.push_back(InjectionArc(value.first, value.second, false));
+        fd_shuffle2.push_back(InjectionArc(value.first, value.second, false));
       }
     }
 
     if (!options.environ.empty())
       SetEnvironment(new_environ.get());
 
     // fd_shuffle1 is mutated by this call because it cannot malloc.
     if (!ShuffleFileDescriptors(&fd_shuffle1))
       _exit(127);
 
@@ skipping 90 matching lines @@
   switch (pid = fork()) {
     case -1:  // error
       close(pipe_fd[0]);
       close(pipe_fd[1]);
       return EXECUTE_FAILURE;
     case 0:  // child
       {
 #if defined(OS_MACOSX)
         RestoreDefaultExceptionHandler();
 #endif
-        // DANGER: no calls to malloc are allowed from now on:
+        // DANGER: no calls to malloc or locks are allowed from now on:
         // http://crbug.com/36678
 
         // 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.
         int dev_null = open("/dev/null", O_WRONLY);
         if (dev_null < 0)
           _exit(127);
 
         // Stop type-profiler.
         // The profiler should be stopped between fork and exec since it inserts
         // locks at new/delete expressions.  See http://crbug.com/36678.
         base::type_profiler::Controller::Stop();
 
         fd_shuffle1.push_back(InjectionArc(pipe_fd[1], STDOUT_FILENO, true));
         fd_shuffle1.push_back(InjectionArc(dev_null, STDERR_FILENO, true));
         fd_shuffle1.push_back(InjectionArc(dev_null, STDIN_FILENO, true));
         // Adding another element here? Remeber to increase the argument to
         // reserve(), above.
 
         std::copy(fd_shuffle1.begin(), fd_shuffle1.end(),

[Elliot Glaysher, 2014/01/07 22:33:14]

So maybe I'm wrong here, but isn't this using stl iterators in a block that says
above that it can't use STL iterators?

[mattm, 2014/01/07 22:43:40]

Oh, yeah. I missed this one and I must not have been using a test that hits
this. Good catch, I'll make a followup cl, thanks!

                   std::back_inserter(fd_shuffle2));
 
         if (!ShuffleFileDescriptors(&fd_shuffle1))
           _exit(127);
 
         CloseSuperfluousFds(fd_shuffle2);
 
         for (size_t i = 0; i < argv.size(); i++)
           argv_cstr[i] = const_cast<char*>(argv[i].c_str());
         argv_cstr[argv.size()] = NULL;
@@ skipping 71 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