On Tue, 25 Nov 2025 06:35:23 GMT, Jaikiran Pai <[email protected]> wrote:
>> Daniel Fuchs has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains eight commits:
>>
>> - Merge branch 'master' into ConnectionCloseLock-8372198
>> - Merge master
>> - 8372198: Avoid closing PlainHttpConnection while holding a lock
>> - Merge branch 'master' into SelectorManagerVT-8372159
>> - Copyright years
>> - Review feedback on test
>> - Revert changes to SelectorManager::shutdown
>> - 8372159: HttpClient SelectorManager thread could be a VirtualThread
>
> src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java
> line 1356:
>
>> 1354: } finally {
>> 1355: // cleanup anything that might have been left behind
>> 1356: owner.stop();
>
> Hello Daniel, since we already call `owner.stop()` unconditionally before the
> `try` block, is this second invocation of that method required?
stop() is reentrant not-reentrant :-)
Calling it a second time means we're going to do a second sweep through the
various list and close anything that might have sneaked through in between the
first call and the time where `closed = true` was set. Remember - the purpose
here is to close existing connections (which should all be idle in the pool)
before closing the selector to prevent exceptions caused by the selector being
thrown. Calling stop() a second time should be harmless.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28430#discussion_r2559471516
