motiejus
/
jgit
1
Fork 0
Code Releases Activity

Remove stray files (probes or lock files) created by background threads 

NOTE: port back from master branch.

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: Iaee2301eb14e6201406398a90228ad10cfea6098
This commit is contained in:
James Z.M. Gao 2022-04-08 00:29:39 +08:00 committed by Matthias Sohn
parent 78c9b9260a
commit d67ac798f1
3 changed files with 39 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/LockFileTest.java
View File

@ -200,4 +200,16 @@ public void testLockForAppend() throws Exception {
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.");
}
}
}

5
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
View File

@ -216,9 +216,10 @@ public void save() throws IOException {
}
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())

31
org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
View File

@ -48,7 +48,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;
@ -263,8 +262,9 @@ public static final class FileStoreAttributes {
*
* @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$
@ -286,8 +286,9 @@ public static final class FileStoreAttributes {
* 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$
@ -297,6 +298,18 @@ public static final class FileStoreAttributes {
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
*
@ -453,11 +466,13 @@ private static FileStoreAttributes getFileStoreAttributes(Path dir) {
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);
@ -475,6 +490,7 @@ private static Duration measureMinimalRacyInterval(Path dir) {
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++;
@ -541,6 +557,7 @@ private static Optional<Duration> measureFsTimestampResolution(
}
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();