On Fri, 19 May 2023 15:00:17 GMT, Jesse Glick <d...@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-b...@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
