RFR: JDK-6956385: URLConnection.getLastModified() leaks file handles for jar:file and file: URLs [v4]

[Jaikiran Pai]

On Mon, 22 May 2023 16:19:40 GMT, Jesse Glick <duke at openjdk.org> wrote:

>> [JDK-6956385](https://bugs.openjdk.org/browse/JDK-6956385): `JarURLConnection` properly tracks any `InputStream` it itself opened, and correspondingly closes the `JarFile` if necessary (when caches are disabled). However if its underlying `FileURLConnection` was used to retrieve a header field, that would have caused a `FileInputStream` to be opened which never gets closed until it is garbage collected. This means that an application which calls certain methods on `jar:file:/…something.jar!/…` URLs will leak file handles, even if `URLConnection` caches are supposed to be turned off. This can delay release of system resources, and on Windows can prevent the JAR file from being deleted even after it is no longer in use (for example after `URLClassLoader.close`).
>> 
>> [JDK-8224095](https://bugs.openjdk.org/browse/JDK-8224095) was marked as a duplicate, but I think incorrectly. It refers to `FileURLConnection`, and seems to be complaining about the confusing API design of `URLConnection` generally: that it is an object which you might expect to be `Closeable` but in fact it is its `inputStream` which must be `close`d. In JDK-6956385, even when the caller _does_ specifically call `InputStream.close`, a file handle may be leaked.
>> 
>> I managed to build the JDK on both Linux and (Cygwin) Windows to confirm the fix via
>> 
>> 
>> ./build/…-x86_64-server-release/jdk/bin/java test/jdk/sun/net/www/protocol/jar/FileURLConnectionLeak.java
>> 
>> 
>> I also ran jtreg on Linux (this test and various others in nearby packages); on Windows the `make test` target just hung for some reason.
>> 
>> I marked the test `othervm` out of caution, since it is mutating global state, and if it fails will leak a handle and prevent scratch dir cleanup on Windows.
>> 
>> (This is my first contribution, at least after the move to GitHub, so let me know if something is missing here technically or stylistically. None of the contribution guides appear to be up to date.)
>
> Jesse Glick has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into FileURLConnectionLeak-JDK-6956385
>  - `URLConnection.getLastModified` reproduces the bug the same as `getContentEncoding` but better matches the bug title (suggested by @ecki https://github.com/openjdk/jdk/pull/12871#discussion_r1200716271)
>  - Leaving `FileURLConnection.is` non-null, and claiming `connected`, even after `closeInputStream` https://github.com/openjdk/jdk/pull/12871#discussion_r1199085883
>  - Encapsulate logic as `FileURLConnection.closeInputStream` as suggested by @dfuch https://github.com/openjdk/jdk/pull/12871#discussion_r1198859517
>  - Merge branch 'master' of https://github.com/openjdk/jdk into FileURLConnectionLeak-JDK-6956385
>  - Simplified `instanceof` form
>    
>    Co-authored-by: ExE Boss <3889017+ExE-Boss at users.noreply.github.com>
>  - 6956385: `JarURLConnection` may fail to close its underlying `FileURLConnection`

Hello Jesse, 

> (Trying to follow instructions in https://openjdk.org/jeps/369, if those are even still valid. https://github.com/openjdk/jdk/blob/master/CONTRIBUTING.md is not very helpful. I guess it should link to https://openjdk.org/guide/#life-of-a-pr which makes no mention of `/summary`?)

Skara bot commands are listed in https://wiki.openjdk.org/display/SKARA/Pull+Request+Commands

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

PR Comment: https://git.openjdk.org/jdk/pull/12871#issuecomment-1612310113