Fix GC for FileRepo in case packfile renames fail

Only on Windows the rename operation which renames temporary Packfiles
(and index-files and bitmap-files) sometime fails. This happens only
when renaming a temporary Packfile to a Packfile which already exists.
Such situations occur if you run GC twice on a repo without modifying
the repo inbetween.

In such situations there was bug in GC which led to a corrupted repo
whithout any packfiles anymore. This commit fixes the problem by
introducing a utility method which renames a file and throws an
IOException if it fails. This method also takes care to repeat a
failing rename if our FS class has found out we are running on a
platform with a unreliable File.renameTo() method.

I am searching for a better solution because even with this utility
method in hand a GC on a already GC'ed repo will fail on Windows. But
at least with this fix we will not produce corrupted repos anymore.

Bug: 389305
Change-Id: Iac1ab3e0b8c419c90404f2e2f3559672eb8f6d28
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java

@@ -435,6 +435,13 @@ public void testPackAllObjectsInOnePack() throws Exception {
 		assertEquals(0, stats.numberOfLooseObjects);
 		assertEquals(4, stats.numberOfPackedObjects);
 		assertEquals(1, stats.numberOfPackFiles);
+
+		// Do the gc again and check that it hasn't changed anything
+		gc.gc();
+		stats = gc.getStatistics();
+		assertEquals(0, stats.numberOfLooseObjects);
+		assertEquals(4, stats.numberOfPackedObjects);
+		assertEquals(1, stats.numberOfPackFiles);
 	}
 
 	@Test

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java

@@ -735,11 +735,21 @@ public int compare(PackExt o1, PackExt o2) {
 
 			// rename the temporary files to real files
 			File realPack = nameFor(id, ".pack"); //$NON-NLS-1$
+
+			// if the packfile already exists (because we are rewriting a
+			// packfile for the same set of objects maybe with different
+			// PackConfig) then make sure we get rid of all handles on the file.
+			// Windows will not allow for rename otherwise.
+			if (realPack.exists())
+				for (PackFile p : repo.getObjectDatabase().getPacks())
+					if (realPack.getPath().equals(p.getPackFile().getPath())) {
+						p.close();
+						break;
+					}
 			tmpPack.setReadOnly();
 			boolean delete = true;
 			try {
-				if (!tmpPack.renameTo(realPack))
-					return null;
+				FileUtils.rename(tmpPack, realPack);
 				delete = false;
 				for (Map.Entry<PackExt, File> tmpEntry : tmpExts.entrySet()) {
 					File tmpExt = tmpEntry.getValue();
@@ -747,7 +757,9 @@ public int compare(PackExt o1, PackExt o2) {
 
 					File realExt = nameFor(
 							id, "." + tmpEntry.getKey().getExtension()); //$NON-NLS-1$
-					if (!tmpExt.renameTo(realExt)) {
+					try {
+						FileUtils.rename(tmpExt, realExt);
+					} catch (IOException e) {
 						File newExt = new File(realExt.getParentFile(),
 								realExt.getName() + ".new"); //$NON-NLS-1$
 						if (!tmpExt.renameTo(newExt))

org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java

@@ -167,6 +167,39 @@ public static void delete(final File f, int options) throws IOException {
 		}
 	}
 
+	/**
+	 * Rename a file or folder. If the rename fails and if we are running on a
+	 * filesystem where it makes sense to repeat a failing rename then repeat
+	 * the rename operation up to 9 times with 100ms sleep time between two
+	 * calls
+	 *
+	 * @see FS#retryFailedLockFileCommit()
+	 * @param src
+	 *            the old {@code File}
+	 * @param dst
+	 *            the new {@code File}
+	 * @throws IOException
+	 *             if the rename has failed
+	 */
+	public static void rename(final File src, final File dst)
+			throws IOException {
+		int attempts = FS.DETECTED.retryFailedLockFileCommit() ? 10 : 1;
+		while (--attempts >= 0) {
+			if (src.renameTo(dst))
+				return;
+			try {
+				Thread.sleep(100);
+			} catch (InterruptedException e) {
+				throw new IOException(MessageFormat.format(
+						JGitText.get().renameFileFailed, src.getAbsolutePath(),
+						dst.getAbsolutePath()));
+			}
+		}
+		throw new IOException(MessageFormat.format(
+				JGitText.get().renameFileFailed, src.getAbsolutePath(),
+				dst.getAbsolutePath()));
+	}
+
 	/**
 	 * Creates the directory named by this abstract pathname.
 	 *