summaryrefslogtreecommitdiffstats
path: root/base
diff options
context:
space:
mode:
authormark@chromium.org <mark@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-09-01 21:44:24 +0000
committermark@chromium.org <mark@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-09-01 21:44:24 +0000
commit84b2d1d81b8c1c39c3cadaf9771e2c78cc0c51c4 (patch)
treee4086b02a50d7cd8b4a149fc54f962f8d6d3f30c /base
parent5efd22803abd20ef4230f5e95443e9fabe06c5d4 (diff)
downloadchromium_src-84b2d1d81b8c1c39c3cadaf9771e2c78cc0c51c4.zip
chromium_src-84b2d1d81b8c1c39c3cadaf9771e2c78cc0c51c4.tar.gz
chromium_src-84b2d1d81b8c1c39c3cadaf9771e2c78cc0c51c4.tar.bz2
Synchronize the parent and child processes during launch, ensuring that the
child doesn't try to check in with the Mach broker before the parent is ready. This resolves a race between the browser and its child processes. base::LaunchOptions gets a new member, synchronize, currently Mac-only although there's nothing Mac-specific about the implementation. When set, it allows base::LaunchProcess clients to request that the child process block until released by the new base::LaunchSynchronize call. When creating child processes, the child process launcher now requests this new synchronization behavior to give it a chance to set up the Mach broker to expect the child before the child tries to check in with it. BUG=94295 TEST=Chrome should work. You see fewer of this logged: Unknown process (pid) is sending Mach IPC messages! But you still might see some of these due to bug 94543. Review URL: http://codereview.chromium.org/7740036 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@99245 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base')
-rw-r--r--base/process_util.h58
-rw-r--r--base/process_util_posix.cc125
-rw-r--r--base/process_util_unittest.cc4
3 files changed, 169 insertions, 18 deletions
diff --git a/base/process_util.h b/base/process_util.h
index 66c63dd..34e9e91 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -200,6 +200,13 @@ BASE_EXPORT void CloseSuperfluousFds(const InjectiveMultimap& saved_map);
typedef std::vector<std::pair<std::string, std::string> > environment_vector;
typedef std::vector<std::pair<int, int> > file_handle_mapping_vector;
+#if defined(OS_MACOSX)
+// Used with LaunchOptions::synchronize and LaunchSynchronize, a
+// LaunchSynchronizationHandle is an opaque value that LaunchProcess will
+// create and set, and that LaunchSynchronize will consume and destroy.
+typedef int* LaunchSynchronizationHandle;
+#endif // defined(OS_MACOSX)
+
// Options for launching a subprocess that are passed to LaunchProcess().
// The default constructor constructs the object with default options.
struct LaunchOptions {
@@ -208,8 +215,13 @@ struct LaunchOptions {
start_hidden(false), inherit_handles(false), as_user(NULL),
empty_desktop_name(false)
#else
- environ(NULL), fds_to_remap(NULL), new_process_group(false),
- clone_flags(0)
+ environ(NULL), fds_to_remap(NULL), new_process_group(false)
+#if defined(OS_LINUX)
+ , clone_flags(0)
+#endif // OS_LINUX
+#if defined(OS_MACOSX)
+ , synchronize(NULL)
+#endif // defined(OS_MACOSX)
#endif // !defined(OS_WIN)
{}
@@ -251,9 +263,33 @@ struct LaunchOptions {
// will be the same as its pid.
bool new_process_group;
+#if defined(OS_LINUX)
// If non-zero, start the process using clone(), using flags as provided.
int clone_flags;
-#endif // !defined(OS_WIN)
+#endif // defined(OS_LINUX)
+
+#if defined(OS_MACOSX)
+ // When non-NULL, a new LaunchSynchronizationHandle will be created and
+ // stored in *synchronize whenever LaunchProcess returns true in the parent
+ // process. The child process will have been created (with fork) but will
+ // be waiting (before exec) for the parent to call LaunchSynchronize with
+ // this handle. Only when LaunchSynchronize is called will the child be
+ // permitted to continue execution and call exec. LaunchSynchronize
+ // destroys the handle created by LaunchProcess.
+ //
+ // When synchronize is non-NULL, the parent must call LaunchSynchronize
+ // whenever LaunchProcess returns true. No exceptions.
+ //
+ // Synchronization is useful when the parent process needs to guarantee that
+ // it can take some action (such as recording the newly-forked child's
+ // process ID) before the child does something (such as using its process ID
+ // to communicate with its parent).
+ //
+ // |synchronize| and |wait| must not both be set simultaneously.
+ LaunchSynchronizationHandle* synchronize;
+#endif // defined(OS_MACOSX)
+
+#endif // !defined(OS_WIN)
};
// Launch a process via the command line |cmdline|.
@@ -265,8 +301,11 @@ struct LaunchOptions {
// Otherwise, the process handle will be implicitly closed.
//
// Unix-specific notes:
-// - Before launching, all FDs open in the parent process will be marked as
-// close-on-exec.
+// - All file descriptors open in the parent process will be closed in the
+// child process except for any preserved by options::fds_to_remap, and
+// stdin, stdout, and stderr. If not remapped by options::fds_to_remap,
+// stdin is reopened as /dev/null, and the child is allowed to inherit its
+// parent's stdout and stderr.
// - If the first argument on the command line does not contain a slash,
// PATH will be searched. (See man execvp.)
BASE_EXPORT bool LaunchProcess(const CommandLine& cmdline,
@@ -319,6 +358,15 @@ BASE_EXPORT bool LaunchProcess(const std::vector<std::string>& argv,
// The returned array is allocated using new[] and must be freed by the caller.
BASE_EXPORT char** AlterEnvironment(const environment_vector& changes,
const char* const* const env);
+
+#if defined(OS_MACOSX)
+
+// After a successful call to LaunchProcess with LaunchOptions::synchronize
+// set, the parent process must call LaunchSynchronize to allow the child
+// process to proceed, and to destroy the LaunchSynchronizationHandle.
+BASE_EXPORT void LaunchSynchronize(LaunchSynchronizationHandle handle);
+
+#endif // defined(OS_MACOSX)
#endif // defined(OS_POSIX)
// Executes the application specified by |cl| and wait for it to exit. Stores
diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 059b7d7..06fa779 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -531,32 +531,72 @@ char** AlterEnvironment(const environment_vector& changes,
bool LaunchProcess(const std::vector<std::string>& argv,
const LaunchOptions& options,
ProcessHandle* process_handle) {
- pid_t pid;
- InjectiveMultimap fd_shuffle1, fd_shuffle2;
+ size_t fd_shuffle_size = 0;
if (options.fds_to_remap) {
- fd_shuffle1.reserve(options.fds_to_remap->size());
- fd_shuffle2.reserve(options.fds_to_remap->size());
+ fd_shuffle_size = options.fds_to_remap->size();
}
+
+#if defined(OS_MACOSX)
+ if (options.synchronize) {
+ // When synchronizing, the "read" end of the synchronization pipe needs
+ // to make it to the child process. This is handled by mapping it back to
+ // itself.
+ ++fd_shuffle_size;
+ }
+#endif // defined(OS_MACOSX)
+
+ InjectiveMultimap fd_shuffle1;
+ InjectiveMultimap fd_shuffle2;
+ fd_shuffle1.reserve(fd_shuffle_size);
+ fd_shuffle2.reserve(fd_shuffle_size);
+
scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
scoped_array<char*> new_environ;
if (options.environ)
new_environ.reset(AlterEnvironment(*options.environ, GetEnvironment()));
- if (options.clone_flags) {
+#if defined(OS_MACOSX)
+ int synchronization_pipe_fds[2];
+ file_util::ScopedFD synchronization_read_fd;
+ file_util::ScopedFD synchronization_write_fd;
+
+ if (options.synchronize) {
+ // wait means "don't return from LaunchProcess until the child exits", and
+ // synchronize means "return from LaunchProcess but don't let the child
+ // run until LaunchSynchronize is called". These two options are highly
+ // incompatible.
+ CHECK(!options.wait);
+
+ // Create the pipe used for synchronization.
+ if (HANDLE_EINTR(pipe(synchronization_pipe_fds)) != 0) {
+ PLOG(ERROR) << "pipe";
+ return false;
+ }
+
+ // The parent process will only use synchronization_write_fd as the write
+ // side of the pipe. It can close the read side as soon as the child
+ // process has forked off. The child process will only use
+ // synchronization_read_fd as the read side of the pipe. In that process,
+ // the write side can be closed as soon as it has forked.
+ synchronization_read_fd.reset(&synchronization_pipe_fds[0]);
+ synchronization_write_fd.reset(&synchronization_pipe_fds[1]);
+ }
+#endif // OS_MACOSX
+
+ pid_t pid;
#if defined(OS_LINUX)
+ if (options.clone_flags) {
pid = syscall(__NR_clone, options.clone_flags, 0, 0, 0);
-#else
- pid = -1; // hygiene; prevent clang warnings
- NOTREACHED() << "Tried to use clone() on non-Linux system";
+ } else
#endif
- } else {
+ {
pid = fork();
}
+
if (pid < 0) {
PLOG(ERROR) << "fork";
return false;
- }
- if (pid == 0) {
+ } else if (pid == 0) {
// Child process
// DANGER: fork() rule: in the child, if you don't end up doing exec*(),
@@ -589,12 +629,20 @@ bool LaunchProcess(const std::vector<std::string>& argv,
_exit(127);
}
}
+
#if defined(OS_MACOSX)
RestoreDefaultExceptionHandler();
-#endif
+#endif // defined(OS_MACOSX)
ResetChildSignalHandlersToDefaults();
+#if defined(OS_MACOSX)
+ if (options.synchronize) {
+ // The "write" side of the synchronization pipe belongs to the parent.
+ synchronization_write_fd.reset(); // closes synchronization_pipe_fds[1]
+ }
+#endif // defined(OS_MACOSX)
+
#if 0
// When debugging it can be helpful to check that we really aren't making
// any hidden calls to malloc.
@@ -602,7 +650,7 @@ bool LaunchProcess(const std::vector<std::string>& argv,
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
+#endif // 0
// DANGER: no calls to malloc are allowed from now on:
// http://crbug.com/36678
@@ -616,6 +664,16 @@ bool LaunchProcess(const std::vector<std::string>& argv,
}
}
+#if defined(OS_MACOSX)
+ if (options.synchronize) {
+ // Remap the read side of the synchronization pipe back onto itself,
+ // ensuring that it won't be closed by CloseSuperfluousFds.
+ int keep_fd = *synchronization_read_fd.get();
+ fd_shuffle1.push_back(InjectionArc(keep_fd, keep_fd, false));
+ fd_shuffle2.push_back(InjectionArc(keep_fd, keep_fd, false));
+ }
+#endif // defined(OS_MACOSX)
+
if (options.environ)
SetEnvironment(new_environ.get());
@@ -625,10 +683,29 @@ bool LaunchProcess(const std::vector<std::string>& argv,
CloseSuperfluousFds(fd_shuffle2);
+#if defined(OS_MACOSX)
+ if (options.synchronize) {
+ // Do a blocking read to wait until the parent says it's OK to proceed.
+ // The byte that's read here is written by LaunchSynchronize.
+ char read_char;
+ int read_result =
+ HANDLE_EINTR(read(*synchronization_read_fd.get(), &read_char, 1));
+ if (read_result != 1) {
+ RAW_LOG(ERROR, "LaunchProcess: synchronization read: error");
+ _exit(127);
+ }
+
+ // The pipe is no longer useful. Don't let it live on in the new process
+ // after exec.
+ synchronization_read_fd.reset(); // closes synchronization_pipe_fds[0]
+ }
+#endif // defined(OS_MACOSX)
+
for (size_t i = 0; i < argv.size(); i++)
argv_cstr[i] = const_cast<char*>(argv[i].c_str());
argv_cstr[argv.size()] = NULL;
execvp(argv_cstr[0], argv_cstr.get());
+
RAW_LOG(ERROR, "LaunchProcess: failed to execvp:");
RAW_LOG(ERROR, argv_cstr[0]);
_exit(127);
@@ -644,17 +721,39 @@ bool LaunchProcess(const std::vector<std::string>& argv,
if (process_handle)
*process_handle = pid;
+
+#if defined(OS_MACOSX)
+ if (options.synchronize) {
+ // The "read" side of the synchronization pipe belongs to the child.
+ synchronization_read_fd.reset(); // closes synchronization_pipe_fds[0]
+ *options.synchronize = new int(*synchronization_write_fd.release());
+ }
+#endif // defined(OS_MACOSX)
}
return true;
}
+
bool LaunchProcess(const CommandLine& cmdline,
const LaunchOptions& options,
ProcessHandle* process_handle) {
return LaunchProcess(cmdline.argv(), options, process_handle);
}
+#if defined(OS_MACOSX)
+void LaunchSynchronize(LaunchSynchronizationHandle handle) {
+ int synchronization_fd = *handle;
+ file_util::ScopedFD synchronization_fd_closer(&synchronization_fd);
+ delete handle;
+
+ // Write a '\0' character to the pipe.
+ if (HANDLE_EINTR(write(synchronization_fd, "", 1)) != 1) {
+ PLOG(ERROR) << "write";
+ }
+}
+#endif // defined(OS_MACOSX)
+
ProcessMetrics::~ProcessMetrics() { }
bool EnableInProcessStackDumping() {
diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc
index bcc1308..0403d7a 100644
--- a/base/process_util_unittest.cc
+++ b/base/process_util_unittest.cc
@@ -538,7 +538,11 @@ std::string TestLaunchProcess(const base::environment_vector& env_changes,
options.wait = true;
options.environ = &env_changes;
options.fds_to_remap = &fds_to_remap;
+#if defined(OS_LINUX)
options.clone_flags = clone_flags;
+#else
+ CHECK_EQ(0, clone_flags);
+#endif // OS_LINUX
EXPECT_TRUE(base::LaunchProcess(args, options, NULL));
PCHECK(HANDLE_EINTR(close(fds[1])) == 0);