On Fri, 23 Aug 2024 10:45:49 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> 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. This pull request has now been integrated. Changeset: 2e174c63 Author: Jaikiran Pai <j...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/2e174c6367c7755d8541f9669f7f10ed89878f58 Stats: 184 lines in 2 files changed: 173 ins; 4 del; 7 mod 8338445: jdk.internal.loader.URLClassPath may leak JarFile instance when dealing with unexpected Class-Path entry in manifest Reviewed-by: michaelm, cstein, alanb ------------- PR: https://git.openjdk.org/jdk/pull/20691
