Remove stray files (probes or lock files) created by background threads
On process exit, it was possible that the filesystem timestamp
resolution measurement left behind .probe files or even a lock file
for the jgit.config.
Ensure the SAVE_RUNNER is shut down when the process exits (via
System.exit() or otherwise). Move lf.lock() into the try-finally
block when saving the config file.
Delete .probe files on JVM shutdown -- they are created in daemon
threads that may terminate abruptly, not executing the "finally"
clause that normally removes these files.
Bug: 579445
Change-Id: Ie27aca1fdaddfa487ebe072c3913d78355910df8
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/LockFileTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/LockFileTest.java
index 509935d..7eab1dc 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/LockFileTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/LockFileTest.java
@@ -200,4 +200,16 @@
assertFalse(lock.isLocked());
checkFile(f, "contentother");
}
+
+ @Test
+ public void testUnlockNoop() throws Exception {
+ File f = writeTrashFile("somefile", "content");
+ try {
+ LockFile lock = new LockFile(f);
+ lock.unlock();
+ lock.unlock();
+ } catch (Throwable e) {
+ fail("unlock should be noop if not locked at all.");
+ }
+ }
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
index 567e409..cba5e16 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
@@ -180,9 +180,10 @@
}
final LockFile lf = new LockFile(getFile());
- if (!lf.lock())
- throw new LockFailedException(getFile());
try {
+ if (!lf.lock()) {
+ throw new LockFailedException(getFile());
+ }
lf.setNeedSnapshotNoConfig(true);
lf.write(out);
if (!lf.commit())
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index 9237c0a..e8f38d8 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -47,7 +47,6 @@
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingQueue;
@@ -262,8 +261,9 @@
*
* @see java.util.concurrent.Executors#newCachedThreadPool()
*/
- private static final Executor FUTURE_RUNNER = new ThreadPoolExecutor(0,
- 5, 30L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),
+ private static final ExecutorService FUTURE_RUNNER = new ThreadPoolExecutor(
+ 0, 5, 30L, TimeUnit.SECONDS,
+ new LinkedBlockingQueue<Runnable>(),
runnable -> {
Thread t = new Thread(runnable,
"JGit-FileStoreAttributeReader-" //$NON-NLS-1$
@@ -285,8 +285,9 @@
* small keep-alive time to avoid delays on shut-down.
* </p>
*/
- private static final Executor SAVE_RUNNER = new ThreadPoolExecutor(0, 1,
- 1L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>(),
+ private static final ExecutorService SAVE_RUNNER = new ThreadPoolExecutor(
+ 0, 1, 1L, TimeUnit.MILLISECONDS,
+ new LinkedBlockingQueue<Runnable>(),
runnable -> {
Thread t = new Thread(runnable,
"JGit-FileStoreAttributeWriter-" //$NON-NLS-1$
@@ -296,6 +297,18 @@
return t;
});
+ static {
+ // Shut down the SAVE_RUNNER on System.exit()
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+ try {
+ SAVE_RUNNER.shutdownNow();
+ SAVE_RUNNER.awaitTermination(100, TimeUnit.MILLISECONDS);
+ } catch (Exception e) {
+ // Ignore; we're shutting down
+ }
+ }));
+ }
+
/**
* Whether FileStore attributes should be determined asynchronously
*
@@ -452,11 +465,13 @@
return null;
}
// fall through and return fallback
- } catch (IOException | InterruptedException
- | ExecutionException | CancellationException e) {
+ } catch (IOException | ExecutionException | CancellationException e) {
LOG.error(e.getMessage(), e);
} catch (TimeoutException | SecurityException e) {
// use fallback
+ } catch (InterruptedException e) {
+ LOG.error(e.getMessage(), e);
+ Thread.currentThread().interrupt();
}
LOG.debug("{}: use fallback timestamp resolution for directory {}", //$NON-NLS-1$
Thread.currentThread(), dir);
@@ -474,6 +489,7 @@
Path probe = dir.resolve(".probe-" + UUID.randomUUID()); //$NON-NLS-1$
Instant end = Instant.now().plusSeconds(3);
try {
+ probe.toFile().deleteOnExit();
Files.createFile(probe);
do {
n++;
@@ -540,6 +556,7 @@
}
Path probe = dir.resolve(".probe-" + UUID.randomUUID()); //$NON-NLS-1$
try {
+ probe.toFile().deleteOnExit();
Files.createFile(probe);
Duration fsResolution = getFsResolution(s, dir, probe);
Duration clockResolution = measureClockResolution();
