Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(768)

Unified Diff: base/process_util_mac.mm

Issue 14497: POSIX: don't leak FDs when launching child Processes. (Closed)
Patch Set: Fix Windows Compilation Created 12 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/process_util_linux.cc ('k') | base/process_util_posix.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/process_util_mac.mm
diff --git a/base/process_util_mac.mm b/base/process_util_mac.mm
index c29b488774d0fd04540a34d56d3cf74dcc87f5e5..330e8b298729f5295c28786e0a94f54117c1aef7 100644
--- a/base/process_util_mac.mm
+++ b/base/process_util_mac.mm
@@ -19,41 +19,81 @@ extern char** environ;
namespace base {
bool LaunchApp(const std::vector<std::string>& argv,
+ const file_handle_mapping_vector& fds_to_remap,
bool wait, ProcessHandle* process_handle) {
bool retval = true;
-
+
char* argv_copy[argv.size() + 1];
for (size_t i = 0; i < argv.size(); i++) {
argv_copy[i] = const_cast<char*>(argv[i].c_str());
}
argv_copy[argv.size()] = NULL;
-
+
+ // Make sure we don't leak any FDs to the child process by marking all FDs
+ // as close-on-exec.
+ int max_files = GetMaxFilesOpenInProcess();
+ for (int i = STDERR_FILENO + 1; i < max_files; i++) {
+ int flags = fcntl(i, F_GETFD);
+ if (flags != -1) {
+ fcntl(i, F_SETFD, flags | FD_CLOEXEC);
+ }
+ }
+
+ posix_spawn_file_actions_t file_actions;
+ if (posix_spawn_file_actions_init(&file_actions) != 0) {
+ return false;
+ }
+
+ // Turn fds_to_remap array into a set of dup2 calls.
+ for (file_handle_mapping_vector::const_iterator it = fds_to_remap.begin();
+ it != fds_to_remap.end();
+ ++it) {
+ int src_fd = it->first;
+ int dest_fd = it->second;
+
+ 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)
+ {
+ posix_spawn_file_actions_destroy(&file_actions);
+ return false;
+ }
+ }
+ }
+
int pid = 0;
- int spawn_succeeded = (posix_spawnp(&pid,
- argv_copy[0],
- NULL,
- NULL,
- argv_copy,
+ int spawn_succeeded = (posix_spawnp(&pid,
+ argv_copy[0],
+ &file_actions,
+ NULL,
+ argv_copy,
environ) == 0);
-
- bool process_handle_valid = pid > 0;
+
+ posix_spawn_file_actions_destroy(&file_actions);
+
+ bool process_handle_valid = pid > 0;
if (!spawn_succeeded || !process_handle_valid) {
retval = false;
} else {
if (wait)
waitpid(pid, 0, 0);
-
+
if(process_handle)
*process_handle = pid;
}
-
+
return retval;
}
bool LaunchApp(const CommandLine& cl,
bool wait, bool start_hidden, ProcessHandle* process_handle) {
// TODO(playmobil): Do we need to respect the start_hidden flag?
- return LaunchApp(cl.argv(), wait, process_handle);
+ file_handle_mapping_vector no_files;
+ return LaunchApp(cl.argv(), no_files, wait, process_handle);
}
bool ProcessMetrics::GetIOCounters(IoCounters* io_counters) {
« no previous file with comments | « base/process_util_linux.cc ('k') | base/process_util_posix.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698