Fixed probe workspace cleanup

- Probe cleanup handles files created during traversal with bounded rescans
- Filesystem failures remain visible
- Symlink targets remain untouched
This commit is contained in:
Brian Neumann-Fopiano committed 2026-10-13 12:28:58 -04:00
commit 6ff13d11c3
5 files changed
+206 -32

No files matched your search

+1
View File
@@ -17,6 +17,7 @@ dependencies {
implementation(project(':core'))
compileOnly(libs.spigot)
testImplementation('junit:junit:4.13.2')
testImplementation('org.mockito:mockito-core:5.23.0')
testImplementation(libs.gson)
testImplementation(testFixtures(project(':core')))
testImplementation('com.github.VolmitSoftware.VolmLib:volmlib-shared:master-SNAPSHOT') {
@@ -56,7 +56,6 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ExecutorService;
import java.util.stream.Stream;
public final class GenerationProbe {
private static final List<Throwable> REPORTED = Collections.synchronizedList(new ArrayList<>());
@@ -564,7 +563,7 @@ public final class GenerationProbe {
}
}
try {
deleteRecursively(workRoot.toPath());
ProbeWorkspaceCleanup.deleteRecursively(workRoot.toPath());
} catch (Throwable e) {
failure = appendFailure(failure, e);
}
@@ -581,19 +580,6 @@ public final class GenerationProbe {
return failure;
}
private static void deleteRecursively(Path root) throws Exception {
if (!Files.exists(root)) {
return;
}
List<Path> paths;
try (Stream<Path> walk = Files.walk(root)) {
paths = walk.sorted(Comparator.reverseOrder()).toList();
}
for (Path path : paths) {
Files.deleteIfExists(path);
}
}
private static List<Throwable> settleAndDrain() throws InterruptedException {
List<Throwable> drained = new ArrayList<>();
long quietSince = System.currentTimeMillis();
@@ -0,0 +1,54 @@
package art.arcane.iris.probe;
import java.io.IOException;
import java.nio.file.DirectoryNotEmptyException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
final class ProbeWorkspaceCleanup {
private static final int MAXIMUM_ATTEMPTS = 4;
private static final SimpleFileVisitor<Path> DELETE_VISITOR = new SimpleFileVisitor<>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attributes) throws IOException {
Files.deleteIfExists(file);
return FileVisitResult.CONTINUE;
}
@Override
public FileVisitResult visitFileFailed(Path file, IOException failure) throws IOException {
if (failure instanceof NoSuchFileException) {
return FileVisitResult.CONTINUE;
}
throw failure;
}
@Override
public FileVisitResult postVisitDirectory(Path directory, IOException failure) throws IOException {
if (failure != null) {
throw failure;
}
Files.deleteIfExists(directory);
return FileVisitResult.CONTINUE;
}
};
private ProbeWorkspaceCleanup() {
}
static void deleteRecursively(Path root) throws IOException {
for (int attempt = 1; ; attempt++) {
try {
Files.walkFileTree(root, DELETE_VISITOR);
return;
} catch (DirectoryNotEmptyException failure) {
if (attempt == MAXIMUM_ATTEMPTS) {
throw failure;
}
}
}
}
}
@@ -43,7 +43,6 @@ import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -56,7 +55,6 @@ import java.util.concurrent.ForkJoinTask;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.locks.LockSupport;
import java.util.stream.Stream;
final class RealPackProbeSupport {
private static final List<Throwable> REPORTED = Collections.synchronizedList(new ArrayList<>());
@@ -616,7 +614,7 @@ final class RealPackProbeSupport {
if (clone.waitFor() == 0) {
return destination;
}
deleteRecursively(destination.toPath());
ProbeWorkspaceCleanup.deleteRecursively(destination.toPath());
}
Path sourcePath = source.toPath();
Files.walkFileTree(sourcePath, EnumSet.of(FileVisitOption.FOLLOW_LINKS), Integer.MAX_VALUE,
@@ -657,26 +655,13 @@ final class RealPackProbeSupport {
private static Throwable deleteWorkRoot(File workRoot) {
try {
deleteRecursively(workRoot.toPath());
ProbeWorkspaceCleanup.deleteRecursively(workRoot.toPath());
return null;
} catch (Throwable failure) {
return failure;
}
}
private static void deleteRecursively(Path root) throws Exception {
if (!Files.exists(root)) {
return;
}
List<Path> paths;
try (Stream<Path> walk = Files.walk(root)) {
paths = walk.sorted(Comparator.reverseOrder()).toList();
}
for (Path path : paths) {
Files.deleteIfExists(path);
}
}
private static Throwable appendFailure(Throwable failure, Throwable additional) {
if (failure == null) {
return additional;
@@ -0,0 +1,148 @@
package art.arcane.iris.probe;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.mockito.MockedStatic;
import java.nio.file.AccessDeniedException;
import java.nio.file.DirectoryNotEmptyException;
import java.nio.file.FileSystemException;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.util.Locale;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeNoException;
import static org.mockito.Mockito.mockStatic;
public final class ProbeWorkspaceCleanupTest {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
@Test
public void deletesNestedContentsAndAcceptsMissingRoots() throws Exception {
Path root = temporaryFolder.newFolder("workspace").toPath();
Files.writeString(Files.createDirectories(root.resolve("pack/regions")).resolve("region.json"), "{}");
ProbeWorkspaceCleanup.deleteRecursively(root);
ProbeWorkspaceCleanup.deleteRecursively(root);
assertFalse(Files.exists(root));
}
@Test
public void rescansWhenAnotherThreadCreatesMetadataAfterDirectoryTraversal() throws Exception {
Path root = temporaryFolder.newFolder("late-metadata").toPath();
Path nested = Files.createDirectory(root.resolve("pack"));
CountDownLatch deletingDirectory = new CountDownLatch(1);
CountDownLatch metadataWritten = new CountDownLatch(1);
ExecutorService writer = Executors.newSingleThreadExecutor();
AtomicInteger attempts = new AtomicInteger();
try (MockedStatic<Files> files = mockStatic(Files.class, invocation -> {
if (invocation.getMethod().getName().equals("deleteIfExists") && nested.equals(invocation.getArgument(0))) {
if (attempts.incrementAndGet() == 1) {
deletingDirectory.countDown();
assertTrue(metadataWritten.await(5, TimeUnit.SECONDS));
}
}
return invocation.callRealMethod();
})) {
Future<?> created = writer.submit(() -> {
assertTrue(deletingDirectory.await(5, TimeUnit.SECONDS));
Files.writeString(nested.resolve(".DS_Store"), "metadata");
metadataWritten.countDown();
return null;
});
ProbeWorkspaceCleanup.deleteRecursively(root);
created.get(5, TimeUnit.SECONDS);
} finally {
deletingDirectory.countDown();
writer.shutdownNow();
}
assertEquals(2, attempts.get());
assertFalse(Files.exists(root));
}
@Test
public void stopsRescanningWhenTheDirectoryKeepsChanging() throws Exception {
Path root = temporaryFolder.newFolder("continuing-writes").toPath();
AtomicInteger attempts = new AtomicInteger();
try (MockedStatic<Files> files = mockStatic(Files.class, invocation -> {
if (invocation.getMethod().getName().equals("deleteIfExists") && root.equals(invocation.getArgument(0))) {
attempts.incrementAndGet();
Files.writeString(root.resolve("late-file"), "content");
}
return invocation.callRealMethod();
})) {
assertThrows(DirectoryNotEmptyException.class, () -> ProbeWorkspaceCleanup.deleteRecursively(root));
}
assertEquals(4, attempts.get());
assertTrue(Files.exists(root.resolve("late-file")));
}
@Test
public void propagatesOtherIoFailuresWithoutRetrying() throws Exception {
Path root = temporaryFolder.newFolder("denied").toPath();
AccessDeniedException failure = new AccessDeniedException(root.toString());
AtomicInteger attempts = new AtomicInteger();
try (MockedStatic<Files> files = mockStatic(Files.class, invocation -> {
if (invocation.getMethod().getName().equals("deleteIfExists") && root.equals(invocation.getArgument(0))) {
attempts.incrementAndGet();
throw failure;
}
return invocation.callRealMethod();
})) {
assertSame(failure, assertThrows(AccessDeniedException.class,
() -> ProbeWorkspaceCleanup.deleteRecursively(root)));
}
assertEquals(1, attempts.get());
}
@Test
public void deletesLinksWithoutFollowingTheirTargets() throws Exception {
Path outside = temporaryFolder.newFolder("outside").toPath();
Path retained = Files.writeString(outside.resolve("retained.txt"), "retained");
Path root = temporaryFolder.newFolder("linked-workspace").toPath();
createSymbolicLink(root.resolve("outside"), outside);
createSymbolicLink(root.resolve("missing"), outside.resolve("missing"));
ProbeWorkspaceCleanup.deleteRecursively(root);
assertEquals("retained", Files.readString(retained));
assertFalse(Files.exists(root, LinkOption.NOFOLLOW_LINKS));
Path rootLink = temporaryFolder.getRoot().toPath().resolve("root-link");
createSymbolicLink(rootLink, outside);
ProbeWorkspaceCleanup.deleteRecursively(rootLink);
assertEquals("retained", Files.readString(retained));
assertFalse(Files.exists(rootLink, LinkOption.NOFOLLOW_LINKS));
}
private static void createSymbolicLink(Path link, Path target) throws Exception {
try {
Files.createSymbolicLink(link, target);
} catch (UnsupportedOperationException | AccessDeniedException failure) {
assumeNoException(failure);
} catch (FileSystemException failure) {
if (!System.getProperty("os.name").startsWith("Windows") || failure.getReason() == null
|| !failure.getReason().toLowerCase(Locale.ROOT).contains("privilege")) {
throw failure;
}
assumeNoException(failure);
}
}
}