> Can I please get a review of this change which proposes to fix the issue > noted in https://bugs.openjdk.org/browse/JDK-8338445? > > The JDK internal class `jdk.internal.loader.URLClassPath` is used by > classloader implementations in the JDK (for example by the > `java.net.URLClassLoader` and the > `jdk.internal.loader.ClassLoaders$AppClassLoader`). The implementation in > `URLClassPath` constructs internal `Loader`s for each URL that is in the > classpath of that instance. `Loader` implementations hold on to underlying > resources from which the classpath resources are served by the `URLClassPath`. > > When constructing a `Loader`, the `URLClassPath` allows the loader > implementation to parse a local classpath for that specific `Loader`. For > example, the `JarLoader` (an internal class of the JDK) will parse > `Class-Path` attribute in the jar file's manifest to construct additional > URLs that will be included in the classpath through which resources will be > served by `URLClassPath`. While parsing the local classpath, if the `Loader` > instance runs into any `IOException` or a `SecurityException`, the > `URLClassPath` will ignore that specific instance of the `Loader` and will > not hold on to it as a classpath element. > > As noted earlier, `Loader` instances may hold onto underlying resources. When > the `URLClassPath` ignores such `Loader`(s) and doesn't call `close()` on > them, then that results in potential resource leaks. The linked JBS issue > demonstrates a case where the `JarFile` held by the `JarLoader` doesn't get > closed (until GC garbage collects the `JarLoader` and thus the `JarFile`), > when the `URLClassPath` ignores that `JarLoader` due to an `IOException` when > the `JarLoader` is parsing the `Class-Path` value from the jar's manifest. > > The commit in this PR addresses the issue by calling `close()` on such > `Loader`s that get rejected by the `URLClassPath` when either an > `IOException` or a `SecurityException` gets thrown when constructing the > `Loader` or parsing the local classpath. > > A new jtreg test has been introduced which consistently reproduces this issue > (on Windows) and verifies the fix. Additionally tier1, tier2 and tier3 > testing has completed with this change without any failures.
Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Christian's suggestion - string ValueSource(s) instead of MethodSource ------------- Changes: - all: https://git.openjdk.org/jdk/pull/20691/files - new: https://git.openjdk.org/jdk/pull/20691/files/b5bf8412..7a26807d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20691&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20691&range=01-02 Stats: 27 lines in 1 file changed: 7 ins; 17 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/20691.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20691/head:pull/20691 PR: https://git.openjdk.org/jdk/pull/20691