From: Jens Baumgart <jens.baumgart@sap.com>
Date: Wed, 21 Jul 2010 07:35:15 +0000 (+0200)
Subject: Fix concurrent read / write issue in GitIndex on Windows
X-Git-Tag: v0.9.1~162^2
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fchanges%2F52%2F1152%2F4;p=jgit.git

Fix concurrent read / write issue in GitIndex on Windows

GitIndex.write fails if another thread concurrently reads
the index file. The problem is fixed by retrying the rename
operation if it fails.

Bug: 311051
Change-Id: Ib243d2a90adae312712d02521de4834d06804944
Signed-off-by: Jens Baumgart <jens.baumgart@sap.com>
---

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitIndex.java
index defda148c8..df4906374f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitIndex.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitIndex.java
@@ -297,10 +297,32 @@ public class GitIndex {
 			fc.write(buf);
 			fc.close();
 			fileOutputStream.close();
-			if (cacheFile.exists())
-				if (!cacheFile.delete())
-					throw new IOException(
-						JGitText.get().couldNotRenameDeleteOldIndex);
+			if (cacheFile.exists()) {
+				if (db.getFS().retryFailedLockFileCommit()) {
+					// file deletion fails on windows if another
+					// thread is reading the file concurrently
+					// So let's try 10 times...
+					boolean deleted = false;
+					for (int i = 0; i < 10; i++) {
+						if (cacheFile.delete()) {
+							deleted = true;
+							break;
+						}
+						try {
+							Thread.sleep(100);
+						} catch (InterruptedException e) {
+							// ignore
+						}
+					}
+					if (!deleted)
+						throw new IOException(
+								JGitText.get().couldNotRenameDeleteOldIndex);
+				} else {
+					if (!cacheFile.delete())
+						throw new IOException(
+								JGitText.get().couldNotRenameDeleteOldIndex);
+				}
+			}
 			if (!tmpIndex.renameTo(cacheFile))
 				throw new IOException(
 						JGitText.get().couldNotRenameTemporaryIndexFileToIndex);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index b8d433762e..cb5c8bda54 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -148,6 +148,13 @@ public abstract class FS {
 		return userHome;
 	}
 
+	/**
+	 * Does this file system have problems with atomic renames?
+	 *
+	 * @return true if the caller should retry a failed rename of a lock file.
+	 */
+	public abstract boolean retryFailedLockFileCommit();
+
 	/**
 	 * Determine the user's home directory (location where preferences are).
 	 *
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java5.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java5.java
index 4ce0366fc8..950d49189c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java5.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java5.java
@@ -57,4 +57,9 @@ class FS_POSIX_Java5 extends FS {
 	public boolean setExecute(final File f, final boolean canExec) {
 		return false;
 	}
+
+	@Override
+	public boolean retryFailedLockFileCommit() {
+		return false;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java6.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java6.java
index 8a86d2e65f..192d9bbbbf 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java6.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX_Java6.java
@@ -104,4 +104,9 @@ class FS_POSIX_Java6 extends FS {
 			throw new Error(e);
 		}
 	}
+
+	@Override
+	public boolean retryFailedLockFileCommit() {
+		return false;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
index c86f3e1e43..4e35e6ee4b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
@@ -71,4 +71,9 @@ class FS_Win32 extends FS {
 	public boolean setExecute(final File f, final boolean canExec) {
 		return false;
 	}
+
+	@Override
+	public boolean retryFailedLockFileCommit() {
+		return true;
+	}
 }