Bazel client: retry moving install base directory

Do not give up immediately if renaming/moving the
install base directory fails, wait and retry
instead.

This is necessary because on Windows the directory
we just created and populated with the extracted
embedded binaries may still be scanned by the
antivirus, so there are open file handles in it so
it cannot be renamed. This new logic ensures the
AV has enough time to scan all files, close the
handles, letting us successfully rename the
directory.

Fixes the occasional "install base directory
cannot be renamed in place" error messages on
Windows.

See https://github.com/bazelbuild/bazel/issues/2107

--
PiperOrigin-RevId: 146899919
MOS_MIGRATED_REVID=146899919

5 files changed, 94 insertions, 6 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index e26b41dab0..b0de6b3aa8 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -916,12 +916,31 @@ static void ExtractData(const string &self_path) {
     uint64_t et = GetMillisecondsMonotonic();
     globals->extract_data_time = et - st;
 
-    // Now rename the completed installation to its final name. If this
-    // fails due to an ENOTEMPTY then we assume another good
-    // installation snuck in before us.
-    if (rename(tmp_install.c_str(), globals->options->install_base.c_str()) ==
-            -1 &&
-        errno != ENOTEMPTY) {
+    // Now rename the completed installation to its final name.
+    int attempts = 0;
+    while (attempts < 120) {
+      int result = blaze_util::RenameDirectory(
+          tmp_install.c_str(), globals->options->install_base.c_str());
+      if (result == blaze_util::kRenameDirectorySuccess ||
+          result == blaze_util::kRenameDirectoryFailureNotEmpty) {
+        // If renaming fails because the directory already exists and is not
+        // empty, then we assume another good installation snuck in before us.
+        break;
+      } else {
+        // Otherwise the install directory may still be scanned by the antivirus
+        // (in case we're running on Windows) so we need to wait for that to
+        // finish and try renaming again.
+        ++attempts;
+        fprintf(stderr,
+                "install base directory '%s' could not be renamed into place"
+                "after %d second(s), trying again\r",
+                tmp_install.c_str(), attempts);
+        std::this_thread::sleep_for(std::chrono::seconds(1));
+      }
+    }
+
+    // Give up renaming after 120 failed attempts / 2 minutes.
+    if (attempts == 120) {
       pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
            "install base directory '%s' could not be renamed into place",
            tmp_install.c_str());
diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h
index 600820ba0c..b104ee7e6a 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -64,6 +64,16 @@ bool ReadFile(const std::string &filename, std::string *content,
 // Returns false on failure, sets errno.
 bool WriteFile(const std::string &content, const std::string &filename);
 
+enum RenameDirectoryResult {
+  kRenameDirectorySuccess = 0,
+  kRenameDirectoryFailureNotEmpty = 1,
+  kRenameDirectoryFailureOtherError = 2,
+};
+
+// Renames the directory at `old_name` to `new_name`.
+// Returns one of the RenameDirectoryResult enum values.
+int RenameDirectory(const std::string &old_name, const std::string &new_name);
+
 // Unlinks the file given by 'file_path'.
 // Returns true on success. In case of failure sets errno.
 bool UnlinkPath(const std::string &file_path);
diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc
index a1d806a9d9..c431ac4775 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -208,6 +208,15 @@ bool WriteFile(const void *data, size_t size, const string &filename) {
   return result;
 }
 
+int RenameDirectory(const std::string &old_name, const std::string &new_name) {
+  if (rename(old_name.c_str(), new_name.c_str()) == 0) {
+    return kRenameDirectorySuccess;
+  } else {
+    return errno == ENOTEMPTY ? kRenameDirectoryFailureNotEmpty
+                              : kRenameDirectoryFailureOtherError;
+  }
+}
+
 bool UnlinkPath(const string &file_path) {
   return unlink(file_path.c_str()) == 0;
 }
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index d8cfad2e93..489b8bab5e 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -596,6 +596,37 @@ bool WriteFile(const void* data, size_t size, const string& filename) {
   return result;
 }
 
+int RenameDirectory(const std::string& old_name, const std::string& new_name) {
+  wstring wold_name;
+  if (!AsWindowsPathWithUncPrefix(old_name, &wold_name)) {
+    std::ostringstream error;
+    error << "RenameDirectory(" << old_name << ", " << new_name
+          << "): AsWindowsPathWithUncPrefix failed for old_name: "
+          << GetLastErrorString() << "\n";
+    PrintError(error.str().c_str());
+    return kRenameDirectoryFailureOtherError;
+  }
+
+  wstring wnew_name;
+  if (!AsWindowsPathWithUncPrefix(new_name, &wnew_name)) {
+    std::ostringstream error;
+    error << "RenameDirectory(" << old_name << ", " << new_name
+          << "): AsWindowsPathWithUncPrefix failed for new_name: "
+          << GetLastErrorString() << "\n";
+    PrintError(error.str().c_str());
+    return kRenameDirectoryFailureOtherError;
+  }
+
+  if (!::MoveFileExW(wold_name.c_str(), wnew_name.c_str(),
+                     MOVEFILE_COPY_ALLOWED | MOVEFILE_FAIL_IF_NOT_TRACKABLE |
+                         MOVEFILE_WRITE_THROUGH)) {
+    return GetLastError() == ERROR_ALREADY_EXISTS
+               ? kRenameDirectoryFailureNotEmpty
+               : kRenameDirectoryFailureOtherError;
+  }
+  return kRenameDirectorySuccess;
+}
+
 static bool UnlinkPathW(const wstring& path) {
   DWORD attrs = ::GetFileAttributesW(path.c_str());
   if (attrs == INVALID_FILE_ATTRIBUTES) {
diff --git a/src/test/cpp/util/file_test.cc b/src/test/cpp/util/file_test.cc
index 83bf3a9845..2b9063c820 100644
--- a/src/test/cpp/util/file_test.cc
+++ b/src/test/cpp/util/file_test.cc
@@ -144,4 +144,23 @@ TEST(FileTest, TestMtimeHandling) {
   ASSERT_FALSE(mtime.get()->GetIfInDistantFuture(file, &actual));
 }
 
+TEST(FileTest, TestRenameDirectory) {
+  const char* tempdir_cstr = getenv("TEST_TMPDIR");
+  EXPECT_NE(tempdir_cstr, nullptr);
+  EXPECT_NE(tempdir_cstr[0], 0);
+  string tempdir(tempdir_cstr);
+
+  string dir1(JoinPath(tempdir, "test_rename_dir/dir1"));
+  string dir2(JoinPath(tempdir, "test_rename_dir/dir2"));
+  EXPECT_TRUE(MakeDirectories(dir1, 0700));
+  string file1(JoinPath(dir1, "file1.txt"));
+  EXPECT_TRUE(WriteFile("hello", 5, file1));
+
+  ASSERT_EQ(RenameDirectory(dir1, dir2), kRenameDirectorySuccess);
+  ASSERT_EQ(RenameDirectory(dir1, dir2), kRenameDirectoryFailureOtherError);
+  EXPECT_TRUE(MakeDirectories(dir1, 0700));
+  EXPECT_TRUE(WriteFile("hello", 5, file1));
+  ASSERT_EQ(RenameDirectory(dir2, dir1), kRenameDirectoryFailureNotEmpty);
+}
+
 }  // namespace blaze_util