On Mon, 22 May 2023 16:19:40 GMT, Jesse Glick <d...@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-b...@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
