8372198: Avoid closing PlainHttpConnection while holding a lock #28430
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 #28421
Progress
Integration blocker
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28430/head:pull/28430$ git checkout pull/28430Update a local copy of the PR:
$ git checkout pull/28430$ git pull https://git.openjdk.org/jdk.git pull/28430/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28430View PR using the GUI difftool:
$ git pr show -t 28430Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28430.diff
Using Webrev
Link to Webrev Comment