On Mon, 15 Dec 2025 09:58:08 GMT, Daniel Jeliński <[email protected]> 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

Reply via email to