Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHan-Wen Nienhuys2019-08-31 19:33:58 +0000
committerMatthias Sohn2019-09-03 08:37:30 +0000
commit0e3d4a273f3d5b79c55205a25f21d688b6f131fc (patch)
treeaff2fa6e991a32c7a2528849b994eb9f577ed211 /org.eclipse.jgit.test/tst
parent400342bbc1cff644ee9d7fd1076e62be89bb54f4 (diff)
downloadjgit-0e3d4a273f3d5b79c55205a25f21d688b6f131fc.tar.gz
jgit-0e3d4a273f3d5b79c55205a25f21d688b6f131fc.tar.xz
jgit-0e3d4a273f3d5b79c55205a25f21d688b6f131fc.zip
BatchRefUpdate: repro racy atomic update, and fix it
PackedBatchRefUpdate was creating a new packed-refs list that was potentially unsorted. This would be papered over when the list was read back from disk in parsePackedRef, which detects unsorted ref lists on reading, and sorts them. However, the BatchRefUpdate also installed the new (unsorted) list in-memory in RefDirectory#packedRefs. With the timestamp granularity code committed to stable-5.1, we can more often accurately decide that the packed-refs file is clean, and will return the erroneous unsorted data more often. Unluckily timed delays also cause the file to be clean, hence this problem was exacerbated under load. The symptom is that refs added by a BatchRefUpdate would stop being visible directly after they were added. In particular, the Gerrit integration tests uses BatchRefUpdate in its setup for creating the Admin group, and then tries to read it out directly afterward. The tests recreates one failure case. A better approach would be to revise RefList.Builder, so it detects out-of-order lists and automatically sorts them. Fixes https://bugs.eclipse.org/bugs/show_bug.cgi?id=548716 and https://bugs.chromium.org/p/gerrit/issues/detail?id=11373. Bug: 548716 Change-Id: I613c8059964513ce2370543620725b540b3cb6d1 Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Diffstat (limited to 'org.eclipse.jgit.test/tst')
-rw-r--r--org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java29
1 files changed, 29 insertions, 0 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java
index 3c4b8cf4bc..2ac4a846ea 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java
@@ -43,6 +43,7 @@
package org.eclipse.jgit.internal.storage.file;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.eclipse.jgit.internal.storage.file.BatchRefUpdateTest.Result.LOCK_FAILURE;
@@ -64,6 +65,7 @@ import static org.junit.Assume.assumeTrue;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Files;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -162,6 +164,33 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase {
}
@Test
+ public void packedRefsFileIsSorted() throws IOException {
+ assumeTrue(atomic);
+
+ for (int i = 0; i < 2; i++) {
+ BatchRefUpdate bu = diskRepo.getRefDatabase().newBatchUpdate();
+ String b1 = String.format("refs/heads/a%d",i);
+ String b2 = String.format("refs/heads/b%d",i);
+ bu.setAtomic(atomic);
+ ReceiveCommand c1 = new ReceiveCommand(ObjectId.zeroId(), A, b1);
+ ReceiveCommand c2 = new ReceiveCommand(ObjectId.zeroId(), B, b2);
+ bu.addCommand(c1, c2);
+ try (RevWalk rw = new RevWalk(diskRepo)) {
+ bu.execute(rw, NullProgressMonitor.INSTANCE);
+ }
+ assertEquals(c1.getResult(), ReceiveCommand.Result.OK);
+ assertEquals(c2.getResult(), ReceiveCommand.Result.OK);
+ }
+
+ File packed = new File(diskRepo.getDirectory(), "packed-refs");
+ String packedStr = new String(Files.readAllBytes(packed.toPath()), UTF_8);
+
+ int a2 = packedStr.indexOf("refs/heads/a1");
+ int b1 = packedStr.indexOf("refs/heads/b0");
+ assertTrue(a2 < b1);
+ }
+
+ @Test
public void simpleNoForce() throws IOException {
writeLooseRef("refs/heads/master", A);
writeLooseRef("refs/heads/masters", B);

Back to the top