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

[Daniel Fuchs]

On Fri, 19 May 2023 13:00:30 GMT, Jesse Glick <duke at openjdk.org> wrote:

>> src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java line 96:
>> 
>>> 94:         return connected;
>>> 95:     }
>>> 96: 
>> 
>> I wonder if it would be better to add a
>> 
>> 
>> void closeInputStream() { ... } // (or void close() { ... }?)
>> 
>> 
>> that would simply close the input stream if it's not null...
>
> `close()` would be natural enough, and would encapsulate behavior a bit better.
> 
> Should it then actually implement `Closeable`, permitting the implicit request in https://github.com/openjdk/jdk/pull/12871#discussion_r1198873512 to be addressed? That would not technically be an API change, since the implementation class is not exported, though it could be a behavioral change in case anyone is checking `instanceof Closeable` (or `instanceof AutoCloseable`); and would be a first step along the way to JDK-8224095, which may or may not be appropriate for something labeled a bug fix.

This is a good question. I we made `FileURLConnection` `Closeable` or `AutoCloseable`, and called `close()` in `JarURLConnection` when the underlying connection is `(Auto)Closeable` then I believe it could warrant a CSR. On the other hand IIRC the `file:` protocol handler cannot be overridden - so I'm not sure whether we could actually have a custom underlying `URLConnection` - that would need to be carefully evaluated.

Also, as you note, instance of FileURLConnection may be returned to user code - so the fact that the URLConnection implements `Closeable`/`Autocloseable` could be observed, and would also require a CSR.

That said - we could simply expose a close() method on `FileURLConnection` without implementing `Closeable`/`Autocloseable` - and could examine that (implementing `Closeable`/`Autocloseable`) in a different PR - perhaps as a Enhencement request - rather than a bug fix.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12871#discussion_r1198989388