RFR: 8373409: java/net/httpclient/http3/H3ErrorHandlingTest.java failed due to deadlock

[Daniel Fuchs]

On Mon, 15 Dec 2025 09:58:08 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> 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.

A look at `startTimer` lets me think that it should be safe, but OK. I suspect the double removal would not be an issue - IIRC we check with == on the removed connection, but we would be potentially adding an event to the timer queue which will wake up the timer queue for nothing. We could add a boolean field to ClosingConnection that we could set to `true` if the connection is added and check that after compute has been called, and start the timer then. I'll let you decide if it's worth it.

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

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