Update TestLauncher to use Fuchsia jobs in place of POSIX process jobs.
TestLauncher was previously using the |LaunchOptions::new_process_group|
to request LaunchProcess() to isolate each test job into a separate
group, for easy process cleanup.
Since the |new_process_group| was not implemented in the Fuchsia
implementation of LaunchProcess, this had no effect besides causing
errors to be logged when we attempted to kill(-pid).
We remove |new_process_group| and update TestLauncher to use native
Fuchsia jobs to group test job processes.
Bug: 799268, 755282
Change-Id: Ia96cd77c5b4066d6da522cc7fe0e4e427229dac3
Reviewed-on: https://chromium-review.googlesource.com/852559
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527925}
CrOS-Libchrome-Original-Commit: dc9eb2b1266e19bbfd4b9c69ff288750fef4004c
diff --git a/base/process/launch.h b/base/process/launch.h
index 681de35..a6ed7fb 100644
--- a/base/process/launch.h
+++ b/base/process/launch.h
@@ -165,16 +165,6 @@
// propagate FDs into the child process.
FileHandleMappingVector fds_to_remap;
- // Each element is an RLIMIT_* constant that should be raised to its
- // rlim_max. This pointer is owned by the caller and must live through
- // the call to LaunchProcess().
- const std::vector<int>* maximize_rlimits = nullptr;
-
- // If true, start the process in a new process group, instead of
- // inheriting the parent's process group. The pgid of the child process
- // will be the same as its pid.
- bool new_process_group = false;
-
#if defined(OS_LINUX)
// If non-zero, start the process using clone(), using flags as provided.
// Unlike in clone, clone_flags may not contain a custom termination signal
@@ -223,6 +213,16 @@
// code running in this delegate essentially needs to be async-signal safe
// (see man 7 signal for a list of allowed functions).
PreExecDelegate* pre_exec_delegate = nullptr;
+
+ // Each element is an RLIMIT_* constant that should be raised to its
+ // rlim_max. This pointer is owned by the caller and must live through
+ // the call to LaunchProcess().
+ const std::vector<int>* maximize_rlimits = nullptr;
+
+ // If true, start the process in a new process group, instead of
+ // inheriting the parent's process group. The pgid of the child process
+ // will be the same as its pid.
+ bool new_process_group = false;
#endif // defined(OS_POSIX)
#if defined(OS_CHROMEOS)
diff --git a/base/test/launcher/test_launcher.cc b/base/test/launcher/test_launcher.cc
index ed29463..c05d214 100644
--- a/base/test/launcher/test_launcher.cc
+++ b/base/test/launcher/test_launcher.cc
@@ -69,6 +69,7 @@
// TODO(scottmg): For temporary code in OnOutputTimeout().
#include <zircon/syscalls.h>
#include <zircon/syscalls/object.h>
+#include "base/fuchsia/default_job.h"
#endif
namespace base {
@@ -261,7 +262,7 @@
const bool kOnBot = getenv("CHROME_HEADLESS") != nullptr;
#endif // OS_FUCHSIA
-#if defined(OS_POSIX)
+#if defined(OS_POSIX) && !defined(OS_FUCHSIA)
// Make sure an option we rely on is present - see LaunchChildGTestProcess.
DCHECK(options.new_process_group);
#endif
@@ -295,7 +296,14 @@
new_options.job_handle = job_handle.Get();
}
-#endif // defined(OS_WIN)
+#elif defined(OS_FUCHSIA)
+ DCHECK(!new_options.job_handle);
+
+ ScopedZxHandle job_handle;
+ zx_status_t result = zx_job_create(GetDefaultJob(), 0, job_handle.receive());
+ CHECK_EQ(ZX_OK, result) << "zx_job_create: " << zx_status_get_string(result);
+ new_options.job_handle = job_handle.get();
+#endif // defined(OS_FUCHSIA)
#if defined(OS_LINUX)
// To prevent accidental privilege sharing to an untrusted child, processes
@@ -335,13 +343,14 @@
if (!process.IsValid())
return -1;
- // TODO(rvargas) crbug.com/417532: Don't store process handles.
#if defined(OS_FUCHSIA) // TODO(scottmg): https://crbug.com/755282
if (kOnBot) {
LOG(ERROR) << base::StringPrintf("adding %x to live process list",
process.Handle());
}
#endif // OS_FUCHSIA
+
+ // TODO(rvargas) crbug.com/417532: Don't store process handles.
GetLiveProcesses()->insert(std::make_pair(process.Handle(), command_line));
}
@@ -372,18 +381,20 @@
// to do that twice and trigger all kinds of log messages.
AutoLock lock(*GetLiveProcessesLock());
-#if defined(OS_POSIX)
+#if defined(OS_FUCHSIA)
+ // TODO(scottmg): https://crbug.com/755282
+ if (kOnBot) {
+ LOG(ERROR) << base::StringPrintf("going to zx_task_kill(job) for %x",
+ process.Handle());
+ }
+
+ CHECK_EQ(zx_task_kill(job_handle.get()), ZX_OK);
+#elif defined(OS_POSIX)
if (exit_code != 0) {
// On POSIX, in case the test does not exit cleanly, either due to a crash
// or due to it timing out, we need to clean up any child processes that
// it might have created. On Windows, child processes are automatically
// cleaned up using JobObjects.
-#if defined(OS_FUCHSIA) // TODO(scottmg): https://crbug.com/755282
- if (kOnBot) {
- LOG(ERROR) << base::StringPrintf("going to KillProcessGroup() for %x",
- process.Handle());
- }
-#endif // OS_FUCHSIA
KillProcessGroup(process.Handle());
}
#endif
@@ -439,7 +450,6 @@
}
}
#elif defined(OS_POSIX)
- options.new_process_group = true;
options.fds_to_remap = test_launch_options.fds_to_remap;
if (redirect_stdio) {
int output_file_fd = fileno(output_file.get());
@@ -450,6 +460,9 @@
std::make_pair(output_file_fd, STDERR_FILENO));
}
+#if !defined(OS_FUCHSIA)
+ options.new_process_group = true;
+#endif
#if defined(OS_LINUX)
options.kill_on_parent_death = true;
#endif