From 92e1357dfd2d874ef1a62ddd69c86a7bb189c6a2 Mon Sep 17 00:00:00 2001 From: Jaikiran Pai Date: Sat, 29 Nov 2025 01:25:25 +0000 Subject: [PATCH] 8371802: Do not let QUIC connection to idle terminate when HTTP/3 is configured with a higher idle timeout Reviewed-by: dfuchs --- .../internal/net/http/Http3Connection.java | 68 +++- .../net/http/Http3ConnectionPool.java | 29 +- .../net/http/quic/ConnectionTerminator.java | 70 +++- .../http/quic/ConnectionTerminatorImpl.java | 12 +- .../net/http/quic/IdleTimeoutManager.java | 316 +++++++++++++----- .../net/http/quic/QuicConnectionImpl.java | 4 +- .../net/http/quic/QuicTimedEvent.java | 3 +- .../http3/H3IdleExceedsQuicIdleTimeout.java | 178 ++++++++++ 8 files changed, 579 insertions(+), 101 deletions(-) create mode 100644 test/jdk/java/net/httpclient/http3/H3IdleExceedsQuicIdleTimeout.java diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http3Connection.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http3Connection.java index b97a441881d..17f230f3b15 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http3Connection.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http3Connection.java @@ -126,11 +126,16 @@ public final class Http3Connection implements AutoCloseable { // as per spec // -1 is used to imply no GOAWAY received so far private final AtomicLong lowestGoAwayReceipt = new AtomicLong(-1); + + private final Duration idleTimeoutDuration; private volatile IdleConnectionTimeoutEvent idleConnectionTimeoutEvent; // value of true implies no more streams will be initiated on this connection, // and the connection will be closed once the in-progress streams complete. private volatile boolean finalStream; private volatile boolean allowOnlyOneStream; + // true if this connection has been placed in the HTTP/3 connection pool of the HttpClient. + // false otherwise. + private volatile boolean presentInConnPool; // set to true if we decide to open a new connection // due to stream limit reached private volatile boolean streamLimitReached; @@ -220,6 +225,17 @@ public final class Http3Connection implements AutoCloseable { // in case of exception. Throws in the dependent // action after wrapping the exception if needed. .exceptionally(this::exceptionallyAndClose); + + this.idleTimeoutDuration = client.client().idleConnectionTimeout(HTTP_3).orElse(null); + if (idleTimeoutDuration == null) { + // The absence of HTTP/3 idle timeout duration is considered to mean + // never idle terminating the connection + quicConnection.connectionTerminator().appLayerMaxIdle(Duration.MAX, + this::isQUICTrafficGenerationRequired); + } else { + quicConnection.connectionTerminator().appLayerMaxIdle(idleTimeoutDuration, + this::isQUICTrafficGenerationRequired); + } if (Log.http3()) { Log.logHttp3("HTTP/3 connection created for " + quicConnectionTag() + " - local address: " + quicConnection.localAddress()); @@ -732,9 +748,8 @@ public final class Http3Connection implements AutoCloseable { try { var te = idleConnectionTimeoutEvent; if (te == null && exchangeStreams.isEmpty()) { - te = idleConnectionTimeoutEvent = client.client().idleConnectionTimeout(HTTP_3) - .map(IdleConnectionTimeoutEvent::new).orElse(null); - if (te != null) { + if (idleTimeoutDuration != null) { + te = idleConnectionTimeoutEvent = new IdleConnectionTimeoutEvent(); client.client().registerTimer(te); } } @@ -873,6 +888,48 @@ public final class Http3Connection implements AutoCloseable { } } + /** + * Mark this connection as being present or absent from the connection pool. + */ + void setPooled(final boolean present) { + this.presentInConnPool = present; + } + + /** + * This callback method is invoked by the QUIC layer when it notices that this + * connection hasn't seen any traffic for certain period of time. QUIC + * invokes this method to ask HTTP/3 whether the QUIC layer + * should generate traffic to keep this connection active. + * This method returns true, indicating that the traffic must be generated, + * if this HTTP/3 connection is in pool and there's no current request/response + * in progress over this connection (i.e. the HTTP/3 connection is idle in the + * pool waiting for any new requests to be issued by the application). + */ + private boolean isQUICTrafficGenerationRequired() { + if (!isOpen()) { + return false; + } + lock(); + try { + // if there's no HTTP/3 request/responses in progress and the connection is + // in the pool (thus idle), then we instruct QUIC to generate traffic on the + // QUIC connection to prevent it from being idle terminated. + final boolean generateTraffic = this.presentInConnPool + && this.exchanges.isEmpty() + && this.reservedStreamCount.get() == 0 + // a connection in the pool could be marked as + // finalStream (for example when it receives a GOAWAY). we don't want + // to generate explicit QUIC traffic for such connections too. + && !this.finalStream; + if (debug.on()) { + debug.log("QUIC traffic generation required = " + generateTraffic); + } + return generateTraffic; + } finally { + unlock(); + } + } + /** * Cancels any event that might have been scheduled to shutdown this connection. Must be called * with the stateLock held. @@ -893,8 +950,9 @@ public final class Http3Connection implements AutoCloseable { private boolean cancelled; private boolean idleShutDownInitiated; - IdleConnectionTimeoutEvent(Duration duration) { - super(duration); + IdleConnectionTimeoutEvent() { + assert idleTimeoutDuration != null : "idle timeout duration is null"; + super(idleTimeoutDuration); } @Override diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ConnectionPool.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http3ConnectionPool.java index eaacd8213cb..0f1fe2b7abf 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ConnectionPool.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http3ConnectionPool.java @@ -155,23 +155,34 @@ class Http3ConnectionPool { assert key.equals(c.key()); var altService = c.connection().getSourceAltService().orElse(null); if (altService != null && altService.wasAdvertised()) { - return advertised.putIfAbsent(key, c); + final var prev = advertised.putIfAbsent(key, c); + if (prev == null) { + c.setPooled(true); // mark the newly pooled connection as pooled + } + return prev; } assert altService == null || altService.originHasSameAuthority(); - return unadvertised.putIfAbsent(key, c); + final var prev = unadvertised.putIfAbsent(key, c); + if (prev == null) { + c.setPooled(true); // mark the newly pooled connection as pooled + } + return prev; } - Http3Connection put(String key, Http3Connection c) { + void put(String key, Http3Connection c) { Objects.requireNonNull(key); Objects.requireNonNull(c); assert key.equals(c.key()) : "key mismatch %s -> %s" .formatted(key, c.key()); var altService = c.connection().getSourceAltService().orElse(null); if (altService != null && altService.wasAdvertised()) { - return advertised.put(key, c); + advertised.put(key, c); + c.setPooled(true); + return; } assert altService == null || altService.originHasSameAuthority(); - return unadvertised.put(key, c); + unadvertised.put(key, c); + c.setPooled(true); } boolean remove(String key, Http3Connection c) { @@ -189,11 +200,17 @@ class Http3ConnectionPool { } assert altService == null || altService.originHasSameAuthority(); - return unadvertised.remove(key, c); + final boolean removed = unadvertised.remove(key, c); + if (removed) { + c.setPooled(false); + } + return removed; } void clear() { + advertised.values().forEach((c) -> c.setPooled(false)); advertised.clear(); + unadvertised.values().forEach((c) -> c.setPooled(false)); unadvertised.clear(); } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminator.java b/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminator.java index 24230a0883d..14559db4a11 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminator.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/quic/ConnectionTerminator.java @@ -24,15 +24,77 @@ */ package jdk.internal.net.http.quic; -// responsible for managing the connection termination of a QUIC connection +import java.time.Duration; +import java.util.function.Supplier; + +/** + * Responsible for managing the connection termination of a QUIC connection + */ public sealed interface ConnectionTerminator permits ConnectionTerminatorImpl { - // lets the terminator know that the connection is still alive and should not be - // idle timed out - void keepAlive(); + /** + * Instructs the connection terminator to consider the connection as active + * at the present point in time. The connection terminator will then restart its + * idle timeout timer from this point in time. + *

+ * This method must be called when an incoming packet is processed successfully + * or when an ack-eliciting packet is sent by the local endpoint on the connection. + */ + void markActive(); + /** + * Terminates the connection, if not already terminated, with the given cause. + *

+ * A connection is terminated only once with a {@link TerminationCause}. However, this method + * can be called any number of times. If the connection is not already terminated, + * then this method does the necessary work to terminate the connection. Any subsequent + * invocations of this method, after the connection has been terminated, will not + * change the termination cause of the connection. + * + * @param cause the termination cause + */ void terminate(TerminationCause cause); + /** + * Returns {@code true} if the connection is allowed for use, {@code false} otherwise. + *

+ * This method is typically called when a connection that has been idle, is about to be used + * for handling some request. This method allows for co-ordination between the connection usage + * and the connection terminator to prevent the connection from being idle timed out when it is + * about to be used for some request. The connection must only be used if this method + * returns {@code true}. + * + * @return true if the connection can be used, false otherwise + */ boolean tryReserveForUse(); + /** + * Instructs the connection terminator that the application layer allows the + * connection to stay idle for the given {@code maxIdle} duration. If the QUIC + * layer has negotiated an idle timeout for the connection, that's lower than + * the application's {@code maxIdle} duration, then the connection terminator + * upon noticing absence of traffic over the connection for certain duration, + * calls the {@code trafficGenerationCheck} to check if the QUIC layer should + * explicitly generate some traffic to prevent the connection + * from idle terminating. + *

+ * When the {@code trafficGenerationCheck} is invoked, the application layer + * must return {@code true} only if explicit traffic generation is necessary + * to keep the connection alive. + *

+ * If the application layer wishes to never idle terminate the connection, then + * a {@code maxIdle} duration of {@linkplain Duration#MAX Duration.MAX} is recommended. + * + * @param maxIdle the maximum idle duration of the connection, + * at the application layer + * @param trafficGenerationCheck the callback that will be invoked by the connection + * terminator to decide if the QUIC layer should generate + * any traffic to prevent the connection from idle terminating + * @throws NullPointerException if either {@code maxIdle} or {@code trafficGenerationCheck} + * is null + * @throws IllegalArgumentException if {@code maxIdle} is + * {@linkplain Duration#isNegative() negative} or + * {@linkplain Duration#isZero() zero} + */ + void appLayerMaxIdle(Duration maxIdle, Supplier trafficGenerationCheck); } 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 0f5aa4cb7ac..7870f4f1d8e 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 @@ -25,11 +25,13 @@ package jdk.internal.net.http.quic; import java.nio.ByteBuffer; +import java.time.Duration; import java.util.List; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; +import java.util.function.Supplier; import jdk.internal.net.http.common.Log; import jdk.internal.net.http.common.Logger; @@ -52,7 +54,6 @@ import static jdk.internal.net.http.quic.QuicConnectionImpl.QuicConnectionState. import static jdk.internal.net.http.quic.TerminationCause.appLayerClose; import static jdk.internal.net.http.quic.TerminationCause.forSilentTermination; import static jdk.internal.net.http.quic.TerminationCause.forTransportError; -import static jdk.internal.net.quic.QuicTransportErrors.INTERNAL_ERROR; import static jdk.internal.net.quic.QuicTransportErrors.NO_ERROR; final class ConnectionTerminatorImpl implements ConnectionTerminator { @@ -70,8 +71,8 @@ final class ConnectionTerminatorImpl implements ConnectionTerminator { } @Override - public void keepAlive() { - this.connection.idleTimeoutManager.keepAlive(); + public void markActive() { + this.connection.idleTimeoutManager.markActive(); } @Override @@ -79,6 +80,11 @@ final class ConnectionTerminatorImpl implements ConnectionTerminator { return this.connection.idleTimeoutManager.tryReserveForUse(); } + @Override + public void appLayerMaxIdle(final Duration maxIdle, final Supplier trafficGenerationCheck) { + this.connection.idleTimeoutManager.appLayerMaxIdle(maxIdle, trafficGenerationCheck); + } + @Override public void terminate(final TerminationCause cause) { Objects.requireNonNull(cause); diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/quic/IdleTimeoutManager.java b/src/java.net.http/share/classes/jdk/internal/net/http/quic/IdleTimeoutManager.java index 72ce0290038..375a0dc8e16 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/quic/IdleTimeoutManager.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/quic/IdleTimeoutManager.java @@ -24,12 +24,15 @@ */ package jdk.internal.net.http.quic; +import java.time.Duration; import java.util.Objects; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Supplier; import jdk.internal.net.http.common.Deadline; import jdk.internal.net.http.common.Log; @@ -58,17 +61,21 @@ final class IdleTimeoutManager { private IdleTimeoutEvent idleTimeoutEvent; // must be accessed only when holding stateLock private StreamDataBlockedEvent streamDataBlockedEvent; - // the time at which the last outgoing packet was sent or an + // the time (in nanos) at which the last outgoing packet was sent or an // incoming packet processed on the connection - private volatile long lastPacketActivityAt; + private volatile long lastPacketActivityNanos; private final ReentrantLock idleTerminationLock = new ReentrantLock(); // true if it has been decided to terminate the connection due to being idle, // false otherwise. should be accessed only when holding the idleTerminationLock private boolean chosenForIdleTermination; - // the time at which the connection was last reserved for use. + // the time (in nanos) at which the connection was last reserved for use. // should be accessed only when holding the idleTerminationLock - private long lastUsageReservationAt; + private long lastUsageReservationNanos; + + private final AtomicReference> trafficGenerationCheck = new AtomicReference<>(); + // must be accessed only when holding stateLock + private PingEvent pingEvent; IdleTimeoutManager(final QuicConnectionImpl connection) { this.connection = Objects.requireNonNull(connection, "connection"); @@ -79,28 +86,12 @@ final class IdleTimeoutManager { * Starts the idle timeout management for the connection. This should be called * after the handshake is complete for the connection. * - * @throw IllegalStateException if handshake hasn't yet completed or if the handshake - * has failed for the connection + * @throws IllegalStateException if handshake hasn't yet completed or if the handshake + * has failed for the connection */ void start() { - final CompletableFuture handshakeCF = - this.connection.handshakeFlow().handshakeCF(); // start idle management only for successfully completed handshake - if (!handshakeCF.isDone()) { - throw new IllegalStateException("handshake isn't yet complete," - + " cannot start idle connection management"); - } - if (handshakeCF.isCompletedExceptionally()) { - throw new IllegalStateException("cannot start idle connection management for a failed" - + " connection"); - } - startTimers(); - } - - /** - * Starts the idle timeout timer of the QUIC connection, if not already started. - */ - private void startTimers() { + requireSuccessfulHandshake(); if (shutdown.get()) { return; } @@ -116,6 +107,19 @@ final class IdleTimeoutManager { } } + private void requireSuccessfulHandshake() { + final CompletableFuture handshakeCF = + this.connection.handshakeFlow().handshakeCF(); + if (!handshakeCF.isDone()) { + throw new IllegalStateException("handshake isn't yet complete," + + " cannot use idle connection management"); + } + if (handshakeCF.isCompletedExceptionally()) { + throw new IllegalStateException("cannot use idle connection management for a failed" + + " connection"); + } + } + private void startIdleTerminationTimer() { assert stateLock.isHeldByCurrentThread() : "not holding state lock"; final Optional idleTimeoutMillis = getIdleTimeout(); @@ -154,18 +158,14 @@ final class IdleTimeoutManager { } final QuicEndpoint endpoint = this.connection.endpoint(); assert endpoint != null : "QUIC endpoint is null"; - // disable the event (refreshDeadline() of IdleTimeoutEvent will return Deadline.MAX) - final Deadline nextDeadline = this.idleTimeoutEvent.nextDeadline; - if (!nextDeadline.equals(Deadline.MAX)) { - this.idleTimeoutEvent.nextDeadline = Deadline.MAX; - endpoint.timer().reschedule(this.idleTimeoutEvent, Deadline.MIN); - } + // disable the idle timeout timer event + disableTimedEvent(endpoint.timer(), this.idleTimeoutEvent); this.idleTimeoutEvent = null; } private void startStreamDataBlockedTimer() { assert stateLock.isHeldByCurrentThread() : "not holding state lock"; - // 75% of idle timeout or if idle timeout is not configured, then 30 seconds + // 75% of QUIC idle timeout or if QUIC idle timeout is not configured, then 30 seconds final long timeoutMillis = getIdleTimeout() .map((v) -> (long) (0.75 * v)) .orElse(30000L); @@ -194,22 +194,95 @@ final class IdleTimeoutManager { } final QuicEndpoint endpoint = this.connection.endpoint(); assert endpoint != null : "QUIC endpoint is null"; - // disable the event (refreshDeadline() of StreamDataBlockedEvent will return Deadline.MAX) - final Deadline nextDeadline = this.streamDataBlockedEvent.nextDeadline; - if (!nextDeadline.equals(Deadline.MAX)) { - this.streamDataBlockedEvent.nextDeadline = Deadline.MAX; - endpoint.timer().reschedule(this.streamDataBlockedEvent, Deadline.MIN); - } + // disable the stream data blocked timer event + disableTimedEvent(endpoint.timer(), this.streamDataBlockedEvent); this.streamDataBlockedEvent = null; } + // set up a PING timer if the application layer's max idle duration for the connection + // is larger than that of the negotiated QUIC idle timeout for that connection + void appLayerMaxIdle(final Duration maxIdle, final Supplier trafficGenerationCheck) { + Objects.requireNonNull(maxIdle, "maxIdle"); + Objects.requireNonNull(trafficGenerationCheck, "trafficGenerationCheck"); + if (maxIdle.isZero() || maxIdle.isNegative()) { + throw new IllegalArgumentException("invalid maxIdle duration: " + maxIdle); + } + // the application layer must not configure its max idle duration + // until the QUIC connection's handshake has successfully completed + requireSuccessfulHandshake(); + + if (!this.trafficGenerationCheck.compareAndSet(null, trafficGenerationCheck)) { + throw new IllegalStateException("app layer max inactivity already set"); + } + final Optional quicIdleTimeout = getIdleTimeout(); + if (quicIdleTimeout.isEmpty()) { + // the QUIC connection will never idle timeout, nothing more to do + return; + } + // we start the PING sending timer event only if the QUIC layer idle timeout + // is lesser than the app layer's desired idle time + if (Duration.ofMillis(quicIdleTimeout.get()).compareTo(maxIdle) < 0) { + this.stateLock.lock(); + try { + if (shutdown.get()) { + return; + } + // QUIC connection has a lower idle timeout than the app layer. start a timer + // which checks with the app layer at regular intervals to decide whether to + // send a PING to keep the QUIC connection active. + startPingTimer(); + } finally { + this.stateLock.unlock(); + } + } + } + + private void startPingTimer() { + assert stateLock.isHeldByCurrentThread() : "not holding state lock"; + // we don't expect the timer to be started more than once + assert this.pingEvent == null : "PING timer already started"; + final Optional quicIdleTimeout = getIdleTimeout(); + assert quicIdleTimeout.isPresent() : "QUIC idle timeout is disabled, no need to start PING timer"; + // potential PING generation every 75% of QUIC idle timeout + final long pingFrequencyMillis = (long) (0.75 * quicIdleTimeout.get()); + assert pingFrequencyMillis > 0 : "unexpected ping frequency: " + pingFrequencyMillis; + final QuicTimerQueue timerQueue = connection.endpoint().timer(); + final Deadline deadline = timeLine().instant().plusMillis(pingFrequencyMillis); + // create the timeout event and register with the QuicTimerQueue. + this.pingEvent = new PingEvent(deadline, pingFrequencyMillis); + timerQueue.offer(this.pingEvent); + if (debug.on()) { + debug.log("started periodic PING for connection," + + " ping event: " + this.pingEvent + + " deadline: " + deadline); + } else { + Log.logQuic("{0} started periodic PING for connection," + + " ping event: {1} deadline: {2}", + connection.logTag(), this.pingEvent, deadline); + } + } + + private void stopPingTimer() { + assert stateLock.isHeldByCurrentThread() : "not holding state lock"; + if (this.pingEvent == null) { + return; + } + final QuicEndpoint endpoint = this.connection.endpoint(); + assert endpoint != null : "QUIC endpoint is null"; + // disable the ping timer event + disableTimedEvent(endpoint.timer(), this.pingEvent); + this.pingEvent = null; + this.trafficGenerationCheck.set(null); + } + + /** * Attempts to notify the idle connection management that this connection should * be considered "in use". This way the idle connection management doesn't close * this connection during the time the connection is handed out from the pool and any * new stream created on that connection. * - * @return true if the connection has been successfully reserved and is {@link #isOpen()}. false + * @return true if the connection has been successfully reserved. false * otherwise; in which case the connection must not be handed out from the pool. */ boolean tryReserveForUse() { @@ -221,7 +294,7 @@ final class IdleTimeoutManager { } // if the connection is nearing idle timeout due to lack of traffic then // don't use it - final long lastPktActivity = lastPacketActivityAt; + final long lastPktActivity = lastPacketActivityNanos; final long currentNanos = System.nanoTime(); final long inactivityMs = MILLISECONDS.convert((currentNanos - lastPktActivity), NANOSECONDS); @@ -232,7 +305,7 @@ final class IdleTimeoutManager { return false; } // express interest in using the connection - this.lastUsageReservationAt = System.nanoTime(); + this.lastUsageReservationNanos = System.nanoTime(); return true; } finally { this.idleTerminationLock.unlock(); @@ -256,8 +329,9 @@ final class IdleTimeoutManager { return val == NO_IDLE_TIMEOUT ? Optional.empty() : Optional.of(val); } - void keepAlive() { - lastPacketActivityAt = System.nanoTime(); // TODO: timeline().instant()? + // consider the connection as active as of this moment + void markActive() { + lastPacketActivityNanos = System.nanoTime(); // TODO: timeline().instant()? } void shutdown() { @@ -270,6 +344,7 @@ final class IdleTimeoutManager { // unregister the timeout events from the QuicTimerQueue stopIdleTerminationTimer(); stopStreamDataBlockedTimer(); + stopPingTimer(); } finally { this.stateLock.unlock(); } @@ -318,6 +393,15 @@ final class IdleTimeoutManager { return this.connection.endpoint().timeSource(); } + private static void disableTimedEvent(final QuicTimerQueue timer, final TimedEvent te) { + // disable the event (refreshDeadline() of TimedEvent will return Deadline.MAX) + final Deadline nextDeadline = te.nextDeadline; + if (!nextDeadline.equals(Deadline.MAX)) { + te.nextDeadline = Deadline.MAX; + timer.reschedule(te, Deadline.MIN); + } + } + // called when the connection has been idle past its idle timeout duration private void idleTimedOut() { if (shutdown.get()) { @@ -354,35 +438,51 @@ final class IdleTimeoutManager { } private long computeInactivityMillis() { + assert idleTerminationLock.isHeldByCurrentThread() : "not holding idle termination lock"; final long currentNanos = System.nanoTime(); - final long lastActiveNanos = Math.max(lastPacketActivityAt, lastUsageReservationAt); + final long lastActiveNanos = Math.max(lastPacketActivityNanos, lastUsageReservationNanos); return MILLISECONDS.convert((currentNanos - lastActiveNanos), NANOSECONDS); } - final class IdleTimeoutEvent implements QuicTimedEvent { - private final long eventId; - private volatile Deadline deadline; - private volatile Deadline nextDeadline; + sealed abstract class TimedEvent implements QuicTimedEvent { + protected final long eventId; + protected volatile Deadline deadline; + protected volatile Deadline nextDeadline; - private IdleTimeoutEvent(final Deadline deadline) { + private TimedEvent(final Deadline deadline) { assert deadline != null : "timeout deadline is null"; this.deadline = this.nextDeadline = deadline; this.eventId = QuicTimerQueue.newEventId(); } @Override - public Deadline deadline() { + public final Deadline deadline() { return this.deadline; } @Override - public Deadline refreshDeadline() { + public final Deadline refreshDeadline() { if (shutdown.get()) { return this.deadline = this.nextDeadline = Deadline.MAX; } return this.deadline = this.nextDeadline; } + @Override + public final long eventId() { + return this.eventId; + } + + @Override + public abstract Deadline handle(); + } + + final class IdleTimeoutEvent extends TimedEvent { + + private IdleTimeoutEvent(final Deadline deadline) { + super(deadline); + } + @Override public Deadline handle() { if (shutdown.get()) { @@ -442,41 +542,18 @@ final class IdleTimeoutManager { } } - @Override - public long eventId() { - return this.eventId; - } - @Override public String toString() { return "QuicIdleTimeoutEvent-" + this.eventId; } } - final class StreamDataBlockedEvent implements QuicTimedEvent { - private final long eventId; + final class StreamDataBlockedEvent extends TimedEvent { private final long timeoutMillis; - private volatile Deadline deadline; - private volatile Deadline nextDeadline; private StreamDataBlockedEvent(final Deadline deadline, final long timeoutMillis) { - assert deadline != null : "timeout deadline is null"; - this.deadline = this.nextDeadline = deadline; + super(deadline); this.timeoutMillis = timeoutMillis; - this.eventId = QuicTimerQueue.newEventId(); - } - - @Override - public Deadline deadline() { - return this.deadline; - } - - @Override - public Deadline refreshDeadline() { - if (shutdown.get()) { - return this.deadline = this.nextDeadline = Deadline.MAX; - } - return this.deadline = this.nextDeadline; } @Override @@ -517,14 +594,95 @@ final class IdleTimeoutManager { } } - @Override - public long eventId() { - return this.eventId; - } - @Override public String toString() { return "StreamDataBlockedEvent-" + this.eventId; } } + + + final class PingEvent extends TimedEvent { + private final long pingFrequencyNanos; + private final long idleTimeoutNanos; + + private PingEvent(final Deadline deadline, final long pingFrequencyMillis) { + super(deadline); + this.pingFrequencyNanos = MILLISECONDS.toNanos(pingFrequencyMillis); + if (this.pingFrequencyNanos <= 0) { + throw new IllegalArgumentException("ping frequency is too small: " + + pingFrequencyMillis + " milliseconds"); + } + this.idleTimeoutNanos = MILLISECONDS.toNanos(getIdleTimeout().get()); + } + + @Override + public Deadline handle() { + if (shutdown.get()) { + // timeout manager is shutdown, nothing more to do + return this.nextDeadline = Deadline.MAX; + } + if (!shouldInitiateAppLayerCheck()) { + // reschedule for next round + this.nextDeadline = timeLine().instant().plusNanos(this.pingFrequencyNanos); + return this.nextDeadline; + } + // check with the app layer if traffic generation is required + final Supplier check = trafficGenerationCheck.get(); + if (check == null) { + // generateTrafficCheck can be null if the timeout manager was shutdown + // when this event handling was in progress. don't send a PING frame + // in that case. + assert shutdown.get() : "trafficGenerationCheck is absent"; + return this.nextDeadline = Deadline.MAX; + } + if (check.get()) { + // app layer OKed sending a PING + connection.requestSendPing(); + if (debug.on()) { + debug.log("enqueued a PING frame"); + } else { + Log.logQuic("{0} enqueued a PING frame", connection.logTag()); + } + } else { + // app layer told us not to send a PING. + // we skip the PING generation only for the current round, no need + // to disable future PING checks + if (debug.on()) { + debug.log("skipping PING generation"); + } else { + Log.logQuic("{0} skipping PING generation", connection.logTag()); + } + } + this.nextDeadline = timeLine().instant().plusNanos(this.pingFrequencyNanos); + return this.nextDeadline; + } + + @Override + public String toString() { + return "PingEvent-" + this.eventId; + } + + // returns true if the app layer traffic generation check needs to be invoked, + // false otherwise. + private boolean shouldInitiateAppLayerCheck() { + final long lastPktAt = lastPacketActivityNanos; + final long now = System.nanoTime(); + if ((now - lastPktAt) >= this.pingFrequencyNanos) { + // no traffic during the ping interval, initiate a app layer check + // to see if explicit traffic needs to be generated + return true; + } + // check if the connection will potentially idle terminate before the next + // ping check is scheduled, if yes, then initiate a app layer traffic + // generation check now + final long idleTerminationAt = lastPktAt + this.idleTimeoutNanos; + final long nextPingCheck = now + this.pingFrequencyNanos; + if (idleTerminationAt - nextPingCheck <= 0) { + return true; + } + // connection appears to be receiving traffic, no need to initiate app layer + // traffic generation check + return false; + } + } } 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 d90ad1b217e..240f90852bc 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 @@ -1987,7 +1987,7 @@ public class QuicConnectionImpl extends QuicConnection implements QuicPacketRece case NONE -> throw new InternalError("Unrecognized packet type"); } // packet has been processed successfully - connection isn't idle (RFC-9000, section 10.1) - this.terminator.keepAlive(); + this.terminator.markActive(); if (packetSpace != null) { packetSpace.packetReceived( packetType, @@ -2819,7 +2819,7 @@ public class QuicConnectionImpl extends QuicConnection implements QuicPacketRece // RFC-9000, section 10.1: An endpoint also restarts its idle timer when sending // an ack-eliciting packet ... if (packet.isAckEliciting()) { - this.terminator.keepAlive(); + this.terminator.markActive(); } } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicTimedEvent.java b/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicTimedEvent.java index 9269b12bf64..ac885c1950e 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicTimedEvent.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicTimedEvent.java @@ -45,8 +45,7 @@ sealed interface QuicTimedEvent permits PacketSpaceManager.PacketTransmissionTask, QuicTimerQueue.Marker, QuicEndpoint.ClosedConnection, - IdleTimeoutManager.IdleTimeoutEvent, - IdleTimeoutManager.StreamDataBlockedEvent, + IdleTimeoutManager.TimedEvent, QuicConnectionImpl.MaxInitialTimer { /** diff --git a/test/jdk/java/net/httpclient/http3/H3IdleExceedsQuicIdleTimeout.java b/test/jdk/java/net/httpclient/http3/H3IdleExceedsQuicIdleTimeout.java new file mode 100644 index 00000000000..6b62fcf6043 --- /dev/null +++ b/test/jdk/java/net/httpclient/http3/H3IdleExceedsQuicIdleTimeout.java @@ -0,0 +1,178 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.io.IOException; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpOption; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import javax.net.ssl.SSLContext; + +import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestExchange; +import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestHandler; +import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestServer; +import jdk.test.lib.net.SimpleSSLContext; +import jdk.test.lib.net.URIBuilder; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import static java.net.http.HttpClient.Builder.NO_PROXY; +import static java.net.http.HttpClient.Version.HTTP_3; +import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/* + * @test + * @bug 8371802 + * @summary verify that if a higher idle timeout is configured for a HTTP/3 connection + * then a lower negotiated QUIC idle timeout doesn't cause QUIC to + * idle terminate the connection + * @library /test/lib /test/jdk/java/net/httpclient/lib + * @build jdk.httpclient.test.lib.common.HttpServerAdapters + * jdk.test.lib.net.SimpleSSLContext + * + * @comment this test has some explicit delays to simulate a idle connection, the timeout=180 + * is merely to provide some leeway to the test and prevent timing out the test on busy + * systems + * @run junit/othervm/timeout=180 -Djdk.httpclient.quic.idleTimeout=30 + * -Djdk.httpclient.keepalive.timeout.h3=120 + * ${test.main.class} + */ +class H3IdleExceedsQuicIdleTimeout { + + private static final String REQ_PATH = "/8371802"; + + private static HttpTestServer h3Server; + private static SSLContext sslCtx; + + @BeforeAll + static void beforeAll() throws Exception { + sslCtx = new SimpleSSLContext().get(); + assert sslCtx != null : "SSLContext is null"; + h3Server = HttpTestServer.create(HTTP_3_URI_ONLY, sslCtx); + h3Server.addHandler(new Handler(), REQ_PATH); + h3Server.start(); + System.err.println("HTTP/3 server started at " + h3Server.getAddress()); + } + + @AfterAll + static void afterAll() throws Exception { + if (h3Server != null) { + System.err.println("stopping server at " + h3Server.getAddress()); + h3Server.stop(); + } + } + + /* + * With QUIC idle connection timeout configured to be lower than the HTTP/3 idle timeout, + * this test issues a HTTP/3 request and expects that request to establish a QUIC connection + * and receive a successful response. The test then stays idle for a duration larger + * than the QUIC idle timeout and issues a HTTP/3 request again after that idle period. + * The test then expects that the second request too is responded by the previously opened + * connection, thus proving that the QUIC layer did the necessary work to prevent the (idle) + * connection from terminating. + */ + @Test + void testQUICKeepsConnAlive() throws Exception { + final long quicIdleTimeoutSecs = 30; + assertEquals(quicIdleTimeoutSecs, + Integer.parseInt(System.getProperty("jdk.httpclient.quic.idleTimeout")), + "unexpected QUIC idle timeout"); + assertEquals(120, + Integer.parseInt(System.getProperty("jdk.httpclient.keepalive.timeout.h3")), + "unexpected HTTP/3 idle timeout"); + try (final HttpClient client = HttpClient.newBuilder() + .sslContext(sslCtx) + .proxy(NO_PROXY) + .version(HTTP_3) + .build()) { + + final URI req1URI = URIBuilder.newBuilder() + .scheme("https") + .host(h3Server.getAddress().getAddress()) + .port(h3Server.getAddress().getPort()) + .path(REQ_PATH) + .query("i=1") + .build(); + System.err.println("issuing request " + req1URI); + final HttpRequest req1 = HttpRequest.newBuilder() + .uri(req1URI) + .setOption(HttpOption.H3_DISCOVERY, HTTP_3_URI_ONLY) + .build(); + final HttpResponse resp1 = client.send(req1, BodyHandlers.discarding()); + assertEquals(200, resp1.statusCode(), "unexpected status code"); + final String resp1ConnLabel = resp1.connectionLabel().orElse(null); + System.err.println("first request handled by connection: " + resp1ConnLabel); + assertNotNull(resp1ConnLabel, "missing connection label on response"); + assertEquals(HTTP_3, resp1.version(), "unexpected response version"); + // don't generate any more traffic from the HTTP/3 side for longer than the QUIC + // idle timeout + stayIdle(quicIdleTimeoutSecs + 13, TimeUnit.SECONDS); + // now send the HTTP/3 request and expect the same previous connection to handle + // respond to this request + final URI req2URI = URIBuilder.newBuilder() + .scheme("https") + .host(h3Server.getAddress().getAddress()) + .port(h3Server.getAddress().getPort()) + .path(REQ_PATH) + .query("i=2") + .build(); + System.err.println("issuing request " + req2URI); + final HttpRequest req2 = HttpRequest.newBuilder() + .uri(req2URI) + .setOption(HttpOption.H3_DISCOVERY, HTTP_3_URI_ONLY) + .build(); + final HttpResponse resp2 = client.send(req2, BodyHandlers.discarding()); + assertEquals(200, resp2.statusCode(), "unexpected status code"); + final String resp2ConnLabel = resp2.connectionLabel().orElse(null); + System.err.println("second request handled by connection: " + resp2ConnLabel); + assertEquals(resp1ConnLabel, resp2ConnLabel, "second request handled by a different connection"); + assertEquals(HTTP_3, resp2.version(), "unexpected response version"); + } + } + + private static void stayIdle(final long time, final TimeUnit unit) throws InterruptedException { + // await on a CountDownLatch which no one counts down. this is merely + // to avoid using Thread.sleep(...) and other similar constructs and then + // having to deal with spurious wakeups. + final boolean countedDown = new CountDownLatch(1).await(time, unit); + assertFalse(countedDown, "wasn't expected to be counted down"); + } + + private static final class Handler implements HttpTestHandler { + private static final int NO_RESP_BODY = 0; + + @Override + public void handle(final HttpTestExchange exchange) throws IOException { + System.err.println("handling request " + exchange.getRequestURI()); + exchange.sendResponseHeaders(200, NO_RESP_BODY); + } + } +}