From 4bbfbf2c22f37fa0ebd32573ea4b9ba3a9d723d3 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 16 Mar 2023 16:01:44 +0100 Subject: [PATCH 1/6] throw exception mounting an already closed FUSE object --- jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java b/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java index 975af65f..5c328bf9 100644 --- a/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java +++ b/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java @@ -105,7 +105,11 @@ public static FuseBuilder builder() { */ @Blocking @MustBeInvokedByOverriders - public void mount(String progName, Path mountPoint, String... flags) throws FuseMountFailedException, IllegalArgumentException { + public synchronized void mount(String progName, Path mountPoint, String... flags) throws FuseMountFailedException, IllegalArgumentException { + if(!fuseScope.isAlive()) { + throw new IllegalStateException("Already closed"); //TODO: throw specialized exception + } + FuseMount lock = new UnmountedFuseMount(); if (!mount.compareAndSet(UNMOUNTED, lock)) { throw new IllegalStateException("Already mounted"); From 7af37d0954268038175024992f10789b3e5e6b4b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 17 Mar 2023 13:43:52 +0100 Subject: [PATCH 2/6] Fixes #30 Check return value of fuse_loop --- .../java/org/cryptomator/jfuse/api/Fuse.java | 15 ++++++++--- .../org/cryptomator/jfuse/api/FuseTest.java | 25 +++++++++++++++++-- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java b/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java index 5c328bf9..be7a9cc3 100644 --- a/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java +++ b/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java @@ -16,8 +16,10 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -123,19 +125,24 @@ public synchronized void mount(String progName, Path mountPoint, String... flags try { var fuseMount = this.mount(args); - executor.submit(() -> fuseLoop(fuseMount)); // TODO keep reference of future and report result - waitForMountingToComplete(mountPoint); + Future fuseLoop = executor.submit(() -> fuseLoop(fuseMount)); + waitForMountingToComplete(mountPoint, fuseLoop); + if (fuseLoop.isDone()) { + throw new FuseMountFailedException("fuse_loop() returned prematurely with non-zero exit code " + fuseLoop.get()); + } mount.compareAndSet(lock, fuseMount); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new FuseMountFailedException("Interrupted while waiting for mounting to finish"); + } catch (ExecutionException e) { + throw new FuseMountFailedException("Exception when starting fuse_loop. Message: " + e.getCause().getMessage()); } finally { mount.compareAndSet(lock, UNMOUNTED); // if value is still `lock`, mount has failed. } } @VisibleForTesting - void waitForMountingToComplete(Path mountPoint) throws InterruptedException { + void waitForMountingToComplete(Path mountPoint, Future fuseLoop) throws InterruptedException { var probe = Files.getFileAttributeView(mountPoint.resolve(MOUNT_PROBE.substring(1)), BasicFileAttributeView.class); do { try { @@ -143,7 +150,7 @@ void waitForMountingToComplete(Path mountPoint) throws InterruptedException { } catch (IOException e) { // noop } - } while (!mountProbeSucceeded.await(200, TimeUnit.MILLISECONDS)); + } while (!fuseLoop.isDone() && !mountProbeSucceeded.await(200, TimeUnit.MILLISECONDS)); } @Blocking diff --git a/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java b/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java index 2fb864d8..7af9d7df 100644 --- a/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java +++ b/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java @@ -13,6 +13,7 @@ import java.nio.file.spi.FileSystemProvider; import java.time.Duration; import java.util.List; +import java.util.concurrent.Future; public class FuseTest { @@ -39,10 +40,30 @@ public void testWaitForMountingToComplete() throws IOException { fuse.fuseOperations.getattr("/jfuse_mount_probe", Mockito.mock(Stat.class), Mockito.mock(FileInfo.class)); throw new NoSuchFileException("/mnt/jfuse_mount_probe still not found"); }).when(attrView).readAttributes(); + Future fuseLoop = Mockito.mock(Future.class); + Mockito.doReturn(false).when(fuseLoop).isDone(); - Assertions.assertTimeoutPreemptively(Duration.ofSeconds(1), () -> fuse.waitForMountingToComplete(mountPoint)); + Assertions.assertTimeoutPreemptively(Duration.ofSeconds(1), () -> fuse.waitForMountingToComplete(mountPoint, fuseLoop)); + Mockito.verify(fuseLoop, Mockito.atLeastOnce()).isDone(); + } + + @Test + @DisplayName("waitForMountingToComplete() waits returns immediately if fuse_loop fails") + public void testPrematurelyFuseLoopReturn() throws IOException { + Path mountPoint = Mockito.mock(Path.class, "/mnt"); + Path probePath = Mockito.mock(Path.class, "/mnt/jfuse_mount_probe"); + FileSystem fs = Mockito.mock(FileSystem.class); + FileSystemProvider fsProv = Mockito.mock(FileSystemProvider.class); + BasicFileAttributeView attrView = Mockito.mock(BasicFileAttributeView.class); + Mockito.doReturn(probePath).when(mountPoint).resolve("jfuse_mount_probe"); + Mockito.doReturn(fs).when(probePath).getFileSystem(); + Mockito.doReturn(fsProv).when(fs).provider(); + Mockito.doReturn(attrView).when(fsProv).getFileAttributeView(probePath, BasicFileAttributeView.class); + Future fuseLoop = Mockito.mock(Future.class); + Mockito.doReturn(true).when(fuseLoop).isDone(); - Mockito.verify(fuseOps).getattr(Mockito.eq("/jfuse_mount_probe"), Mockito.any(), Mockito.any()); + Assertions.assertTimeoutPreemptively(Duration.ofSeconds(1), () -> fuse.waitForMountingToComplete(mountPoint, fuseLoop)); + Mockito.verify(fuseLoop, Mockito.atLeastOnce()).isDone(); } private static class FuseStub extends Fuse { From e683bd363d64590b037346553381aef6c361f52a Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 17 Mar 2023 13:51:50 +0100 Subject: [PATCH 3/6] clean-up --- jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java b/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java index be7a9cc3..61f6ced4 100644 --- a/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java +++ b/jfuse-api/src/main/java/org/cryptomator/jfuse/api/Fuse.java @@ -108,7 +108,7 @@ public static FuseBuilder builder() { @Blocking @MustBeInvokedByOverriders public synchronized void mount(String progName, Path mountPoint, String... flags) throws FuseMountFailedException, IllegalArgumentException { - if(!fuseScope.isAlive()) { + if (!fuseScope.isAlive()) { throw new IllegalStateException("Already closed"); //TODO: throw specialized exception } From 1939ecb6f7df8d55dd28e21cd71815208c04b74b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 22 Mar 2023 12:11:58 +0100 Subject: [PATCH 4/6] match mount method match super method regarding synchronization --- jfuse-win/src/main/java/org/cryptomator/jfuse/win/FuseImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jfuse-win/src/main/java/org/cryptomator/jfuse/win/FuseImpl.java b/jfuse-win/src/main/java/org/cryptomator/jfuse/win/FuseImpl.java index 166860c9..b0504cea 100644 --- a/jfuse-win/src/main/java/org/cryptomator/jfuse/win/FuseImpl.java +++ b/jfuse-win/src/main/java/org/cryptomator/jfuse/win/FuseImpl.java @@ -30,7 +30,7 @@ public FuseImpl(FuseOperations fuseOperations) { } @Override - public void mount(String progName, Path mountPoint, String... flags) throws FuseMountFailedException { + public synchronized void mount(String progName, Path mountPoint, String... flags) throws FuseMountFailedException { var adjustedMP = mountPoint; if (mountPoint.equals(mountPoint.getRoot()) && mountPoint.isAbsolute()) { //winfsp accepts only drive letters written in drive relative notation From 3b2e2fc129c19a0d18a54904e316dca33414c133 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 22 Mar 2023 13:11:19 +0100 Subject: [PATCH 5/6] add unit tests --- .../org/cryptomator/jfuse/api/FuseTest.java | 73 +++++++++++++------ 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java b/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java index 7af9d7df..fa1ea532 100644 --- a/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java +++ b/jfuse-api/src/test/java/org/cryptomator/jfuse/api/FuseTest.java @@ -17,13 +17,14 @@ public class FuseTest { - private FuseOperations fuseOps = Mockito.mock(FuseOperations.class); - private Fuse fuse = Mockito.spy(new FuseStub(fuseOps)); + private final FuseOperations fuseOps = Mockito.mock(FuseOperations.class); + private final FuseMount fuseMount = Mockito.spy(new FuseMountStub()); + private final Fuse fuse = Mockito.spy(new FuseStub(fuseMount, fuseOps)); + private final Path mountPoint = Mockito.mock(Path.class, "/mnt"); @Test @DisplayName("waitForMountingToComplete() waits for getattr(\"/jfuse_mount_probe\")") public void testWaitForMountingToComplete() throws IOException { - Path mountPoint = Mockito.mock(Path.class, "/mnt"); Path probePath = Mockito.mock(Path.class, "/mnt/jfuse_mount_probe"); FileSystem fs = Mockito.mock(FileSystem.class); FileSystemProvider fsProv = Mockito.mock(FileSystemProvider.class); @@ -50,7 +51,6 @@ public void testWaitForMountingToComplete() throws IOException { @Test @DisplayName("waitForMountingToComplete() waits returns immediately if fuse_loop fails") public void testPrematurelyFuseLoopReturn() throws IOException { - Path mountPoint = Mockito.mock(Path.class, "/mnt"); Path probePath = Mockito.mock(Path.class, "/mnt/jfuse_mount_probe"); FileSystem fs = Mockito.mock(FileSystem.class); FileSystemProvider fsProv = Mockito.mock(FileSystemProvider.class); @@ -66,10 +66,39 @@ public void testPrematurelyFuseLoopReturn() throws IOException { Mockito.verify(fuseLoop, Mockito.atLeastOnce()).isDone(); } + @Test + @DisplayName("Already closed fuseMount throws IllegalStateException on mount") + public void testMountThrowsIllegalStateIfClosed() { + Assertions.assertDoesNotThrow(fuse::close); + Assertions.assertThrows(IllegalStateException.class, () -> fuse.mount("test3000", mountPoint)); + } + + @Test + @DisplayName("Already mounted fuseMount throws IllegalStateException on mount") + public void testMountThrowsIllegalStateIfAlreadyMounted() throws InterruptedException { + Mockito.doNothing().when(fuse).waitForMountingToComplete(Mockito.eq(mountPoint), Mockito.any()); + Assertions.assertDoesNotThrow(() -> fuse.mount("test3000", mountPoint)); + Assertions.assertThrows(IllegalStateException.class, () -> fuse.mount("test3000", mountPoint)); + } + + @Test + @DisplayName("If fuse_loop instantly returns with non-zero result, throw FuseMountFailedException") + public void testMountThrowsFuseMountFailedIfLoopReturnsNonZero() throws InterruptedException { + Mockito.doAnswer(invocation -> { + Thread.sleep(1000); + return null; + }).when(fuse).waitForMountingToComplete(Mockito.eq(mountPoint), Mockito.any()); + Mockito.doReturn(1).when(fuseMount).loop(); + Assertions.assertThrows(FuseMountFailedException.class, () -> fuse.mount("test3000", mountPoint)); + } + private static class FuseStub extends Fuse { - protected FuseStub(FuseOperations fuseOperations) { + FuseMount fuseMount; + + protected FuseStub(FuseMount mountStub, FuseOperations fuseOperations) { super(fuseOperations, allocator -> allocator.allocate(0L)); + this.fuseMount = mountStub; } @Override @@ -79,23 +108,23 @@ protected void bind(FuseOperations.Operation operation) { @Override protected FuseMount mount(List args) { - return new FuseMount() { - - @Override - public int loop() { - return 0; - } - - @Override - public void unmount() { - // no-op - } - - @Override - public void destroy() { - // no-op - } - }; + return fuseMount; + } + } + + private record FuseMountStub() implements FuseMount { + + @Override + public int loop() { + return 0; + } + + @Override + public void unmount() { + } + + @Override + public void destroy() { } } From 601b4dfb3778748ee9f84e4d1148e39136d29b8b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 22 Mar 2023 13:25:53 +0100 Subject: [PATCH 6/6] prepare 0.4.2 --- jfuse-api/pom.xml | 2 +- jfuse-examples/pom.xml | 2 +- jfuse-linux-aarch64/pom.xml | 2 +- jfuse-linux-amd64/pom.xml | 2 +- jfuse-mac/pom.xml | 2 +- jfuse-tests/pom.xml | 2 +- jfuse-win/pom.xml | 2 +- jfuse/pom.xml | 2 +- pom.xml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/jfuse-api/pom.xml b/jfuse-api/pom.xml index b982b7fe..6e4f7489 100644 --- a/jfuse-api/pom.xml +++ b/jfuse-api/pom.xml @@ -5,7 +5,7 @@ org.cryptomator jfuse-parent - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse-api diff --git a/jfuse-examples/pom.xml b/jfuse-examples/pom.xml index 1096434b..15c100df 100644 --- a/jfuse-examples/pom.xml +++ b/jfuse-examples/pom.xml @@ -5,7 +5,7 @@ org.cryptomator jfuse-parent - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse-examples diff --git a/jfuse-linux-aarch64/pom.xml b/jfuse-linux-aarch64/pom.xml index 2c2133a3..e35adde6 100644 --- a/jfuse-linux-aarch64/pom.xml +++ b/jfuse-linux-aarch64/pom.xml @@ -5,7 +5,7 @@ jfuse-parent org.cryptomator - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse-linux-aarch64 diff --git a/jfuse-linux-amd64/pom.xml b/jfuse-linux-amd64/pom.xml index 0e7ba6a1..9259d906 100644 --- a/jfuse-linux-amd64/pom.xml +++ b/jfuse-linux-amd64/pom.xml @@ -5,7 +5,7 @@ org.cryptomator jfuse-parent - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse-linux-amd64 diff --git a/jfuse-mac/pom.xml b/jfuse-mac/pom.xml index 2fdc893a..56c2c0a0 100644 --- a/jfuse-mac/pom.xml +++ b/jfuse-mac/pom.xml @@ -5,7 +5,7 @@ org.cryptomator jfuse-parent - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse-mac diff --git a/jfuse-tests/pom.xml b/jfuse-tests/pom.xml index 4a016e94..6465b8bd 100644 --- a/jfuse-tests/pom.xml +++ b/jfuse-tests/pom.xml @@ -5,7 +5,7 @@ org.cryptomator jfuse-parent - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse-tests diff --git a/jfuse-win/pom.xml b/jfuse-win/pom.xml index 8f836716..2fc80a1a 100644 --- a/jfuse-win/pom.xml +++ b/jfuse-win/pom.xml @@ -5,7 +5,7 @@ org.cryptomator jfuse-parent - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse-win diff --git a/jfuse/pom.xml b/jfuse/pom.xml index 6f0c755a..b826d6ef 100644 --- a/jfuse/pom.xml +++ b/jfuse/pom.xml @@ -5,7 +5,7 @@ org.cryptomator jfuse-parent - 0.5.0-SNAPSHOT + 0.4.2 4.0.0 jfuse diff --git a/pom.xml b/pom.xml index 60439f66..51300262 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.cryptomator jfuse-parent pom - 0.5.0-SNAPSHOT + 0.4.2 jFUSE Java bindings for FUSE using foreign functions & memory API https://github.com/cryptomator/jfuse