From 7a7e7c9ae11cb124c14d5d2d3b7e2f5649205106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Jeli=C5=84ski?= Date: Thu, 18 Dec 2025 13:17:44 +0000 Subject: [PATCH] 8373877: QUIC connections are removed too early Reviewed-by: dfuchs --- .../net/http/quic/ConnectionTerminatorImpl.java | 6 +----- .../net/http/quic/QuicConnectionImpl.java | 2 +- .../internal/net/http/quic/QuicEndpoint.java | 5 +++-- .../quic/StatelessResetReceiptTest.java | 17 +++++++++++++++++ 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminatorImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminatorImpl.java index 7870f4f1d8e..5e2384dce27 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminatorImpl.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminatorImpl.java @@ -344,10 +344,6 @@ final class ConnectionTerminatorImpl implements ConnectionTerminator { } } failHandshakeCFs(); - // remap the connection to a draining connection - final QuicEndpoint endpoint = this.connection.endpoint(); - assert endpoint != null : "QUIC endpoint is null"; - endpoint.draining(connection); discardConnectionState(); connection.streams.terminate(terminationCause); if (Log.quic()) { @@ -439,7 +435,7 @@ final class ConnectionTerminatorImpl implements ConnectionTerminator { final ProtectionRecord protectionRecord = ProtectionRecord.single(packet, connection::allocateDatagramForEncryption); // while sending the packet containing the CONNECTION_CLOSE frame, the pushDatagram will - // remap (or remove) the QuicConnectionImpl in QuicEndpoint. + // remap the QuicConnectionImpl in QuicEndpoint. connection.pushDatagram(protectionRecord); } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java index c07df1c6eb2..580bbe23d31 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java @@ -2811,7 +2811,7 @@ public class QuicConnectionImpl extends QuicConnection implements QuicPacketRece // a CONNECTION_CLOSE frame is being sent to the peer when the local // connection state is in DRAINING. This implies that the local endpoint // is responding to an incoming CONNECTION_CLOSE frame from the peer. - // we remove the connection from the endpoint for such cases. + // we switch this connection to one that does not respond to incoming packets. endpoint.pushClosedDatagram(this, peerAddress(), datagram); } else if (stateHandle.isMarked(QuicConnectionState.CLOSING)) { // a CONNECTION_CLOSE frame is being sent to the peer when the local diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java b/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java index 8bdba21594c..91b4a678a23 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java @@ -1509,7 +1509,8 @@ public abstract sealed class QuicEndpoint implements AutoCloseable /** * Called to schedule sending of a datagram that contains a single {@code ConnectionCloseFrame} * sent in response to a {@code ConnectionClose} frame. - * This will completely remove the connection from the connection map. + * This will replace the {@link QuicConnectionImpl} with a {@link DrainingConnection} that + * will discard all incoming packets. * @param connection the connection being closed * @param destination the peer address * @param datagram the datagram @@ -1518,7 +1519,7 @@ public abstract sealed class QuicEndpoint implements AutoCloseable InetSocketAddress destination, ByteBuffer datagram) { if (debug.on()) debug.log("Pushing closed datagram for " + connection.logTag()); - removeConnection(connection); + draining(connection); pushDatagram(connection, destination, datagram); } diff --git a/test/jdk/java/net/httpclient/quic/StatelessResetReceiptTest.java b/test/jdk/java/net/httpclient/quic/StatelessResetReceiptTest.java index 8eb7a3663d8..bee56824381 100644 --- a/test/jdk/java/net/httpclient/quic/StatelessResetReceiptTest.java +++ b/test/jdk/java/net/httpclient/quic/StatelessResetReceiptTest.java @@ -47,6 +47,7 @@ import jdk.internal.net.http.common.MinimalFuture; import jdk.internal.net.http.quic.QuicClient; import jdk.internal.net.http.quic.QuicConnectionId; import jdk.internal.net.http.quic.QuicConnectionImpl; +import jdk.internal.net.http.quic.QuicTransportParameters; import jdk.internal.net.http.quic.TerminationCause; import jdk.internal.net.quic.QuicTLSContext; import jdk.internal.net.quic.QuicVersion; @@ -58,11 +59,13 @@ import static jdk.internal.net.http.quic.TerminationCause.forTransportError; import static jdk.internal.net.quic.QuicTransportErrors.NO_VIABLE_PATH; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; /* * @test + * @bug 8373877 * @summary verify that when a QUIC (client) connection receives a stateless reset * from the peer, then the connection is properly terminated * @library /test/lib /test/jdk/java/net/httpclient/lib @@ -87,6 +90,10 @@ public class StatelessResetReceiptTest { .availableVersions(new QuicVersion[]{QuicVersion.QUIC_V1}) .sslContext(sslContext) .build(); + // Use a longer max ack delay to inflate the draining time (3xPTO) + final QuicTransportParameters transportParameters = new QuicTransportParameters(); + transportParameters.setIntParameter(QuicTransportParameters.ParameterId.max_ack_delay, + (1 << 14) - 1); // 16 seconds, maximum allowed server.start(); System.out.println("Server started at " + server.getAddress()); } @@ -179,6 +186,9 @@ public class StatelessResetReceiptTest { conn.close(); // verify connection is no longer open assertFalse(conn.underlyingQuicConnection().isOpen(), "QUIC connection is still open"); + // Check that the (closing) connection is still registered with the endpoint + assertNotEquals(0, conn.endpoint().connectionCount(), + "Expected the QUIC connection to be registered"); // now send a stateless reset from the server connection sendStatelessResetFrom(serverConn); // wait for the stateless reset to be processed @@ -231,12 +241,19 @@ public class StatelessResetReceiptTest { .loggedAs("intentionally closed by server to initiate draining state" + " on client connection"); serverConn.connectionTerminator().terminate(tc); + // send ping in case the connection_close message gets lost + conn.underlyingQuicConnection().requestSendPing(); // wait for client conn to terminate final TerminationCause clientTC = ((QuicConnectionImpl) conn.underlyingQuicConnection()) .futureTerminationCause().get(); // verify connection closed for the right reason assertEquals(NO_VIABLE_PATH.code(), clientTC.getCloseCode(), "unexpected termination cause"); + // wait for a while to let the connection close completely + Thread.sleep(10); + // Check that the (draining) connection is still registered with the endpoint + assertNotEquals(0, conn.endpoint().connectionCount(), + "Expected the QUIC connection to be registered"); // now send a stateless reset from the server connection sendStatelessResetFrom(serverConn); // wait for the stateless reset to be processed