8373877: QUIC connections are removed too early

Reviewed-by: dfuchs
This commit is contained in:
Daniel Jeliński 2025-12-18 13:17:44 +00:00
parent b848ddf6d3
commit 7a7e7c9ae1
4 changed files with 22 additions and 8 deletions

View File

@ -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);
}

View File

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

View File

@ -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);
}

View File

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