RFR: 8372198: Avoid closing PlainHttpConnection while holding a lock [v10]

[Daniel Fuchs]

On Thu, 27 Nov 2025 12:46:34 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> An experimental change to SelectorManager::shutdown unveiled a potential deadlock between the SelectorManager thread trying to stop the HttpClientImpl, and an executor thread concurrently trying to return a connection to the pool.
>> 
>> The issue seems to be caused by the ConnectionPool::returnToPool trying to close the returned connection when stopping, while holding the ConnectionPool state lock, and the SelectorManager thread trying to close a pooled connection, holding the connection stateLock and trying to close the channel, which caused the CleanupTrigger to fire and attempt to remove the connection from the pool.
>> 
>> This problem was observed once with the java/net/httpclient/ThrowingSubscribersAsLimitingAsync.java test.
>> 
>> To avoid the problem, the proposed fix is to wait until the ConnectionPool::stateLock has been released before actually closing the connection, and to wait until the PlainHttpConnection::stateLock has been released before actually closing the channel. Indeed, there should be no need to close those while holding the lock.
>> 
>> This PR was recreated due to a bad merge pushed to https://github.com/openjdk/jdk/pull/28421
>
> Daniel Fuchs has updated the pull request incrementally with four additional commits since the last revision:
> 
>  - Update test/jdk/java/net/httpclient/PlainConnectionLockTest.java
>    
>    Co-authored-by: Volkan Yazıcı <volkan.yazici at oracle.com>
>  - Update test/jdk/java/net/httpclient/PlainConnectionLockTest.java
>    
>    Co-authored-by: Volkan Yazıcı <volkan.yazici at oracle.com>
>  - Update test/jdk/java/net/httpclient/PlainConnectionLockTest.java
>    
>    Co-authored-by: Volkan Yazıcı <volkan.yazici at oracle.com>
>  - Update test/jdk/java/net/httpclient/PlainConnectionLockTest.java
>    
>    Co-authored-by: Volkan Yazıcı <volkan.yazici at oracle.com>

The issue being fixed is a race condition so you would have to be very lucky to make it fail. It might require adding Thread.sleep in the product code and I haven't investigated that far.

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

PR Comment: https://git.openjdk.org/jdk/pull/28430#issuecomment-3585714126