diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc index 7f9407949b7a22..3319faf37e3420 100644 --- a/src/main/cpp/blaze_util_windows.cc +++ b/src/main/cpp/blaze_util_windows.cc @@ -690,9 +690,8 @@ int ExecuteDaemon(const string& exe, // Create an attribute list. wstring werror; std::unique_ptr lpAttributeList; - HANDLE handlesToInherit[3] = {devnull, stdout_file, stderr_handle}; - if (!AutoAttributeList::Create(handlesToInherit, 3, &lpAttributeList, - &werror)) { + if (!AutoAttributeList::Create(devnull, stdout_file, stderr_handle, + &lpAttributeList, &werror)) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) << "ExecuteDaemon(" << exe << "): attribute list creation failed: " << blaze_util::WstringToString(werror); @@ -700,13 +699,7 @@ int ExecuteDaemon(const string& exe, PROCESS_INFORMATION processInfo = {0}; STARTUPINFOEXA startupInfoEx = {0}; - - startupInfoEx.StartupInfo.cb = sizeof(startupInfoEx); - startupInfoEx.StartupInfo.hStdInput = devnull; - startupInfoEx.StartupInfo.hStdOutput = stdout_file; - startupInfoEx.StartupInfo.hStdError = stderr_handle; - startupInfoEx.StartupInfo.dwFlags = STARTF_USESTDHANDLES; - startupInfoEx.lpAttributeList = *(lpAttributeList.get()); + lpAttributeList->InitStartupInfoExA(&startupInfoEx); CmdLine cmdline; CreateCommandLine(&cmdline, exe, args_vector); diff --git a/src/main/native/windows/util.cc b/src/main/native/windows/util.cc index a7451a64b35bee..f3182c2184834e 100644 --- a/src/main/native/windows/util.cc +++ b/src/main/native/windows/util.cc @@ -70,7 +70,7 @@ wstring GetLastErrorString(DWORD error_code) { return result; } -bool AutoAttributeList::Create(HANDLE* handles, size_t count, +bool AutoAttributeList::Create(HANDLE stdin_h, HANDLE stdout_h, HANDLE stderr_h, std::unique_ptr* result, wstring* error_msg) { static constexpr DWORD kAttributeCount = 1; @@ -79,7 +79,7 @@ bool AutoAttributeList::Create(HANDLE* handles, size_t count, // According to MSDN, the first call to InitializeProcThreadAttributeList is // expected to fail. InitializeProcThreadAttributeList(NULL, kAttributeCount, 0, &size); - std::unique_ptr data(new uint8_t[size]); + std::unique_ptr data(new uint8_t[size]); LPPROC_THREAD_ATTRIBUTE_LIST attrs = reinterpret_cast(data.get()); if (!InitializeProcThreadAttributeList(attrs, kAttributeCount, 0, &size)) { @@ -92,8 +92,12 @@ bool AutoAttributeList::Create(HANDLE* handles, size_t count, return false; } + std::unique_ptr attr_list( + new AutoAttributeList(std::move(data), stdin_h, stdout_h, stderr_h)); if (!UpdateProcThreadAttribute(attrs, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, - handles, count * sizeof(HANDLE), NULL, NULL)) { + attr_list->handles_.handle_array, + StdHandles::kHandleCount * sizeof(HANDLE), + NULL, NULL)) { if (error_msg) { DWORD err = GetLastError(); *error_msg = MakeErrorMessage(WSTR(__FILE__), __LINE__, @@ -101,11 +105,18 @@ bool AutoAttributeList::Create(HANDLE* handles, size_t count, } return false; } - result->reset(new AutoAttributeList(data.release())); + *result = std::move(attr_list); return true; } -AutoAttributeList::AutoAttributeList(uint8_t* data) : data_(data) {} +AutoAttributeList::AutoAttributeList(std::unique_ptr&& data, + HANDLE stdin_h, HANDLE stdout_h, + HANDLE stderr_h) + : data_(std::move(data)) { + handles_.stdin_h = stdin_h; + handles_.stdout_h = stdout_h; + handles_.stderr_h = stderr_h; +} AutoAttributeList::~AutoAttributeList() { DeleteProcThreadAttributeList(*this); @@ -115,6 +126,26 @@ AutoAttributeList::operator LPPROC_THREAD_ATTRIBUTE_LIST() const { return reinterpret_cast(data_.get()); } +void AutoAttributeList::InitStartupInfoExA(STARTUPINFOEXA* startup_info) const { + ZeroMemory(startup_info, sizeof(STARTUPINFOEXA)); + startup_info->StartupInfo.cb = sizeof(STARTUPINFOEXA); + startup_info->StartupInfo.dwFlags = STARTF_USESTDHANDLES; + startup_info->StartupInfo.hStdInput = handles_.stdin_h; + startup_info->StartupInfo.hStdOutput = handles_.stdout_h; + startup_info->StartupInfo.hStdError = handles_.stderr_h; + startup_info->lpAttributeList = *this; +} + +void AutoAttributeList::InitStartupInfoExW(STARTUPINFOEXW* startup_info) const { + ZeroMemory(startup_info, sizeof(STARTUPINFOEXW)); + startup_info->StartupInfo.cb = sizeof(STARTUPINFOEXW); + startup_info->StartupInfo.dwFlags = STARTF_USESTDHANDLES; + startup_info->StartupInfo.hStdInput = handles_.stdin_h; + startup_info->StartupInfo.hStdOutput = handles_.stdout_h; + startup_info->StartupInfo.hStdError = handles_.stderr_h; + startup_info->lpAttributeList = *this; +} + static void QuotePath(const wstring& path, wstring* result) { *result = wstring(L"\"") + path + L"\""; } diff --git a/src/main/native/windows/util.h b/src/main/native/windows/util.h index 12ca399f8e6450..1cd2dabe9e78f8 100644 --- a/src/main/native/windows/util.h +++ b/src/main/native/windows/util.h @@ -66,17 +66,37 @@ class AutoHandle { class AutoAttributeList { public: - static bool Create(HANDLE* handles, size_t count, + static bool Create(HANDLE stdin_h, HANDLE stdout_h, HANDLE stderr_h, std::unique_ptr* result, wstring* error_msg = nullptr); ~AutoAttributeList(); - operator LPPROC_THREAD_ATTRIBUTE_LIST() const; + + void InitStartupInfoExA(STARTUPINFOEXA* startup_info) const; + + void InitStartupInfoExW(STARTUPINFOEXW* startup_info) const; private: - AutoAttributeList(uint8_t* data); + struct StdHandles { + static constexpr size_t kHandleCount = 3; + union { + HANDLE handle_array[kHandleCount]; + struct { + HANDLE stdin_h; + HANDLE stdout_h; + HANDLE stderr_h; + }; + }; + }; + + AutoAttributeList(std::unique_ptr&& data, HANDLE stdin_h, + HANDLE stdout_h, HANDLE stderr_h); AutoAttributeList(const AutoAttributeList&) = delete; AutoAttributeList& operator=(const AutoAttributeList&) = delete; + + operator LPPROC_THREAD_ATTRIBUTE_LIST() const; + std::unique_ptr data_; + StdHandles handles_; }; #define WSTR1(x) L##x diff --git a/tools/test/windows/tw.cc b/tools/test/windows/tw.cc index 62316ce499e561..9bfdc73ff5066e 100644 --- a/tools/test/windows/tw.cc +++ b/tools/test/windows/tw.cc @@ -1146,11 +1146,10 @@ bool StartSubprocess(const Path& path, const std::vector& args, // Create an attribute object that specifies which particular handles shall // the subprocess inherit. We pass this object to CreateProcessW. - HANDLE handle_array[] = {devnull_read, pipe_write, pipe_write_dup}; std::unique_ptr attr_list; std::wstring werror; - if (!bazel::windows::AutoAttributeList::Create(handle_array, 3, &attr_list, - &werror)) { + if (!bazel::windows::AutoAttributeList::Create( + devnull_read, pipe_write, pipe_write_dup, &attr_list, &werror)) { LogError(__LINE__, werror); return false; } @@ -1185,16 +1184,7 @@ bool StartSubprocess(const Path& path, const std::vector& args, PROCESS_INFORMATION process_info; STARTUPINFOEXW startup_info; - ZeroMemory(&startup_info, sizeof(STARTUPINFOEXW)); - startup_info.StartupInfo.cb = sizeof(STARTUPINFOEXW); - startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES; - // Do not Release() `devnull_read`, `pipe_write`, and `pipe_write_dup`. The - // subprocess inherits a copy of these handles and we need to close them in - // this process (via ~AutoHandle()). - startup_info.StartupInfo.hStdInput = devnull_read; - startup_info.StartupInfo.hStdOutput = pipe_write; - startup_info.StartupInfo.hStdError = pipe_write_dup; - startup_info.lpAttributeList = *attr_list.get(); + attr_list->InitStartupInfoExW(&startup_info); std::unique_ptr cmdline; if (!CreateCommandLine(path, args, &cmdline)) {