Commit 89fbe66c authored by Yuke Liao's avatar Yuke Liao Committed by Commit Bot

Make test launcher leak proof in clang profiling build

When test passes, there is no guarantee that the test itself will clean
up all created child processes, so to make the test launcher leak proof,
issues a group kill. This only applies to clang profiling build because
those lingering processes usually indicate prod issues, so we don't
want them to be masked in regular build.

Bug: 1073832
Change-Id: I92ce55d9e5122e4aae18d4c4df46a378fe57159c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2241641Reviewed-by: default avatarErik Staab <estaab@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@google.com>
Reviewed-by: default avatarIlia Samsonov <isamsonov@google.com>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#778535}
parent ce7cd7c9
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/at_exit.h" #include "base/at_exit.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/clang_profiling_buildflags.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/environment.h" #include "base/environment.h"
#include "base/files/file_enumerator.h" #include "base/files/file_enumerator.h"
...@@ -437,6 +438,29 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line, ...@@ -437,6 +438,29 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
// Cleanup the data directory. // Cleanup the data directory.
CHECK(DeleteFileRecursively(nested_data_path)); CHECK(DeleteFileRecursively(nested_data_path));
#elif defined(OS_POSIX) #elif defined(OS_POSIX)
#if BUILDFLAG(CLANG_PROFILING)
// TODO(crbug.com/1094369): Remove this condition once the child process
// leaking bug is fixed.
//
// TODO(crbug.com/1095075): Make test launcher treat lingering child
// processes hard failures so that they can be detected and surfaced
// gracefully.
//
// When profiling is enabled, browser child processes take extra time to
// dump profiles, which means that lingering processes are much more likely
// to happen than non-profiling build. Therefore, on POSIX, in order to
// avoid polluting the machine state, ensure any child processes that the
// test might have created are cleaned up to avoid potential leaking even
// when tests have passed. On Windows, child processes are automatically
// cleaned up using JobObjects.
//
// On non-profiling build, when tests have passed, we don't clean up the
// lingering processes even when there are any, and the reason is that they
// usually indicate prod issues, letting them slip to the following test
// tasks and cause failures increses the chance of them being surfaced.
kill(-1 * process.Handle(), SIGKILL);
#else
if (exit_code != 0) { if (exit_code != 0) {
// On POSIX, in case the test does not exit cleanly, either due to a crash // 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 // or due to it timing out, we need to clean up any child processes that
...@@ -444,6 +468,7 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line, ...@@ -444,6 +468,7 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
// cleaned up using JobObjects. // cleaned up using JobObjects.
KillProcessGroup(process.Handle()); KillProcessGroup(process.Handle());
} }
#endif
#endif #endif
GetLiveProcesses()->erase(process.Handle()); GetLiveProcesses()->erase(process.Handle());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment