> 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

Reply via email to