From e99c48a61a835149133fca57be117dc9248f5cd2 Mon Sep 17 00:00:00 2001 From: Jens Baumgart Date: Wed, 21 Jul 2010 09:35:15 +0200 Subject: [PATCH] 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 --- .../src/org/eclipse/jgit/lib/GitIndex.java | 30 ++++++++++++++++--- .../src/org/eclipse/jgit/util/FS.java | 7 +++++ .../org/eclipse/jgit/util/FS_POSIX_Java5.java | 5 ++++ .../org/eclipse/jgit/util/FS_POSIX_Java6.java | 5 ++++ .../src/org/eclipse/jgit/util/FS_Win32.java | 5 ++++ 5 files changed, 48 insertions(+), 4 deletions(-) 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 defda148c..df4906374 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 void write() throws IOException { 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 b8d433762..cb5c8bda5 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 File userHome() { 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 4ce0366fc..950d49189 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 @@ public boolean canExecute(final File f) { 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 8a86d2e65..192d9bbbb 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 @@ public boolean setExecute(final File f, final boolean canExec) { 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 c86f3e1e4..4e35e6ee4 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 @@ public boolean canExecute(final File f) { public boolean setExecute(final File f, final boolean canExec) { return false; } + + @Override + public boolean retryFailedLockFileCommit() { + return true; + } }