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