diff --git a/probe/build.gradle b/probe/build.gradle index d9dbc22ec..bc874dddb 100644 --- a/probe/build.gradle +++ b/probe/build.gradle @@ -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') { diff --git a/probe/src/main/java/art/arcane/iris/probe/GenerationProbe.java b/probe/src/main/java/art/arcane/iris/probe/GenerationProbe.java index c14475859..38c63808c 100644 --- a/probe/src/main/java/art/arcane/iris/probe/GenerationProbe.java +++ b/probe/src/main/java/art/arcane/iris/probe/GenerationProbe.java @@ -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 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 paths; - try (Stream walk = Files.walk(root)) { - paths = walk.sorted(Comparator.reverseOrder()).toList(); - } - for (Path path : paths) { - Files.deleteIfExists(path); - } - } - private static List settleAndDrain() throws InterruptedException { List drained = new ArrayList<>(); long quietSince = System.currentTimeMillis(); diff --git a/probe/src/main/java/art/arcane/iris/probe/ProbeWorkspaceCleanup.java b/probe/src/main/java/art/arcane/iris/probe/ProbeWorkspaceCleanup.java new file mode 100644 index 000000000..0550b587c --- /dev/null +++ b/probe/src/main/java/art/arcane/iris/probe/ProbeWorkspaceCleanup.java @@ -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 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; + } + } + } + } +} diff --git a/probe/src/main/java/art/arcane/iris/probe/RealPackProbeSupport.java b/probe/src/main/java/art/arcane/iris/probe/RealPackProbeSupport.java index 41a23172d..c78adf08c 100644 --- a/probe/src/main/java/art/arcane/iris/probe/RealPackProbeSupport.java +++ b/probe/src/main/java/art/arcane/iris/probe/RealPackProbeSupport.java @@ -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 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 paths; - try (Stream 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; diff --git a/probe/src/test/java/art/arcane/iris/probe/ProbeWorkspaceCleanupTest.java b/probe/src/test/java/art/arcane/iris/probe/ProbeWorkspaceCleanupTest.java new file mode 100644 index 000000000..383d44439 --- /dev/null +++ b/probe/src/test/java/art/arcane/iris/probe/ProbeWorkspaceCleanupTest.java @@ -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 = 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 = 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 = 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); + } + } +}