Make GC more robust against corrupt reflogs

With JGit it is possible to write reflog entries where new objectid and
old objectid is null. Such reflogs cause FileRepository GC to crash
because it doesn't expect the new objectid to be null. One case where
this happened is in Gerrit's allProjects repo. In the same way as we
expect the old objectid to be potentially null we should also ignore
null values in the new objectid column.

Change-Id: Icf666c7ef803179b84306ca8deb602369b8df16e

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java
index 8f26f7058d2936bc7d7a8ad13d47cb57aa92fee1..bc60f64886472855d382a1e40c92439f3b2274ea 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GCTest.java
@@ -49,8 +49,8 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
-import java.util.Collection;
 import java.io.IOException;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.Iterator;
@@ -62,6 +62,7 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
 import org.eclipse.jgit.internal.storage.file.GC;
@@ -76,9 +77,9 @@ import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
-import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.junit.TestRepository.CommitBuilder;
 import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.EmptyProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref.Storage;
@@ -436,6 +437,20 @@ public class GCTest extends LocalDiskRepositoryTestCase {
                assertEquals(1, stats.numberOfPackFiles);
        }
 
+       @Test
+       public void testPackRepoWithCorruptReflog() throws Exception {
+               // create a reflog entry "0000... 0000... foobar" by doing an initial
+               // refupdate for HEAD which points to a non-existing ref. The
+               // All-Projects repo of gerrit instances had such entries
+               RefUpdate ru = repo.updateRef(Constants.HEAD);
+               ru.link("refs/to/garbage");
+               tr.branch("refs/heads/master").commit().add("A", "A").add("B", "B")
+                               .create();
+               // make sure HEAD exists
+               Git.wrap(repo).checkout().setName("refs/heads/master").call();
+               gc.gc();
+       }
+
        @Test
        public void testKeepFiles() throws Exception {
                BranchBuilder bb = tr.branch("refs/heads/master");
@@ -477,9 +492,10 @@ public class GCTest extends LocalDiskRepositoryTestCase {
                assertEquals(4, ind2.getObjectCount());
                for (MutableEntry e: ind1)
                        if (ind2.hasObject(e.toObjectId()))
-                       assertFalse(
-                                       "the following object is in both packfiles: "
-                                                       + e.toObjectId(), ind2.hasObject(e.toObjectId()));
+                               assertFalse(
+                                               "the following object is in both packfiles: "
+                                                               + e.toObjectId(),
+                                               ind2.hasObject(e.toObjectId()));
        }
 
        @Test

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
index 67bb664b5bf382972c3e8f28838d252fe5fc4b1a..22fa827044ba1dfb09445fd62f1f3de26239ceeb 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
@@ -555,7 +555,9 @@ public class GC {
                for (ReflogEntry e : rlEntries) {
                        if (e.getWho().getWhen().getTime() < minTime)
                                break;
-                       ret.add(e.getNewId());
+                       ObjectId newId = e.getNewId();
+                       if (newId != null && !ObjectId.zeroId().equals(newId))
+                               ret.add(newId);
                        ObjectId oldId = e.getOldId();
                        if (oldId != null && !ObjectId.zeroId().equals(oldId))
                                ret.add(oldId);