Windows, test wrapper: use WaitableProcess
The Windows-native test wrapper now uses the JNI
library's WaitableProcess to create, wait for, and
terminate the subprocess.
This allows killing the whole subprocess tree and
not waiting for sub-subprocesses.
Fixes https://github.com/bazelbuild/bazel/issues/8005
Closes #8075.
PiperOrigin-RevId: 243984578
diff --git a/src/main/native/windows/process.cc b/src/main/native/windows/process.cc
index 82fec19..48e54bc 100644
--- a/src/main/native/windows/process.cc
+++ b/src/main/native/windows/process.cc
@@ -33,6 +33,7 @@
const std::wstring& argv_rest, void* env,
const std::wstring& wcwd, HANDLE stdin_process,
HANDLE stdout_process, HANDLE stderr_process,
+ LARGE_INTEGER* opt_out_start_time,
std::wstring* error) {
std::wstring cwd;
std::wstring error_msg(AsShortPath(wcwd, &cwd));
@@ -177,11 +178,26 @@
return false;
}
+ if (opt_out_start_time) {
+ QueryPerformanceCounter(opt_out_start_time);
+ }
*error = L"";
return true;
}
-int WaitableProcess::WaitFor(int64_t timeout_msec, std::wstring* error) {
+int WaitableProcess::WaitFor(int64_t timeout_msec,
+ LARGE_INTEGER* opt_out_end_time,
+ std::wstring* error) {
+ struct Defer {
+ LARGE_INTEGER* t;
+ Defer(LARGE_INTEGER* cnt) : t(cnt) {}
+ ~Defer() {
+ if (t) {
+ QueryPerformanceCounter(t);
+ }
+ }
+ } defer_query_end_time(opt_out_end_time);
+
DWORD win32_timeout = timeout_msec < 0 ? INFINITE : timeout_msec;
int result;
switch (WaitForSingleObject(process_, win32_timeout)) {
diff --git a/src/main/native/windows/process.h b/src/main/native/windows/process.h
index e750f46..de0e1e4 100644
--- a/src/main/native/windows/process.h
+++ b/src/main/native/windows/process.h
@@ -42,9 +42,10 @@
bool Create(const std::wstring& argv0, const std::wstring& argv_rest,
void* env, const std::wstring& wcwd, HANDLE stdin_process,
HANDLE stdout_process, HANDLE stderr_process,
- std::wstring* error);
+ LARGE_INTEGER* opt_out_start_time, std::wstring* error);
- int WaitFor(int64_t timeout_msec, std::wstring* error);
+ int WaitFor(int64_t timeout_msec, LARGE_INTEGER* opt_out_end_time,
+ std::wstring* error);
int GetExitCode(std::wstring* error);
diff --git a/src/main/native/windows/processes-jni.cc b/src/main/native/windows/processes-jni.cc
index e3bb480..f58397e 100644
--- a/src/main/native/windows/processes-jni.cc
+++ b/src/main/native/windows/processes-jni.cc
@@ -314,7 +314,7 @@
return proc_.Create(
wpath, bazel::windows::GetJavaWstring(env, java_argv_rest),
env_map.ptr(), bazel::windows::GetJavaWpath(env, java_cwd),
- stdin_process, stdout_process, stderr_process, &error_);
+ stdin_process, stdout_process, stderr_process, nullptr, &error_);
}
void CloseStdin() {
@@ -325,7 +325,7 @@
// Wait for this process to exit (or timeout).
int WaitFor(int64_t timeout_msec) {
- return proc_.WaitFor(timeout_msec, &error_);
+ return proc_.WaitFor(timeout_msec, nullptr, &error_);
}
// Returns the exit code of the process if it has already exited. If the
diff --git a/src/test/shell/bazel/testdata/embedded_tools_srcs_deps b/src/test/shell/bazel/testdata/embedded_tools_srcs_deps
index 4caa1c9..1f15403 100644
--- a/src/test/shell/bazel/testdata/embedded_tools_srcs_deps
+++ b/src/test/shell/bazel/testdata/embedded_tools_srcs_deps
@@ -21,6 +21,7 @@
//src/tools/launcher/util:util
//src/main/cpp/util:filesystem
//src/main/native/windows:lib-file
+//src/main/native/windows:lib-process
//src/main/native/windows:lib-util
//src/main/cpp/util:strings
//src/main/cpp/util:errors
diff --git a/tools/test/BUILD b/tools/test/BUILD
index 6deb7dd..d259336 100644
--- a/tools/test/BUILD
+++ b/tools/test/BUILD
@@ -79,6 +79,7 @@
"//src/main/cpp/util:filesystem",
"//src/main/cpp/util:strings",
"//src/main/native/windows:lib-file",
+ "//src/main/native/windows:lib-process",
"//src/main/native/windows:lib-util",
"//src/tools/launcher/util",
"//third_party/ijar:zip",
diff --git a/tools/test/windows/tw.cc b/tools/test/windows/tw.cc
index d65312b..57f3787 100644
--- a/tools/test/windows/tw.cc
+++ b/tools/test/windows/tw.cc
@@ -18,12 +18,14 @@
#include "tools/test/windows/tw.h"
+#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
-#include <lmcons.h> // UNLEN
+#endif
#include <windows.h>
#include <errno.h>
#include <limits.h> // INT_MAX
+#include <lmcons.h> // UNLEN
#include <string.h>
#include <sys/types.h>
#include <wchar.h>
@@ -42,6 +44,7 @@
#include "src/main/cpp/util/path_platform.h"
#include "src/main/cpp/util/strings.h"
#include "src/main/native/windows/file.h"
+#include "src/main/native/windows/process.h"
#include "src/main/native/windows/util.h"
#include "src/tools/launcher/util/launcher_util.h"
#include "third_party/ijar/common.h"
@@ -1098,7 +1101,7 @@
bool StartSubprocess(const Path& path, const std::wstring& args,
const Path& outerr, std::unique_ptr<Tee>* tee,
LARGE_INTEGER* start_time,
- bazel::windows::AutoHandle* process) {
+ bazel::windows::WaitableProcess* process) {
SECURITY_ATTRIBUTES inheritable_handle_sa = {sizeof(SECURITY_ATTRIBUTES),
NULL, TRUE};
@@ -1139,16 +1142,6 @@
return false;
}
- // Create an attribute object that specifies which particular handles shall
- // the subprocess inherit. We pass this object to CreateProcessW.
- std::unique_ptr<bazel::windows::AutoAttributeList> attr_list;
- std::wstring werror;
- if (!bazel::windows::AutoAttributeList::Create(
- devnull_read, pipe_write, pipe_write_dup, &attr_list, &werror)) {
- LogError(__LINE__, werror);
- return false;
- }
-
// Open a handle to the test log file. The "tee" thread will write everything
// into it that the subprocess writes to the pipe.
bazel::windows::AutoHandle test_outerr;
@@ -1177,57 +1170,13 @@
return false;
}
- PROCESS_INFORMATION process_info;
- STARTUPINFOEXW startup_info;
- attr_list->InitStartupInfoExW(&startup_info);
-
- std::unique_ptr<WCHAR[]> cmdline;
- if (!CreateCommandLine(path, args, &cmdline)) {
+ std::wstring werror;
+ if (!process->Create(path.Get(), args, nullptr, L"", devnull_read, pipe_write,
+ pipe_write_dup, start_time, &werror)) {
+ LogError(__LINE__, werror);
return false;
}
-
- QueryPerformanceCounter(start_time);
- if (CreateProcessW(NULL, cmdline.get(), NULL, NULL, TRUE,
- CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT,
- NULL, NULL, &startup_info.StartupInfo,
- &process_info) != 0) {
- CloseHandle(process_info.hThread);
- *process = process_info.hProcess;
- return true;
- } else {
- DWORD err = GetLastError();
- LogErrorWithValue(
- __LINE__,
- (std::wstring(L"CreateProcessW failed (") + cmdline.get() + L")")
- .c_str(),
- err);
- return false;
- }
-}
-
-int WaitForSubprocess(HANDLE process, LARGE_INTEGER* end_time) {
- DWORD result = WaitForSingleObject(process, INFINITE);
- QueryPerformanceCounter(end_time);
- switch (result) {
- case WAIT_OBJECT_0: {
- DWORD exit_code;
- if (!GetExitCodeProcess(process, &exit_code)) {
- DWORD err = GetLastError();
- LogErrorWithValue(__LINE__, "GetExitCodeProcess failed", err);
- return 1;
- }
- return exit_code;
- }
- case WAIT_FAILED: {
- DWORD err = GetLastError();
- LogErrorWithValue(__LINE__, "WaitForSingleObject failed", err);
- return 1;
- }
- default:
- LogErrorWithValue(
- __LINE__, "WaitForSingleObject returned unexpected result", result);
- return 1;
- }
+ return true;
}
bool ArchiveUndeclaredOutputs(const UndeclaredOutputs& undecl) {
@@ -1385,14 +1334,27 @@
int RunSubprocess(const Path& test_path, const std::wstring& args,
const Path& test_outerr, Duration* test_duration) {
std::unique_ptr<Tee> tee;
- bazel::windows::AutoHandle process;
+ bazel::windows::WaitableProcess process;
LARGE_INTEGER start, end, freq;
if (!StartSubprocess(test_path, args, test_outerr, &tee, &start, &process)) {
LogError(__LINE__, std::wstring(L"Failed to start test process \"") +
test_path.Get() + L"\"");
return 1;
}
- int result = WaitForSubprocess(process, &end);
+
+ std::wstring werror;
+ int wait_res = process.WaitFor(-1, &end, &werror);
+ if (wait_res != bazel::windows::WaitableProcess::kWaitSuccess) {
+ LogErrorWithValue(__LINE__, werror, wait_res);
+ return 1;
+ }
+
+ werror.clear();
+ int result = process.GetExitCode(&werror);
+ if (!werror.empty()) {
+ LogError(__LINE__, werror);
+ return 1;
+ }
QueryPerformanceFrequency(&freq);
end.QuadPart -= start.QuadPart;
