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>

1 files 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 77c71e4bf0..842d8ba338 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 class FileUtils {
 			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 class FileUtils {
 			} 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);
 	}
 
 	/**