Windows, JNI: use WaitableProcess in blaze_util
Replace custom process- and job-creation logic in
blaze::ExecuteProgram with the JNI library's
WaitableProcess.
Add a new overload of WaitableProcess::Create,
which allows connecting the caller's
stdin/stdout/stderr to the child process.
Closes #8113.
PiperOrigin-RevId: 245214607
diff --git a/src/main/cpp/BUILD b/src/main/cpp/BUILD
index 7135055..483df2c 100644
--- a/src/main/cpp/BUILD
+++ b/src/main/cpp/BUILD
@@ -61,7 +61,11 @@
"//src/main/cpp/util:blaze_exit_code",
"//src/main/cpp/util:logging",
] + select({
- "//src/conditions:windows": ["//src/main/native/windows:lib-file"],
+ "//src/conditions:windows": [
+ "//src/main/native/windows:lib-file",
+ "//src/main/native/windows:lib-process",
+ "//src/main/native/windows:lib-util",
+ ],
"//conditions:default": [],
}),
)
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index 04f7eab..fe13876 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -14,17 +14,19 @@
#include "src/main/cpp/blaze_util_platform.h"
-#include <fcntl.h>
-#include <stdarg.h> // va_start, va_end, va_list
-
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN
+#endif
#include <windows.h>
-#include <lmcons.h> // UNLEN
-#include <versionhelpers.h> // IsWindows8OrGreater
-#include <io.h> // _open
-#include <knownfolders.h> // FOLDERID_Profile
-#include <objbase.h> // CoTaskMemFree
-#include <shlobj.h> // SHGetKnownFolderPath
+#include <fcntl.h>
+#include <io.h> // _open
+#include <knownfolders.h> // FOLDERID_Profile
+#include <lmcons.h> // UNLEN
+#include <objbase.h> // CoTaskMemFree
+#include <shlobj.h> // SHGetKnownFolderPath
+#include <stdarg.h> // va_start, va_end, va_list
+#include <versionhelpers.h> // IsWindows8OrGreater
#include <algorithm>
#include <cstdio>
@@ -51,6 +53,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"
namespace blaze {
@@ -329,8 +332,6 @@
exit(exit_code);
}
-
-
// A signal-safe version of fprintf(stderr, ...).
//
// WARNING: any output from the blaze client may be interleaved
@@ -482,51 +483,56 @@
static const int MAX_CMDLINE_LENGTH = 32768;
struct CmdLine {
- char cmdline[MAX_CMDLINE_LENGTH];
+ WCHAR cmdline[MAX_CMDLINE_LENGTH];
};
static void CreateCommandLine(CmdLine* result, const string& exe,
const std::vector<string>& args_vector) {
- std::ostringstream cmdline;
+ std::wstringstream cmdline;
string short_exe;
- string error;
- if (!blaze_util::AsShortWindowsPath(exe, &short_exe, &error)) {
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "CreateCommandLine: AsShortWindowsPath(" << exe << "): " << error;
+ if (!exe.empty()) {
+ string error;
+ if (!blaze_util::AsShortWindowsPath(exe, &short_exe, &error)) {
+ BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
+ << "CreateCommandLine: AsShortWindowsPath(" << exe << "): " << error;
+ }
+ wstring wshort_exe = blaze_util::CstringToWstring(short_exe.c_str()).get();
+ cmdline << L'\"' << wshort_exe << L'\"';
}
+
bool first = true;
for (const auto& s : args_vector) {
if (first) {
+ // Skip first argument, it is equal to 'exe'.
first = false;
- // Skip first argument, instead use quoted executable name.
- cmdline << '\"' << short_exe << '\"';
continue;
} else {
- cmdline << ' ';
+ cmdline << L' ';
}
bool has_space = s.find(" ") != string::npos;
if (has_space) {
- cmdline << '\"';
+ cmdline << L'\"';
}
- std::string::const_iterator it = s.begin();
- while (it != s.end()) {
- char ch = *it++;
+ wstring ws = blaze_util::CstringToWstring(s.c_str()).get();
+ std::wstring::const_iterator it = ws.begin();
+ while (it != ws.end()) {
+ wchar_t ch = *it++;
switch (ch) {
- case '"':
+ case L'"':
// Escape double quotes
- cmdline << "\\\"";
+ cmdline << L"\\\"";
break;
- case '\\':
- if (it == s.end()) {
+ case L'\\':
+ if (it == ws.end()) {
// Backslashes at the end of the string are quoted if we add quotes
- cmdline << (has_space ? "\\\\" : "\\");
+ cmdline << (has_space ? L"\\\\" : L"\\");
} else {
// Backslashes everywhere else are quoted if they are followed by a
// quote or a backslash
- cmdline << (*it == '"' || *it == '\\' ? "\\\\" : "\\");
+ cmdline << (*it == L'"' || *it == L'\\' ? L"\\\\" : L"\\");
}
break;
@@ -536,20 +542,21 @@
}
if (has_space) {
- cmdline << '\"';
+ cmdline << L'\"';
}
}
- string cmdline_str = cmdline.str();
+ wstring cmdline_str = cmdline.str();
if (cmdline_str.size() >= MAX_CMDLINE_LENGTH) {
BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR)
<< "Command line too long (" << cmdline_str.size() << " > "
- << MAX_CMDLINE_LENGTH << "): " << cmdline_str;
+ << MAX_CMDLINE_LENGTH
+ << "): " << blaze_util::WstringToCstring(cmdline_str.c_str()).get();
}
// Copy command line into a mutable buffer.
// CreateProcess is allowed to mutate its command line argument.
- strncpy(result->cmdline, cmdline_str.c_str(), MAX_CMDLINE_LENGTH - 1);
+ wcsncpy(result->cmdline, cmdline_str.c_str(), MAX_CMDLINE_LENGTH - 1);
result->cmdline[MAX_CMDLINE_LENGTH - 1] = 0;
}
@@ -711,8 +718,8 @@
}
PROCESS_INFORMATION processInfo = {0};
- STARTUPINFOEXA startupInfoEx = {0};
- lpAttributeList->InitStartupInfoExA(&startupInfoEx);
+ STARTUPINFOEXW startupInfoEx = {0};
+ lpAttributeList->InitStartupInfoExW(&startupInfoEx);
CmdLine cmdline;
CreateCommandLine(&cmdline, exe, args_vector);
@@ -721,14 +728,14 @@
{
WithEnvVars env_obj(env);
- ok = CreateProcessA(
+ ok = CreateProcessW(
/* lpApplicationName */ NULL,
/* lpCommandLine */ cmdline.cmdline,
/* lpProcessAttributes */ NULL,
/* lpThreadAttributes */ NULL,
/* bInheritHandles */ TRUE,
/* dwCreationFlags */ DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP |
- EXTENDED_STARTUPINFO_PRESENT,
+ EXTENDED_STARTUPINFO_PRESENT,
/* lpEnvironment */ NULL,
/* lpCurrentDirectory */ NULL,
/* lpStartupInfo */ &startupInfoEx.StartupInfo,
@@ -770,85 +777,28 @@
// argument vector, wait for it to finish, then exit ourselves with the exitcode
// of that program.
void ExecuteProgram(const string& exe, const std::vector<string>& args_vector) {
+ std::wstring wexe = blaze_util::CstringToWstring(exe.c_str()).get();
+
CmdLine cmdline;
- CreateCommandLine(&cmdline, exe, args_vector);
+ CreateCommandLine(&cmdline, "", args_vector);
- STARTUPINFOA startupInfo = {0};
- startupInfo.cb = sizeof(STARTUPINFOA);
-
- PROCESS_INFORMATION processInfo = {0};
-
- HANDLE job = INVALID_HANDLE_VALUE;
- if (NestedJobsSupported()) {
- job = CreateJobObject(NULL, NULL);
- if (job == NULL) {
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "ExecuteProgram(" << exe
- << "): CreateJobObject failed: " << GetLastErrorString();
- }
-
- JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info = {0};
- job_info.BasicLimitInformation.LimitFlags =
- JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
-
- if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation,
- &job_info, sizeof(job_info))) {
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "ExecuteProgram(" << exe
- << "): SetInformationJobObject failed: " << GetLastErrorString();
- }
- }
-
- BOOL success = CreateProcessA(
- /* lpApplicationName */ NULL,
- /* lpCommandLine */ cmdline.cmdline,
- /* lpProcessAttributes */ NULL,
- /* lpThreadAttributes */ NULL,
- /* bInheritHandles */ TRUE,
- /* dwCreationFlags */ CREATE_SUSPENDED,
- /* lpEnvironment */ NULL,
- /* lpCurrentDirectory */ NULL,
- /* lpStartupInfo */ &startupInfo,
- /* lpProcessInformation */ &processInfo);
-
- if (!success) {
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "ExecuteProgram(" << exe << "): CreateProcess(" << cmdline.cmdline
- << ") failed: " << GetLastErrorString();
- }
-
- // On Windows versions that support nested jobs (Windows 8 and above), we
- // assign the Bazel server to a job object. Every process that Bazel creates,
- // as well as all their child processes, will be assigned to this job object.
- // When the Bazel server terminates the OS can reliably kill the entire
- // process tree under it. On Windows versions that don't support nested jobs
- // (Windows 7), we don't assign the Bazel server to a big job object. Instead,
- // when Bazel creates new processes, it does so using the JNI library. The
- // library assigns individual job objects to each subprocess. This way when
- // these processes terminate, the OS can kill all their subprocesses. Bazel's
- // own subprocesses are not in a job object though, so we only create
- // subprocesses via the JNI library.
- if (job != INVALID_HANDLE_VALUE) {
- if (!AssignProcessToJobObject(job, processInfo.hProcess)) {
- BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "ExecuteProgram(" << exe
- << "): AssignProcessToJobObject failed: " << GetLastErrorString();
- }
- }
- // Now that we potentially put the process into a new job object, we can start
- // running it.
- if (ResumeThread(processInfo.hThread) == -1) {
+ bazel::windows::WaitableProcess proc;
+ std::wstring werror;
+ if (!proc.Create(wexe, cmdline.cmdline, nullptr, L"", &werror) ||
+ proc.WaitFor(-1, nullptr, &werror) !=
+ bazel::windows::WaitableProcess::kWaitSuccess) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "ExecuteProgram(" << exe
- << "): ResumeThread failed: " << GetLastErrorString();
+ << ") failed: " << blaze_util::WstringToCstring(werror.c_str()).get();
}
-
- WaitForSingleObject(processInfo.hProcess, INFINITE);
- DWORD exit_code;
- GetExitCodeProcess(processInfo.hProcess, &exit_code);
- CloseHandle(processInfo.hProcess);
- CloseHandle(processInfo.hThread);
- exit(exit_code);
+ werror.clear();
+ int x = proc.GetExitCode(&werror);
+ if (!werror.empty()) {
+ BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
+ << "ExecuteProgram(" << exe
+ << ") failed: " << blaze_util::WstringToCstring(werror.c_str()).get();
+ }
+ exit(x);
}
const char kListSeparator = ';';
diff --git a/src/main/native/windows/BUILD b/src/main/native/windows/BUILD
index e8b7a0b..8567b35 100644
--- a/src/main/native/windows/BUILD
+++ b/src/main/native/windows/BUILD
@@ -38,6 +38,7 @@
srcs = ["process.cc"],
hdrs = ["process.h"],
visibility = [
+ "//src/main/cpp:__pkg__",
"//tools/test:__pkg__",
],
deps = [":lib-util"],
@@ -48,6 +49,7 @@
srcs = ["util.cc"],
hdrs = ["util.h"],
visibility = [
+ "//src/main/cpp:__pkg__",
"//src/tools/launcher/util:__pkg__",
"//tools/test:__pkg__",
],
diff --git a/src/main/native/windows/process.cc b/src/main/native/windows/process.cc
index 7aa6092..4d61a39 100644
--- a/src/main/native/windows/process.cc
+++ b/src/main/native/windows/process.cc
@@ -29,12 +29,48 @@
return s.str();
}
+static bool DupeHandle(HANDLE h, AutoHandle* out, std::wstring* error) {
+ HANDLE dup;
+ if (!DuplicateHandle(GetCurrentProcess(), h, GetCurrentProcess(), &dup, 0,
+ TRUE, DUPLICATE_SAME_ACCESS)) {
+ DWORD err = GetLastError();
+ *error =
+ MakeErrorMessage(WSTR(__FILE__), __LINE__, L"DupeHandle", L"", err);
+ return false;
+ }
+ *out = dup;
+ return true;
+}
+
+bool WaitableProcess::Create(const std::wstring& argv0,
+ const std::wstring& argv_rest, void* env,
+ const std::wstring& wcwd, std::wstring* error) {
+ AutoHandle dup_in, dup_out, dup_err;
+ if (!DupeHandle(GetStdHandle(STD_INPUT_HANDLE), &dup_in, error) ||
+ !DupeHandle(GetStdHandle(STD_OUTPUT_HANDLE), &dup_out, error) ||
+ !DupeHandle(GetStdHandle(STD_ERROR_HANDLE), &dup_err, error)) {
+ return false;
+ }
+ return Create(argv0, argv_rest, env, wcwd, dup_in, dup_out, dup_err, nullptr,
+ true, error);
+}
+
bool WaitableProcess::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,
LARGE_INTEGER* opt_out_start_time,
std::wstring* error) {
+ return Create(argv0, argv_rest, env, wcwd, stdin_process, stdout_process,
+ stderr_process, opt_out_start_time, false, error);
+}
+
+bool WaitableProcess::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,
+ LARGE_INTEGER* opt_out_start_time,
+ bool create_window, std::wstring* error) {
std::wstring cwd;
std::wstring error_msg(AsShortPath(wcwd, &cwd));
if (!error_msg.empty()) {
@@ -131,9 +167,8 @@
/* lpProcessAttributes */ NULL,
/* lpThreadAttributes */ NULL,
/* bInheritHandles */ attr_list->InheritAnyHandles() ? TRUE : FALSE,
- /* dwCreationFlags */ CREATE_NO_WINDOW // Don't create console
- // window
- | CREATE_NEW_PROCESS_GROUP // So that Ctrl-Break isn't propagated
+ /* dwCreationFlags */ (create_window ? 0 : CREATE_NO_WINDOW) |
+ CREATE_NEW_PROCESS_GROUP // So that Ctrl-Break isn't propagated
| CREATE_SUSPENDED // So that it doesn't start a new job itself
| EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT,
/* lpEnvironment */ env,
diff --git a/src/main/native/windows/process.h b/src/main/native/windows/process.h
index de0e1e4..0f43eda 100644
--- a/src/main/native/windows/process.h
+++ b/src/main/native/windows/process.h
@@ -40,6 +40,9 @@
WaitableProcess() : pid_(0), exit_code_(STILL_ACTIVE) {}
bool Create(const std::wstring& argv0, const std::wstring& argv_rest,
+ void* env, const std::wstring& wcwd, std::wstring* error);
+
+ 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,
LARGE_INTEGER* opt_out_start_time, std::wstring* error);
@@ -54,6 +57,12 @@
DWORD GetPid() const { return pid_; }
private:
+ 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,
+ LARGE_INTEGER* opt_out_start_time, bool create_window,
+ std::wstring* error);
+
AutoHandle process_, job_, ioport_;
DWORD pid_, exit_code_;
};
diff --git a/src/main/native/windows/util.cc b/src/main/native/windows/util.cc
index 829fd5c..ea3a59b 100644
--- a/src/main/native/windows/util.cc
+++ b/src/main/native/windows/util.cc
@@ -161,18 +161,6 @@
return reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(data_.get());
}
-void AutoAttributeList::InitStartupInfoExA(STARTUPINFOEXA* startup_info) const {
- ZeroMemory(startup_info, sizeof(STARTUPINFOEXA));
- startup_info->StartupInfo.cb = sizeof(STARTUPINFOEXA);
- if (InheritAnyHandles()) {
- startup_info->StartupInfo.dwFlags = STARTF_USESTDHANDLES;
- startup_info->StartupInfo.hStdInput = handles_.StdIn();
- startup_info->StartupInfo.hStdOutput = handles_.StdOut();
- startup_info->StartupInfo.hStdError = handles_.StdErr();
- startup_info->lpAttributeList = *this;
- }
-}
-
void AutoAttributeList::InitStartupInfoExW(STARTUPINFOEXW* startup_info) const {
ZeroMemory(startup_info, sizeof(STARTUPINFOEXW));
startup_info->StartupInfo.cb = sizeof(STARTUPINFOEXW);
diff --git a/src/main/native/windows/util.h b/src/main/native/windows/util.h
index 2766017..f5bf048 100644
--- a/src/main/native/windows/util.h
+++ b/src/main/native/windows/util.h
@@ -75,8 +75,6 @@
bool InheritAnyHandles() const { return handles_.ValidHandlesCount() > 0; }
- void InitStartupInfoExA(STARTUPINFOEXA* startup_info) const;
-
void InitStartupInfoExW(STARTUPINFOEXW* startup_info) const;
private:
