From f6774fa8ee09f51ace5257b6cc43b7886b3f285a Mon Sep 17 00:00:00 2001
From: Thomas Wolf <twolf@apache.org>
Date: Sun, 8 Oct 2023 22:03:22 +0200
Subject: [PATCH] FileUtils.rename(): better retry handling

When the atomic move fails on Windows, it may be because some other
thread is currently reading the destination. If we delete the file
then, that reader may get an exception, and conclude the file didn't
exist, even though the rename() would re-create it right away.

Try to avoid this from happening frequently by only deleting the
destination on the last retry. Also don't sleep after the last attempt.

Bug: 451508
Change-Id: I95bb4ec59d6e7efb4a7fc8d67f5df301f690257a
Signed-off-by: Thomas Wolf <twolf@apache.org>
---
 .../src/org/eclipse/jgit/util/FileUtils.java  | 39 +++++++++++--------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
index 77c71e4bf..842d8ba33 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
@@ -295,6 +295,7 @@ public static void rename(final File src, final File dst,
 			CopyOption... options)
 					throws AtomicMoveNotSupportedException, IOException {
 		int attempts = FS.DETECTED.retryFailedLockFileCommit() ? 10 : 1;
+		IOException finalError = null;
 		while (--attempts >= 0) {
 			try {
 				Files.move(toPath(src), toPath(dst), options);
@@ -302,29 +303,35 @@ public static void rename(final File src, final File dst,
 			} catch (AtomicMoveNotSupportedException e) {
 				throw e;
 			} catch (IOException e) {
-				try {
-					if (!dst.delete()) {
-						delete(dst, EMPTY_DIRECTORIES_ONLY | RECURSIVE);
+				if (attempts == 0) {
+					// Only delete on the last attempt.
+					try {
+						if (!dst.delete()) {
+							delete(dst, EMPTY_DIRECTORIES_ONLY | RECURSIVE);
+						}
+						// On *nix there is no try, you do or do not
+						Files.move(toPath(src), toPath(dst), options);
+						return;
+					} catch (IOException e2) {
+						e2.addSuppressed(e);
+						finalError = e2;
 					}
-					// On *nix there is no try, you do or do not
-					Files.move(toPath(src), toPath(dst), options);
-					return;
-				} catch (IOException e2) {
-					// ignore and continue retry
 				}
 			}
-			try {
-				Thread.sleep(100);
-			} catch (InterruptedException e) {
-				throw new IOException(
-						MessageFormat.format(JGitText.get().renameFileFailed,
-								src.getAbsolutePath(), dst.getAbsolutePath()),
-						e);
+			if (attempts > 0) {
+				try {
+					Thread.sleep(100);
+				} catch (InterruptedException e) {
+					throw new IOException(MessageFormat.format(
+							JGitText.get().renameFileFailed,
+							src.getAbsolutePath(), dst.getAbsolutePath()), e);
+				}
 			}
 		}
 		throw new IOException(
 				MessageFormat.format(JGitText.get().renameFileFailed,
-						src.getAbsolutePath(), dst.getAbsolutePath()));
+						src.getAbsolutePath(), dst.getAbsolutePath()),
+				finalError);
 	}
 
 	/**