On Mon, 15 Dec 2025 09:10:21 GMT, Daniel Fuchs <[email protected]> wrote:

>> This PR fixes a deadlock between the `localConnectionIdManager` and the 
>> `connections` map by closing the manager before calling 
>> `connections.compute`.
>> 
>> No new tests; the issue requires a complex setup to reproduce, and the new 
>> code is easy enough to reason about. Existing tests continue to pass.
>
> src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java 
> line 1593:
> 
>> 1591:         // we can ignore stateless reset in the draining state.
>> 1592:         remapPeerIssuedResetToken(connection, draining);
>> 1593:         draining.startTimer();
> 
> shouldn't we start the timer only if the connection has been added, and 
> therefore call startTimer in compute?

I'd rather not call this from compute; every method called from the compute 
lambda increases the risk of reintroducing the deadlock, so I'd like to keep 
the lambda to a minimum.

Most of the time the connection will be added; the only case where it won't is 
when there are multiple threads attempting to close the connection in parallel. 
The timer task only removes the connection from the endpoint, so, worst case, 
we will remove the connection IDs and the reset tokens twice. The second 
removal will likely be a no-op, unless we somehow manage to reassign the IDs or 
the tokens to a different connection.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28788#discussion_r2618740481

Reply via email to