From 62f215929c5b368ec00d3ffb2663af1c8b5f7d2e Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 11 Apr 2024 13:11:54 +0200 Subject: [PATCH] [core] Disable Caching in URLConnection for ClasspathClassLoader Fixes #4899 --- docs/pages/release_notes.md | 1 + .../internal/util/ClasspathClassLoader.java | 10 ++ .../util/ClasspathClassLoaderTest.java | 105 ++++++++++++++++-- .../java/symbols/internal/asm/ClassStub.java | 3 + .../java/symbols/internal/asm/Loader.java | 2 +- 5 files changed, 112 insertions(+), 9 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 386305f89f..912c4ce1c5 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -31,6 +31,7 @@ This is a {{ site.pmd.release_type }} release. * apex-errorprone * [#3953](https://github.com/pmd/pmd/issues/3953): \[apex] EmptyCatchBlock false positive with formal (doc) comments * java + * [#4899](https://github.com/pmd/pmd/issues/4899): \[java] Parsing failed in ParseLock#doParse() java.io.IOException: Stream closed * [#4902](https://github.com/pmd/pmd/issues/4902): \[java] "Bad intersection, unrelated class types" for Constable\[] and Enum\[] * java-bestpractices * [#1084](https://github.com/pmd/pmd/issues/1084): \[java] Allow JUnitTestsShouldIncludeAssert to configure verification methods diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/ClasspathClassLoader.java b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/ClasspathClassLoader.java index 2d5da86589..3a73f98a23 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/ClasspathClassLoader.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/ClasspathClassLoader.java @@ -53,6 +53,16 @@ public class ClasspathClassLoader extends URLClassLoader { static { registerAsParallelCapable(); + + // Disable caching for jar files to prevent issues like #4899 + try { + // Uses a pseudo URL to be able to call URLConnection#setDefaultUseCaches + // with Java9+ there is a static method for that per protocol: + // https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/net/URLConnection.html#setDefaultUseCaches(java.lang.String,boolean) + URI.create("jar:file:file.jar!/").toURL().openConnection().setDefaultUseCaches(false); + } catch (IOException e) { + throw new RuntimeException(e); + } } public ClasspathClassLoader(List files, ClassLoader parent) throws IOException { diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/internal/util/ClasspathClassLoaderTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/internal/util/ClasspathClassLoaderTest.java index 50103b9091..d4661fd276 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/internal/util/ClasspathClassLoaderTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/internal/util/ClasspathClassLoaderTest.java @@ -17,6 +17,11 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.Semaphore; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; @@ -60,27 +65,111 @@ class ClasspathClassLoaderTest { } } - @Test - void loadFromJar() throws IOException { - final String RESOURCE_NAME = "net/sourceforge/pmd/Sample.txt"; - final String TEST_CONTENT = "Test\n"; + private static final String CUSTOM_JAR_RESOURCE = "net/sourceforge/pmd/Sample.txt"; + private static final String CUSTOM_JAR_RESOURCE2 = "net/sourceforge/pmd/Sample2.txt"; + private static final String CUSTOM_JAR_RESOURCE_CONTENT = "Test\n"; + private Path prepareCustomJar() throws IOException { Path jarPath = tempDir.resolve("custom.jar"); try (ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(jarPath))) { - out.putNextEntry(new ZipEntry(RESOURCE_NAME)); - out.write(TEST_CONTENT.getBytes(StandardCharsets.UTF_8)); + out.putNextEntry(new ZipEntry(CUSTOM_JAR_RESOURCE)); + out.write(CUSTOM_JAR_RESOURCE_CONTENT.getBytes(StandardCharsets.UTF_8)); + out.putNextEntry(new ZipEntry(CUSTOM_JAR_RESOURCE2)); + out.write(CUSTOM_JAR_RESOURCE_CONTENT.getBytes(StandardCharsets.UTF_8)); } + return jarPath; + } + + @Test + void loadFromJar() throws IOException { + Path jarPath = prepareCustomJar(); String classpath = jarPath.toString(); try (ClasspathClassLoader loader = new ClasspathClassLoader(classpath, null)) { - try (InputStream in = loader.getResourceAsStream(RESOURCE_NAME)) { + try (InputStream in = loader.getResourceAsStream(CUSTOM_JAR_RESOURCE)) { assertNotNull(in); String s = IOUtil.readToString(in, StandardCharsets.UTF_8); - assertEquals(TEST_CONTENT, s); + assertEquals(CUSTOM_JAR_RESOURCE_CONTENT, s); } } } + /** + * @see [java] Parsing failed in ParseLock#doParse() java.io.IOException: Stream closed #4899 + */ + @Test + void loadMultithreadedFromJar() throws IOException, InterruptedException { + Path jarPath = prepareCustomJar(); + String classpath = jarPath.toString(); + + int numberOfThreads = 2; + + final CyclicBarrier waitForClosed = new CyclicBarrier(numberOfThreads); + final Semaphore grabResource = new Semaphore(1); + final List caughtExceptions = new ArrayList<>(); + + class ThreadRunnable extends Thread { + private final int number; + + ThreadRunnable(int number) { + super("Thread" + number); + this.number = number; + } + + @Override + public void run() { + try (ClasspathClassLoader loader = new ClasspathClassLoader(classpath, null)) { + // Make sure, the threads get the resource stream one after another, so that the + // underlying Jar File is definitively cached (if caching is enabled). + grabResource.acquire(); + InputStream stream; + try { + stream = loader.getResourceAsStream(CUSTOM_JAR_RESOURCE); + } finally { + grabResource.release(); + } + try (InputStream in = stream) { + assertNotNull(in); + if (number > 0) { + // all except the first thread should wait until the first thread is finished + // and has closed the ClasspathClassLoader + waitForClosed.await(); + } + String s = IOUtil.readToString(in, StandardCharsets.UTF_8); + assertEquals(CUSTOM_JAR_RESOURCE_CONTENT, s); + } + } catch (Exception e) { + caughtExceptions.add(e); + throw new RuntimeException(e); + } finally { + try { + if (number == 0) { + // signal the other waiting threads to continue. Here, we have closed + // already the ClasspathClassLoader. + waitForClosed.await(); + } + } catch (InterruptedException e) { + throw new RuntimeException(e); + } catch (BrokenBarrierException e) { + throw new RuntimeException(e); + } + } + } + } + + List threads = new ArrayList<>(numberOfThreads); + for (int i = 0; i < numberOfThreads; i++) { + threads.add(new ThreadRunnable(i)); + } + + threads.forEach(Thread::start); + for (Thread thread : threads) { + thread.join(); + } + + assertTrue(caughtExceptions.isEmpty()); + } + /** * Verifies, that we load the class files from the runtime image of the correct java home. * This tests multiple versions, in order to avoid that the test accidentally is successful when diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java index 581b556399..813a6390f9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java @@ -99,6 +99,9 @@ final class ClassStub implements JClassSymbol, AsmStub, AnnotationOwner { } else { return false; } + } catch (IOException e) { + // add a bit more info to the exception + throw new IOException("While loading class from " + loader, e); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/Loader.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/Loader.java index 8289601568..0eb7b64328 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/Loader.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/Loader.java @@ -50,7 +50,7 @@ abstract class Loader { @Override public String toString() { - return "(StreamLoader for " + name + ")"; + return "StreamLoader(for " + name + ")"; } } }