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.

-------------

Commit messages:
 - 8338445: close loaders that are not used by the URLClassPath
 - 8338445: add test

Changes: https://git.openjdk.org/jdk/pull/20691/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20691&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8338445
  Stats: 195 lines in 2 files changed: 183 ins; 3 del; 9 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

Reply via email to