From 0b30e3e286731d5eade8d48abd1abad6b2c272ad Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 13 Nov 2023 16:08:45 -0600 Subject: [PATCH 1/6] Don't track mounts for newResource() that doesn't exist. --- .../resource/ResourceFactoryInternals.java | 19 ++++++++++- .../jetty/util/resource/PathResourceTest.java | 32 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java index 1bc7728382f1..2afa99005248 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java @@ -127,7 +127,12 @@ static boolean isSupported(String str) return RESOURCE_FACTORIES.getBest(str) != null; } - static class Closeable implements ResourceFactory.Closeable + interface Mountable + { + List getMounts(); + } + + static class Closeable implements ResourceFactory.Closeable, Mountable { private final CompositeResourceFactory _compositeResourceFactory = new CompositeResourceFactory(); @@ -144,6 +149,11 @@ public void close() IO.close(mount); _compositeResourceFactory.clearMounts(); } + + public List getMounts() + { + return _compositeResourceFactory.getMounts(); + } } static class LifeCycle extends AbstractLifeCycle implements ResourceFactory.LifeCycle @@ -210,8 +220,15 @@ public Resource newResource(URI uri) FileSystemPool.Mount mount = mountIfNeeded(uri); if (mount != null) { + Resource res = resourceFactory.newResource(uri); + if (res == null) + { + mount.close(); // decrement + return null; + } _mounts.add(mount); onMounted(mount, uri); + return res; } } return resourceFactory.newResource(uri); diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 46a7d3709bf5..e0a7d850e00b 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -236,6 +236,38 @@ public void testGetPathTo(WorkDir workDir) throws IOException } } + @Test + public void testJarMountNonExistent(WorkDir workDir) throws IOException + { + Path tmpPath = workDir.getEmptyPathDir(); + Path testJar = tmpPath.resolve("test.jar"); + + Map env = new HashMap<>(); + env.put("create", "true"); + + URI jarUri = URIUtil.uriJarPrefix(testJar.toUri(), "!/"); + try (FileSystem zipfs = FileSystems.newFileSystem(jarUri, env)) + { + Path root = zipfs.getPath("/"); + Files.writeString(root.resolve("test.txt"), "Contents of test.txt", StandardCharsets.UTF_8); + } + + try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable()) + { + Resource resRoot = resourceFactory.newResource(jarUri); + Resource resBadDir = resourceFactory.newResource(jarUri.toASCIIString() + "does-not-exist/"); + assertNull(resBadDir); + Resource resBadFile = resourceFactory.newResource(jarUri.toASCIIString() + "bad/file.txt"); + assertNull(resBadFile); + + if (resourceFactory instanceof ResourceFactoryInternals.Mountable mountable) + { + List mounts = mountable.getMounts(); + assertThat(mounts.size(), is(1)); + } + } + } + @Test public void testJarFileIsAliasFile(WorkDir workDir) throws IOException { From d01b55aa6566fe7b2a8b7cbe5bec763db91c3525 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 13 Nov 2023 16:32:40 -0600 Subject: [PATCH 2/6] Cannot dereference if not tracking both resources that use the JAR. --- .../jetty/util/resource/FileSystemPool.java | 18 +++++++++++++----- .../resource/ResourceFactoryInternals.java | 2 +- .../jetty/util/resource/PathResourceTest.java | 12 ++++++++++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java index 2908a0833773..058600a651cd 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java @@ -108,23 +108,24 @@ Mount mount(URI uri) throws IOException throw new IllegalArgumentException("not an supported scheme: " + uri); FileSystem fileSystem = null; + URI jarURIRoot = toJarURIRoot(uri); try (AutoLock ignore = poolLock.lock()) { try { - fileSystem = FileSystems.newFileSystem(uri, ENV_MULTIRELEASE_RUNTIME); + fileSystem = FileSystems.newFileSystem(jarURIRoot, ENV_MULTIRELEASE_RUNTIME); if (LOG.isDebugEnabled()) - LOG.debug("Mounted new FS {}", uri); + LOG.debug("Mounted new FS {}", jarURIRoot); } catch (FileSystemAlreadyExistsException fsaee) { - fileSystem = Paths.get(uri).getFileSystem(); + fileSystem = Paths.get(jarURIRoot).getFileSystem(); if (LOG.isDebugEnabled()) - LOG.debug("Using existing FS {}", uri); + LOG.debug("Using existing FS {}", jarURIRoot); } catch (ProviderNotFoundException pnfe) { - throw new IllegalArgumentException("Unable to mount FileSystem from unsupported URI: " + uri, pnfe); + throw new IllegalArgumentException("Unable to mount FileSystem from unsupported URI: " + jarURIRoot, pnfe); } // use root FS URI so that pool key/release/sweep is sane URI rootURI = fileSystem.getPath("/").toUri(); @@ -139,6 +140,13 @@ Mount mount(URI uri) throws IOException } } + private URI toJarURIRoot(URI uri) + { + String rawURI = uri.toASCIIString(); + int idx = rawURI.indexOf("!/"); + return URI.create(rawURI.substring(0, idx + 2)); + } + private void unmount(URI fsUri) { try (AutoLock ignore = poolLock.lock()) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java index 2afa99005248..d47477615949 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java @@ -244,7 +244,7 @@ public Resource newResource(URI uri) * * @param uri The URI to mount that may require a FileSystem (e.g. "jar:file://tmp/some.jar!/directory/file.txt") * @return A reference counted {@link FileSystemPool.Mount} for that file system or null. Callers should call - * {@link FileSystemPool.Mount#close()} once they no longer require any resources from a mounted resource. + * {@link FileSystemPool.Mount#close()} once they no longer require this specific Mount. * @throws IllegalArgumentException If the uri could not be mounted. */ private FileSystemPool.Mount mountIfNeeded(URI uri) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index e0a7d850e00b..8e5854bc8cda 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -250,22 +250,30 @@ public void testJarMountNonExistent(WorkDir workDir) throws IOException { Path root = zipfs.getPath("/"); Files.writeString(root.resolve("test.txt"), "Contents of test.txt", StandardCharsets.UTF_8); + + Path dir = root.resolve("datainf"); + Files.createDirectory(dir); + Files.writeString(dir.resolve("info.txt"), "Contents of info.txt", StandardCharsets.UTF_8); } try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable()) { + // First reference to Resource in test.jar Resource resRoot = resourceFactory.newResource(jarUri); Resource resBadDir = resourceFactory.newResource(jarUri.toASCIIString() + "does-not-exist/"); assertNull(resBadDir); Resource resBadFile = resourceFactory.newResource(jarUri.toASCIIString() + "bad/file.txt"); assertNull(resBadFile); + // Second reference to Resource in test.jar + Resource resInfoTxt = resourceFactory.newResource(jarUri.toASCIIString() + "datainf/info.txt"); + assertTrue(Resources.isReadableFile(resInfoTxt)); if (resourceFactory instanceof ResourceFactoryInternals.Mountable mountable) { List mounts = mountable.getMounts(); - assertThat(mounts.size(), is(1)); + assertThat(mounts.size(), is(2)); } - } + } // close dereferences both references to test.jar } @Test From c0e3c5b4de8c1f2e9ff215878d4ebe8fc8929a6b Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 14 Nov 2023 11:07:50 +1100 Subject: [PATCH 3/6] Only mount the root. Check if root already mounted in mountIfNeeded --- .../jetty/util/resource/FileSystemPool.java | 2 +- .../resource/ResourceFactoryInternals.java | 18 ++++--- .../jetty/util/resource/PathResourceTest.java | 50 ++++++++++++++----- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java index 058600a651cd..81db00791eef 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileSystemPool.java @@ -129,7 +129,7 @@ Mount mount(URI uri) throws IOException } // use root FS URI so that pool key/release/sweep is sane URI rootURI = fileSystem.getPath("/").toUri(); - Mount mount = new Mount(rootURI, new MountedPathResource(uri)); + Mount mount = new Mount(rootURI, new MountedPathResource(jarURIRoot)); retain(rootURI, fileSystem, mount); return mount; } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java index d47477615949..945ce3fbc08b 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java @@ -220,15 +220,8 @@ public Resource newResource(URI uri) FileSystemPool.Mount mount = mountIfNeeded(uri); if (mount != null) { - Resource res = resourceFactory.newResource(uri); - if (res == null) - { - mount.close(); // decrement - return null; - } _mounts.add(mount); onMounted(mount, uri); - return res; } } return resourceFactory.newResource(uri); @@ -249,9 +242,20 @@ public Resource newResource(URI uri) */ private FileSystemPool.Mount mountIfNeeded(URI uri) { + // do not mount if it is not a jar URI String scheme = uri.getScheme(); if (!"jar".equalsIgnoreCase(scheme)) return null; + + // Do not mount if we have already mounted + // TODO there is probably a better way of doing this other than string comparisons + String uriString = uri.toASCIIString(); + for (FileSystemPool.Mount mount : _mounts) + { + if (uriString.startsWith(mount.root().toString())) + return null; + } + try { return FileSystemPool.INSTANCE.mount(uri); diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 8e5854bc8cda..92d2707b55e6 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -242,6 +242,35 @@ public void testJarMountNonExistent(WorkDir workDir) throws IOException Path tmpPath = workDir.getEmptyPathDir(); Path testJar = tmpPath.resolve("test.jar"); + URI jarUri = URIUtil.uriJarPrefix(testJar.toUri(), "!/"); + + Map env = new HashMap<>(); + env.put("create", "true"); + try (FileSystem zipfs = FileSystems.newFileSystem(jarUri, env)) + { + zipfs.getPath("/"); + } + try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable()) + { + Resource resBadDir = resourceFactory.newResource(jarUri.toASCIIString() + "does-not-exist/"); + assertNull(resBadDir); + Resource resBadFile = resourceFactory.newResource(jarUri.toASCIIString() + "bad/file.txt"); + assertNull(resBadFile); + + if (resourceFactory instanceof ResourceFactoryInternals.Mountable mountable) + { + List mounts = mountable.getMounts(); + assertThat(mounts.size(), is(1)); + } + } + } + + @Test + public void testMountsForSameJar(WorkDir workDir) throws IOException + { + Path tmpPath = workDir.getEmptyPathDir(); + Path testJar = tmpPath.resolve("test.jar"); + Map env = new HashMap<>(); env.put("create", "true"); @@ -249,31 +278,26 @@ public void testJarMountNonExistent(WorkDir workDir) throws IOException try (FileSystem zipfs = FileSystems.newFileSystem(jarUri, env)) { Path root = zipfs.getPath("/"); - Files.writeString(root.resolve("test.txt"), "Contents of test.txt", StandardCharsets.UTF_8); + Files.writeString(root.resolve("one.txt"), "Contents of one.txt", StandardCharsets.UTF_8); Path dir = root.resolve("datainf"); Files.createDirectory(dir); - Files.writeString(dir.resolve("info.txt"), "Contents of info.txt", StandardCharsets.UTF_8); + Files.writeString(dir.resolve("two.txt"), "Contents of two.txt", StandardCharsets.UTF_8); } try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable()) { - // First reference to Resource in test.jar - Resource resRoot = resourceFactory.newResource(jarUri); - Resource resBadDir = resourceFactory.newResource(jarUri.toASCIIString() + "does-not-exist/"); - assertNull(resBadDir); - Resource resBadFile = resourceFactory.newResource(jarUri.toASCIIString() + "bad/file.txt"); - assertNull(resBadFile); - // Second reference to Resource in test.jar - Resource resInfoTxt = resourceFactory.newResource(jarUri.toASCIIString() + "datainf/info.txt"); - assertTrue(Resources.isReadableFile(resInfoTxt)); + Resource oneTxt = resourceFactory.newResource(jarUri.toASCIIString() + "one.txt"); + assertTrue(Resources.isReadableFile(oneTxt)); + Resource twoTxt = resourceFactory.newResource(jarUri.toASCIIString() + "datainf/two.txt"); + assertTrue(Resources.isReadableFile(twoTxt)); if (resourceFactory instanceof ResourceFactoryInternals.Mountable mountable) { List mounts = mountable.getMounts(); - assertThat(mounts.size(), is(2)); + assertThat(mounts.size(), is(1)); } - } // close dereferences both references to test.jar + } } @Test From 89b1ed98f1d43b9efcfea9a93baa660cc0b91d43 Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 14 Nov 2023 17:09:17 +1100 Subject: [PATCH 4/6] simplified produceChunk --- .../jetty/util/resource/MountedPathResourceTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/MountedPathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/MountedPathResourceTest.java index 559791764192..10631a6d6cf9 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/MountedPathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/MountedPathResourceTest.java @@ -397,7 +397,7 @@ public void testMountByJarNameClosable() assertThat(FileSystemPool.INSTANCE.mounts().size(), is(1)); int mountCount = FileSystemPool.INSTANCE.getReferenceCount(uriRoot); - assertThat(mountCount, is(4)); + assertThat(mountCount, is(1)); } assertThat(FileSystemPool.INSTANCE.mounts().size(), is(0)); @@ -433,13 +433,10 @@ public void testMountByJarNameLifeCycle() throws Exception assertThat(FileSystemPool.INSTANCE.mounts().size(), is(1)); int mountCount = FileSystemPool.INSTANCE.getReferenceCount(uriRoot); - assertThat(mountCount, is(4)); + assertThat(mountCount, is(1)); String dump = resourceFactory.dump(); - assertThat(dump, containsString("newResourceReferences size=4")); + assertThat(dump, containsString("newResourceReferences size=1")); assertThat(dump, containsString(uriRoot.toASCIIString())); - assertThat(dump, containsString(uriRez.toASCIIString())); - assertThat(dump, containsString(uriDeep.toASCIIString())); - assertThat(dump, containsString(uriZzz.toASCIIString())); } finally { From 4c853653e7c765adb86344eec10560e0b750f2de Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 14 Nov 2023 13:34:00 -0600 Subject: [PATCH 5/6] Don't track mounts for newResource() that doesn't exist. --- .../resource/ResourceFactoryInternals.java | 15 ++-- .../jetty/util/resource/PathResourceTest.java | 79 +++++++++++++++++-- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java index 945ce3fbc08b..a870ae70596c 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java @@ -127,32 +127,36 @@ static boolean isSupported(String str) return RESOURCE_FACTORIES.getBest(str) != null; } - interface Mountable + interface Tracking { - List getMounts(); + int getTrackingCount(); } - static class Closeable implements ResourceFactory.Closeable, Mountable + static class Closeable implements ResourceFactory.Closeable, Tracking { + private boolean closed = false; private final CompositeResourceFactory _compositeResourceFactory = new CompositeResourceFactory(); @Override public Resource newResource(URI uri) { + if (closed) + throw new IllegalStateException("Unable to create new Resource on closed ResourceFactory"); return _compositeResourceFactory.newResource(uri); } @Override public void close() { + closed = true; for (FileSystemPool.Mount mount : _compositeResourceFactory.getMounts()) IO.close(mount); _compositeResourceFactory.clearMounts(); } - public List getMounts() + public int getTrackingCount() { - return _compositeResourceFactory.getMounts(); + return _compositeResourceFactory.getMounts().size(); } } @@ -163,6 +167,7 @@ static class LifeCycle extends AbstractLifeCycle implements ResourceFactory.Life @Override public Resource newResource(URI uri) { + // TODO: add check that LifeCycle is started before allowing this method to be used? return _compositeResourceFactory.newResource(uri); } diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 92d2707b55e6..9d0688685fc0 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -54,6 +54,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -257,10 +258,9 @@ public void testJarMountNonExistent(WorkDir workDir) throws IOException Resource resBadFile = resourceFactory.newResource(jarUri.toASCIIString() + "bad/file.txt"); assertNull(resBadFile); - if (resourceFactory instanceof ResourceFactoryInternals.Mountable mountable) + if (resourceFactory instanceof ResourceFactoryInternals.Tracking tracking) { - List mounts = mountable.getMounts(); - assertThat(mounts.size(), is(1)); + assertThat(tracking.getTrackingCount(), is(1)); } } } @@ -292,14 +292,81 @@ public void testMountsForSameJar(WorkDir workDir) throws IOException Resource twoTxt = resourceFactory.newResource(jarUri.toASCIIString() + "datainf/two.txt"); assertTrue(Resources.isReadableFile(twoTxt)); - if (resourceFactory instanceof ResourceFactoryInternals.Mountable mountable) + if (resourceFactory instanceof ResourceFactoryInternals.Tracking tracking) { - List mounts = mountable.getMounts(); - assertThat(mounts.size(), is(1)); + assertThat(tracking.getTrackingCount(), is(1)); } } } + @Test + public void testMountsForSameJarDifferentResourceFactories(WorkDir workDir) throws IOException + { + Path tmpPath = workDir.getEmptyPathDir(); + Path testJar = tmpPath.resolve("test.jar"); + + Map env = new HashMap<>(); + env.put("create", "true"); + + URI jarUri = URIUtil.uriJarPrefix(testJar.toUri(), "!/"); + try (FileSystem zipfs = FileSystems.newFileSystem(jarUri, env)) + { + Path root = zipfs.getPath("/"); + Files.writeString(root.resolve("one.txt"), "Contents of one.txt", StandardCharsets.UTF_8); + + Path dir = root.resolve("datainf"); + Files.createDirectory(dir); + Files.writeString(dir.resolve("two.txt"), "Contents of two.txt", StandardCharsets.UTF_8); + } + + assertThat(FileSystemPool.INSTANCE.mounts(), is(empty())); + + try (ResourceFactory.Closeable resourceFactory1 = ResourceFactory.closeable(); + ResourceFactory.Closeable resourceFactory2 = ResourceFactory.closeable()) + { + Resource oneTxt = resourceFactory1.newResource(jarUri.toASCIIString() + "one.txt"); + assertTrue(Resources.isReadableFile(oneTxt)); + + Resource oneTxt2 = resourceFactory1.newResource(jarUri.toASCIIString() + "one.txt"); + assertTrue(Resources.isReadableFile(oneTxt)); + + Resource twoTxt = resourceFactory2.newResource(jarUri.toASCIIString() + "datainf/two.txt"); + assertTrue(Resources.isReadableFile(twoTxt)); + + assertThat("Should see only 1 FS Mount", FileSystemPool.INSTANCE.mounts().size(), is(1)); + + if (resourceFactory1 instanceof ResourceFactoryInternals.Tracking tracking) + { + assertThat(tracking.getTrackingCount(), is(1)); + } + + if (resourceFactory2 instanceof ResourceFactoryInternals.Tracking tracking) + { + assertThat(tracking.getTrackingCount(), is(1)); + } + + // Close Resource Factory 1 + resourceFactory1.close(); + + if (resourceFactory1 instanceof ResourceFactoryInternals.Tracking tracking) + { + assertThat(tracking.getTrackingCount(), is(0)); + } + + // should not be able to use closed ResourceFactory.Closable + assertThrows(IllegalStateException.class, () -> resourceFactory1.newResource(jarUri.toASCIIString() + "one.txt")); + + assertThat("Should see only 1 FS Mount", FileSystemPool.INSTANCE.mounts().size(), is(1)); + + Resource oneAlt = resourceFactory2.newResource(jarUri.toASCIIString() + "one.txt"); + assertTrue(Resources.isReadableFile(oneAlt)); + } + finally + { + assertThat(FileSystemPool.INSTANCE.mounts(), is(empty())); + } + } + @Test public void testJarFileIsAliasFile(WorkDir workDir) throws IOException { From 62c6b6e9a407908b55dfdaedc5158a2ccfa669e2 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 15 Nov 2023 10:05:16 +1100 Subject: [PATCH 6/6] improved test --- .../jetty/util/resource/PathResourceTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java index 9d0688685fc0..b3ea77bb4ee5 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java @@ -18,6 +18,7 @@ import java.net.URI; import java.nio.channels.ReadableByteChannel; import java.nio.charset.StandardCharsets; +import java.nio.file.ClosedFileSystemException; import java.nio.file.FileSystem; import java.nio.file.FileSystemException; import java.nio.file.FileSystems; @@ -353,6 +354,9 @@ public void testMountsForSameJarDifferentResourceFactories(WorkDir workDir) thro assertThat(tracking.getTrackingCount(), is(0)); } + // Resource one still works because factory 2 is holding filesystem open + assertThat(IO.toString(oneTxt.newInputStream()), is("Contents of one.txt")); + // should not be able to use closed ResourceFactory.Closable assertThrows(IllegalStateException.class, () -> resourceFactory1.newResource(jarUri.toASCIIString() + "one.txt")); @@ -360,6 +364,15 @@ public void testMountsForSameJarDifferentResourceFactories(WorkDir workDir) thro Resource oneAlt = resourceFactory2.newResource(jarUri.toASCIIString() + "one.txt"); assertTrue(Resources.isReadableFile(oneAlt)); + + // Close Resource Factory 2 + resourceFactory2.close(); + + // Neither Resource one nor two works because filesystem is now closed + assertThrows(ClosedFileSystemException.class, oneTxt::newInputStream); + assertThrows(ClosedFileSystemException.class, oneTxt2::newInputStream); + + assertThat("Should see only 0 FS Mount", FileSystemPool.INSTANCE.mounts().size(), is(0)); } finally {