On Wed, 1 Apr 2026 12:19:14 GMT, Daniel Jeliński <[email protected]> wrote:
> The QuicEndpoint keeps a collection of active stateless reset tokens mapped
> to connections. Updates to the map are not fully synchronized, and sometimes
> tokens were added after a connection was closed and unregistered from the
> endpoint.
>
> The code adds two new assertions (in QuicEndpoint and in PeerConnIdManager)
> that caused occasional test failures.
>
> All java/net/httpclient tests are stable.
src/java.net.http/share/classes/jdk/internal/net/http/quic/PeerConnIdManager.java
line 288:
> 286: lock.lock();
> 287: try {
> 288: assert closed;
Looking at the code it seems OK to assert this, but it would be good to have a
comment explaining when this method is called to explain the assert. And
possibly add some statement to method API doc stating that it should not be
called before the connection manager is closed.
I mean - someone could legitimately want to call this method to log the
stateless reset tokens at any point in time, even if the connection manager is
not closed. We don't do that - we call this method only after having closed the
connection manager, and the assert here validate that assumptions.
src/java.net.http/share/classes/jdk/internal/net/http/quic/PeerConnIdManager.java
line 551:
> 549: public void close() {
> 550: lock.lock();
> 551: closed = true;
nit: I know that `closed = true` is not supposed to throw anything but not
having try-finally is hurting my eyes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30534#discussion_r3022811828
PR Review Comment: https://git.openjdk.org/jdk/pull/30534#discussion_r3022705471
