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

[Daniel Fuchs]

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

>> 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 four additional commits since the last revision:
>> 
>>  - 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`
>
> src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java line 93:
> 
>> 91:     }
>> 92: 
>> 93:     public synchronized void closeInputStream() throws IOException {
> 
> Note that synchronization matches https://github.com/openjdk/jdk/blob/37b042c121d39f7eeca6bd75d77224ed433b65ea/src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java#L182-L214 though https://github.com/openjdk/jdk/blob/37b042c121d39f7eeca6bd75d77224ed433b65ea/src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java#L73-L91 is not synchronized, which smells suspicious (at least I think SpotBugs would flag this). There are other oddities in this old code such as the unused variables https://github.com/openjdk/jdk/blob/37b042c121d39f7eeca6bd75d77224ed433b65ea/src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java#L185-L186 so I am trying to avoid larger changes.

Yes - that seems reasonable. I agree the use of synchronization here is a bit dubious. URL / URLConnection and connection Handlers are quite old and unfortunately suffer from a bad case of technical debt - which is hard to remove.

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

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