Windows, JNI: AutoAttributeList owns HANDLE array

Also in this commit:

- AutoAttributeList can initialize a STARTUPINFOEX
  structure (both A and W)

Closes #6758.

PiperOrigin-RevId: 222818516

src/main/cpp/blaze_util_windows.cc

@@ -690,23 +690,16 @@ int ExecuteDaemon(const string& exe,
   // Create an attribute list.
   wstring werror;
   std::unique_ptr<AutoAttributeList> 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);
   }
 
   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);

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<AutoAttributeList>* 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<uint8_t> data(new uint8_t[size]);
+  std::unique_ptr<uint8_t[]> data(new uint8_t[size]);
   LPPROC_THREAD_ATTRIBUTE_LIST attrs =
       reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(data.get());
   if (!InitializeProcThreadAttributeList(attrs, kAttributeCount, 0, &size)) {
@@ -92,20 +92,31 @@ bool AutoAttributeList::Create(HANDLE* handles, size_t count,
     return false;
   }
 
+  std::unique_ptr<AutoAttributeList> 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__,
                                     L"UpdateProcThreadAttribute", L"", err);
     }
     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<uint8_t[]>&& 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<LPPROC_THREAD_ATTRIBUTE_LIST>(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"\"";
 }

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<AutoAttributeList>* 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<uint8_t[]>&& 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<uint8_t[]> data_;
+  StdHandles handles_;
 };
 
 #define WSTR1(x) L##x

tools/test/windows/tw.cc

@@ -1146,11 +1146,10 @@ bool StartSubprocess(const Path& path, const std::vector<const wchar_t*>& 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<bazel::windows::AutoAttributeList> 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<const wchar_t*>& 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<WCHAR[]> cmdline;
   if (!CreateCommandLine(path, args, &cmdline)) {