Bazel client, Windows: use more of AutoHandle

--
PiperOrigin-RevId: 147008328
MOS_MIGRATED_REVID=147008328

4 files changed, 35 insertions, 45 deletions

diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index a8462d2e38..b9653ff74a 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -1400,7 +1400,7 @@ int GetTerminalColumns() {
   // Windows console attached so GetConsoleScreenBufferInfo fails.
   windows_util::AutoHandle stdout_handle(::GetStdHandle(STD_OUTPUT_HANDLE));
   CONSOLE_SCREEN_BUFFER_INFO screen_info;
-  if (GetConsoleScreenBufferInfo(stdout_handle.handle, &screen_info)) {
+  if (GetConsoleScreenBufferInfo(stdout_handle, &screen_info)) {
     int width = 1 + screen_info.srWindow.Right - screen_info.srWindow.Left;
     if (width > 1) {
       return width;
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index 489b8bab5e..8b7950a608 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -119,21 +119,10 @@ class WindowsPipe : public IPipe {
 
   WindowsPipe() = delete;
 
-  virtual ~WindowsPipe() {
-    if (_read_handle != INVALID_HANDLE_VALUE) {
-      CloseHandle(_read_handle);
-      _read_handle = INVALID_HANDLE_VALUE;
-    }
-    if (_write_handle != INVALID_HANDLE_VALUE) {
-      CloseHandle(_write_handle);
-      _write_handle = INVALID_HANDLE_VALUE;
-    }
-  }
-
   bool Send(const void* buffer, int size) override {
     DWORD actually_written = 0;
-    return ::WriteFile(_write_handle, buffer, size, &actually_written, NULL) ==
-           TRUE;
+    return ::WriteFile(_write_handle, buffer, size, &actually_written,
+                       NULL) == TRUE;
   }
 
   int Receive(void* buffer, int size) override {
@@ -144,8 +133,8 @@ class WindowsPipe : public IPipe {
   }
 
  private:
-  HANDLE _read_handle;
-  HANDLE _write_handle;
+  windows_util::AutoHandle _read_handle;
+  windows_util::AutoHandle _write_handle;
 };
 
 IPipe* CreatePipe() {
@@ -203,12 +192,12 @@ bool WindowsFileMtime::GetIfInDistantFuture(const string& path, bool* result) {
           ? (FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS)
           : FILE_ATTRIBUTE_NORMAL,
       /* hTemplateFile */ NULL));
-  if (handle.handle == INVALID_HANDLE_VALUE) {
+  if (!handle.IsValid()) {
     return false;
   }
   FILETIME mtime;
   if (!::GetFileTime(
-          /* hFile */ handle.handle,
+          /* hFile */ handle,
           /* lpCreationTime */ NULL,
           /* lpLastAccessTime */ NULL,
           /* lpLastWriteTime */ &mtime)) {
@@ -252,11 +241,11 @@ bool WindowsFileMtime::Set(const string& path, const FILETIME& time) {
           ? (FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS)
           : FILE_ATTRIBUTE_NORMAL,
       /* hTemplateFile */ NULL));
-  if (handle.handle == INVALID_HANDLE_VALUE) {
+  if (!handle.IsValid()) {
     return false;
   }
   return ::SetFileTime(
-             /* hFile */ handle.handle,
+             /* hFile */ handle,
              /* lpCreationTime */ NULL,
              /* lpLastAccessTime */ NULL,
              /* lpLastWriteTime */ &time) == TRUE;
@@ -540,26 +529,26 @@ bool ReadFile(const string& filename, string* content, int max_size) {
   if (!AsWindowsPathWithUncPrefix(filename, &wfilename)) {
     return false;
   }
-  HANDLE handle = ::CreateFileW(
+  windows_util::AutoHandle handle(::CreateFileW(
       /* lpFileName */ wfilename.c_str(),
       /* dwDesiredAccess */ GENERIC_READ,
       /* dwShareMode */ FILE_SHARE_READ,
       /* lpSecurityAttributes */ NULL,
       /* dwCreationDisposition */ OPEN_EXISTING,
       /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL,
-      /* hTemplateFile */ NULL);
-  if (handle == INVALID_HANDLE_VALUE) {
+      /* hTemplateFile */ NULL));
+  if (!handle.IsValid()) {
     return false;
   }
 
+  HANDLE h = handle;
   bool result = ReadFrom(
-      [handle](void* buf, int len) {
+      [h](void* buf, int len) {
         DWORD actually_read = 0;
-        ::ReadFile(handle, buf, len, &actually_read, NULL);
+        ::ReadFile(h, buf, len, &actually_read, NULL);
         return actually_read;
       },
       content, max_size);
-  CloseHandle(handle);
   return result;
 }
 
@@ -573,26 +562,26 @@ bool WriteFile(const void* data, size_t size, const string& filename) {
   }
 
   UnlinkPathW(wpath);  // We don't care about the success of this.
-  HANDLE handle = ::CreateFileW(
+  windows_util::AutoHandle handle(::CreateFileW(
       /* lpFileName */ wpath.c_str(),
       /* dwDesiredAccess */ GENERIC_WRITE,
       /* dwShareMode */ FILE_SHARE_READ,
       /* lpSecurityAttributes */ NULL,
       /* dwCreationDisposition */ CREATE_ALWAYS,
       /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL,
-      /* hTemplateFile */ NULL);
-  if (handle == INVALID_HANDLE_VALUE) {
+      /* hTemplateFile */ NULL));
+  if (!handle.IsValid()) {
     return false;
   }
 
+  HANDLE h = handle;
   bool result = WriteTo(
-      [handle](const void* buf, size_t bufsize) {
+      [h](const void* buf, size_t bufsize) {
         DWORD actually_written = 0;
-        ::WriteFile(handle, buf, bufsize, &actually_written, NULL);
+        ::WriteFile(h, buf, bufsize, &actually_written, NULL);
         return actually_written;
       },
       data, size);
-  CloseHandle(handle);
   return result;
 }
 
@@ -758,8 +747,9 @@ bool JunctionResolver::Resolve(const WCHAR* path, unique_ptr<WCHAR[]>* result,
         return false;
       }
       // Get a handle to the directory.
-      HANDLE handle = OpenDirectory(path, /* read_write */ false);
-      if (handle == INVALID_HANDLE_VALUE) {
+      windows_util::AutoHandle handle(
+          OpenDirectory(path, /* read_write */ false));
+      if (!handle.IsValid()) {
         // Opening the junction failed for whatever reason. For all intents and
         // purposes we can treat this file as if it didn't exist.
         return false;
@@ -769,7 +759,6 @@ bool JunctionResolver::Resolve(const WCHAR* path, unique_ptr<WCHAR[]>* result,
       BOOL ok = ::DeviceIoControl(
           handle, FSCTL_GET_REPARSE_POINT, NULL, 0, reparse_buffer_,
           MAXIMUM_REPARSE_DATA_BUFFER_SIZE, &bytes_returned, NULL);
-      CloseHandle(handle);
       if (!ok) {
         // Reading the junction data failed. For all intents and purposes we can
         // treat this file as if it didn't exist.
@@ -827,20 +816,15 @@ static bool CanReadFileW(const wstring& path) {
   }
   // The only easy way to find out if a file is readable is to attempt to open
   // it for reading.
-  HANDLE handle = ::CreateFileW(
+  windows_util::AutoHandle handle(::CreateFileW(
       /* lpFileName */ path.c_str(),
       /* dwDesiredAccess */ GENERIC_READ,
       /* dwShareMode */ FILE_SHARE_READ,
       /* lpSecurityAttributes */ NULL,
       /* dwCreationDisposition */ OPEN_EXISTING,
       /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL,
-      /* hTemplateFile */ NULL);
-  if (handle == INVALID_HANDLE_VALUE) {
-    return false;
-  } else {
-    CloseHandle(handle);
-    return true;
-  }
+      /* hTemplateFile */ NULL));
+  return handle.IsValid();
 }
 
 bool CanReadFile(const std::string& path) {
diff --git a/src/main/native/windows_processes.cc b/src/main/native/windows_processes.cc
index 4b84c8ef8b..f016550d48 100644
--- a/src/main/native/windows_processes.cc
+++ b/src/main/native/windows_processes.cc
@@ -235,7 +235,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess(
         /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL,
         /* hTemplateFile */ NULL);
 
-    if (stdout_process.handle == INVALID_HANDLE_VALUE) {
+    if (!stdout_process.IsValid()) {
       result->error_ = windows_util::GetLastErrorString("CreateFile(stdout)");
       return PtrAsJlong(result);
     }
@@ -359,7 +359,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess(
   }
 
   // Now that we put the process in a new job object, we can start executing it
-  if (ResumeThread(thread.handle) == -1) {
+  if (ResumeThread(thread) == -1) {
     result->error_ = windows_util::GetLastErrorString("ResumeThread()");
     return PtrAsJlong(result);
   }
diff --git a/src/main/native/windows_util.h b/src/main/native/windows_util.h
index a2f09abc06..a79d9dde64 100644
--- a/src/main/native/windows_util.h
+++ b/src/main/native/windows_util.h
@@ -29,6 +29,8 @@ using std::unique_ptr;
 using std::wstring;
 
 // A wrapper for the `HANDLE` type that calls CloseHandle in its d'tor.
+// WARNING: do not use for HANDLE returned by FindFirstFile; those must be
+// closed with FindClose (otherwise they aren't closed properly).
 struct AutoHandle {
   AutoHandle(HANDLE _handle = INVALID_HANDLE_VALUE) : handle(_handle) {}
 
@@ -37,6 +39,10 @@ struct AutoHandle {
     handle = INVALID_HANDLE_VALUE;
   }
 
+  bool IsValid() { return handle != INVALID_HANDLE_VALUE && handle != NULL; }
+
+  operator HANDLE() const { return handle; }
+
   HANDLE handle;
 };