RFR: JDK-6956385: URLConnection.getLastModified() leaks file handles for jar:file and file: URLs [v2]
Daniel Fuchs
dfuchs at openjdk.org
Fri May 19 15:48:53 UTC 2023
On Fri, 19 May 2023 15:34:45 GMT, Jesse Glick <duke at openjdk.org> wrote:
>> src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java line 97:
>>
>>> 95: is.close();
>>> 96: is = null;
>>> 97: connected = false;
>>
>> Not sure about switching `connected` to false or setting `is` to null. That might allow to call `connect` again and might have unintended side effects?
>
> It depends on whether you consider the new method to be the opposite of `connect`, toggling between connected and unconnected states. Removing these two lines would mean that a subsequent call to `getInputStream()` would return a closed stream, which may or may not be surprising. An aspect of JDK-8224095 is that the state transitions for `URLConnection` are generally confusing. For now it seems irrelevant since `JarURLConnection.jarFileURLConnection` is never used for its input stream.
>
> ~In fact I think `closeInputStream` could just be moved into `JarURLConnection.<init>` for this reason.~ No, it cannot, since `FileURLConnection.is` is opened only if and when a header is requested.
Well yes - exactly my point. At the moment if you call getInputStream() twice in succession you will get the same input stream. The transition from not connected to connected is stable. `connect` can only transition once from `false` to `true`. There's nothing that sets `is` back to `null` or `connect` back to `false` so I believe it should stay that way.
Let's keep the (behaviour) changes minimal.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12871#discussion_r1199112611
More information about the net-dev
mailing list