From a6da439b47bd2d7c3772f4a94d199894d3edaee9 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 02:42:32 +0100 Subject: Check if FileLock is valid before using or releasing it Change-Id: I23ba67b61b9b03772f33a929c080c0d02b8c8652 --- org.eclipse.jgit/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 3f281a5d6c..04c8418f6d 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 @@ public class GC { 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 @@ public class GC { public void close() { boolean wasLocked = false; try { - if (lock != null) { + if (lock != null && lock.isValid()) { lock.release(); wasLocked = true; } -- cgit v1.2.3 From 49f527386723ff219de0c726d74f200d89c959d7 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 19:27:30 +0100 Subject: 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 --- org.eclipse.jgit/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 04c8418f6d..7feb50688a 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 @@ public class GC { 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 @@ public class GC { JGitText.get().closePidLockFailed, pidFile), e1); } - return false; + throw e; } return true; } -- cgit v1.2.3 From 1691e38779d1f1d25cc6c388a91a82667ff12b34 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 20:35:01 +0100 Subject: Fix GcConcurrentTest#testInterruptGc With the new GC.PidLock interrupting a running GC throws a ClosedByInterruptException. Change-Id: I7ccea1ae9a43d4edfdab2fcfd1324c64cc22b38f --- .../org/eclipse/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 5cac1e3429..d53d5eb4b8 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 java.lang.Integer.valueOf; 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 class GcConcurrentTest extends GcTestCase { 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); } -- cgit v1.2.3 From d9f75e8bb2af7307ce1d6e0a7376eb5ebe583ae4 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 22 Feb 2023 20:36:39 +0100 Subject: If tryLock fails to get the lock another gc has it Change-Id: Ifd3bbcc5e0591883b774d23256949a83010ea134 --- org.eclipse.jgit/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 7feb50688a..f25feee8f5 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 @@ public class GC { channel = f.getChannel(); lock = channel.tryLock(); if (lock == null || !lock.isValid()) { - failedToLock(); + gcAlreadyRunning(); return false; } channel.write(ByteBuffer -- cgit v1.2.3