From a6da439b47bd2d7c3772f4a94d199894d3edaee9 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 02:42:32 +0100 Subject: [PATCH 1/4] Check if FileLock is valid before using or releasing it Change-Id: I23ba67b61b9b03772f33a929c080c0d02b8c8652 --- .../src/org/eclipse/jgit/internal/storage/file/GC.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 3f281a5d6..04c8418f6 100644 --- 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 @@ -1649,7 +1649,7 @@ boolean lock() { f = new RandomAccessFile(pidFile.toFile(), "rw"); //$NON-NLS-1$ channel = f.getChannel(); lock = channel.tryLock(); - if (lock == null) { + if (lock == null || !lock.isValid()) { failedToLock(); return false; } @@ -1738,7 +1738,7 @@ private String getHostName() { public void close() { boolean wasLocked = false; try { - if (lock != null) { + if (lock != null && lock.isValid()) { lock.release(); wasLocked = true; } From 49f527386723ff219de0c726d74f200d89c959d7 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 19:27:30 +0100 Subject: [PATCH 2/4] Don't swallow IOException in GC.PidLock#lock This broke the test GcConcurrentTest#testInterruptGc which expects ClosedByInterruptException when the thread doing gc is interrupted. Change-Id: I89e02fc37aceeccb04c20cfc5b71cb8fa21793d6 --- .../src/org/eclipse/jgit/internal/storage/file/GC.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 04c8418f6..7feb50688 100644 --- 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 @@ -1631,7 +1631,7 @@ private class PidLock implements AutoCloseable { pidFile = repo.getDirectory().toPath().resolve(GC_PID); } - boolean lock() { + boolean lock() throws IOException { if (Files.exists(pidFile)) { Instant mtime = FS.DETECTED .lastModifiedInstant(pidFile.toFile()); @@ -1670,7 +1670,7 @@ boolean lock() { JGitText.get().closePidLockFailed, pidFile), e1); } - return false; + throw e; } return true; } From 1691e38779d1f1d25cc6c388a91a82667ff12b34 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 20:35:01 +0100 Subject: [PATCH 3/4] Fix GcConcurrentTest#testInterruptGc With the new GC.PidLock interrupting a running GC throws a ClosedByInterruptException. Change-Id: I7ccea1ae9a43d4edfdab2fcfd1324c64cc22b38f --- .../jgit/internal/storage/file/GcConcurrentTest.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java index 5cac1e342..d53d5eb4b 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java @@ -14,10 +14,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; +import java.nio.channels.ClosedByInterruptException; import java.util.Collection; import java.util.Collections; import java.util.concurrent.BrokenBarrierException; @@ -226,10 +226,8 @@ public void testInterruptGc() throws Exception { if (cause instanceof CancelledException) { assertEquals(JGitText.get().operationCanceled, cause.getMessage()); - } else if (cause instanceof IOException) { - Throwable cause2 = cause.getCause(); - assertTrue(cause2 instanceof InterruptedException - || cause2 instanceof ExecutionException); + } else if (cause instanceof ClosedByInterruptException) { + // thread was interrupted } else { fail("unexpected exception " + e); } From d9f75e8bb2af7307ce1d6e0a7376eb5ebe583ae4 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 20:36:39 +0100 Subject: [PATCH 4/4] If tryLock fails to get the lock another gc has it Change-Id: Ifd3bbcc5e0591883b774d23256949a83010ea134 --- .../src/org/eclipse/jgit/internal/storage/file/GC.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7feb50688..f25feee8f 100644 --- 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 @@ -1650,7 +1650,7 @@ boolean lock() throws IOException { channel = f.getChannel(); lock = channel.tryLock(); if (lock == null || !lock.isValid()) { - failedToLock(); + gcAlreadyRunning(); return false; } channel.write(ByteBuffer