Withdrawn: 8227020: Unclosed input streams in URL Class Loader for JARs with META-INF/INDEX.LIST

[Jaikiran Pai]

On Mon, 15 Nov 2021 12:58:45 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

> Can I please get a review for this change which proposes to fix the issue noted in https://bugs.openjdk.java.net/browse/JDK-8227020?
> 
> The linked issue talks about non-normalized paths contributing to the leak. However, the root cause is applicable for regular normalized paths too. The `URLClassLoader` is backed by an instance of `jdk.internal.loader.URLClassPath`. This `URLClassPath` keeps track of a `loader` for each of the paths that are either provided directly as the `URL[]` or are inferred through the `Class-Path` attribute of each loaded jar.  This it does in 2 different fields. One is a mapping between the String representation of the URL for which the loader was created. This field is the `lmap` and the other is a list of `loaders`. The list of `loaders` is the search path for loading any resources from this `URLClassPath`. When the `closeLoaders()` is called by the `URLClassLoader`, this `loaders` list is iterated to close each of the `loader` for the opened resources (which in the context of this issue translates to closing a `java.util.jar.JarFile`).
> 
> A jar can have a `META-INF/INDEX.LIST`, which is a index of resources listed against each jar file. The `URLClassPath` when looking for resources in a `Loader` parses this index to search for resources. While doing so, if it finds a jar listed in the index, for which a `Loader` hasn't yet been created then it creates a new loader for such a jar and adds it to the `lmap`. However, it doesn't add this to the `loaders` list, the one which is used to close the opened jar files. As a result, it ends up opening a `JarFile` which is never closed, even if the `URLClassPath`'s `closeLoaders()` is called, thus triggering a leak.
> 
> The commit in this PR adds the newly created loader to the list of `loaders` so that it is properly closed, when the `closeLoaders` gets called.
> 
> I have reviewed the code to see if this has any other side effects and from what I can see, the addition of this new loader to the list shouldn't cause any other issues for reasons that follow:
> 
> - the order in the `loaders` list doesn't seem to have any one-to-one mapping with the URL[] of classpath elements. In fact, loaders for newly discovered URLs from the `Class-Path` attribute get added dynamically to this list. So adding this new loader found in the index shouldn't cause any issues when it comes to the order of loaders in the list.
> - The `URLClassPath` is already returning resources from a loader corresponding to a jar `X` listed in the index even if that jar `X` isn't part of the original URL[] forming the classpath of the `URLClassPath`. So adding this `loader` to the "search path" won't introduce any new behaviour.
> 
> A new jtreg test has been introduced to reproduce this issue and verify the fix. The test has been run on a Windows setup where the test reproduces this issue without this fix and verifies the fix.
> 
> Recently, the jar index feature has been disabled as part of https://bugs.openjdk.java.net/browse/JDK-8273401. However, since users can enable this feature by using a specific system property and since this issue relates to a leak, I decided to see if there is interest in this fix.

This pull request has been closed without being integrated.

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

PR: https://git.openjdk.java.net/jdk/pull/6389