From 40cf5b04655db95509a769bf600503d94f7b2914 Mon Sep 17 00:00:00 2001 From: Kai Burjack Date: Wed, 5 Nov 2025 23:21:36 +0100 Subject: [PATCH] Add drainTimeout property to Http2Protocol/Http2UpgradeHandler This is to delay sending the second GOAWAY (with last seen stream id) after sending the first GOAWAY (with max stream id), in order to mitigate a race between the client already having buffered frames for new streams after the server has sent the first graceful GOAWAY frame and the second GOAWAY. Once the second GOAWAY frame reaches the client, its own HTTP/2 client implementation may then itself signal a stream reset (rejected stream) error condition to the client application or might send the new forbidden stream frames to the server, where it then would be rejected. Therefore, we implement the same behaviour as provided by the Envoy proxy's HTTP Connection Manager drain_timeout: Delay sending the second final GOAWAY after sending the first graceful GOAWAY to give the client enough time to react to the first GOAWAY and stop sending new streams and close the connection on its side first. --- .../apache/coyote/http2/Http2Protocol.java | 23 +++++++++++++++++++ .../coyote/http2/Http2UpgradeHandler.java | 6 ++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/java/org/apache/coyote/http2/Http2Protocol.java b/java/org/apache/coyote/http2/Http2Protocol.java index b8a7d6fde68d..efc961ffce9b 100644 --- a/java/org/apache/coyote/http2/Http2Protocol.java +++ b/java/org/apache/coyote/http2/Http2Protocol.java @@ -112,6 +112,19 @@ public class Http2Protocol implements UpgradeProtocol { private boolean discardRequestsAndResponses = false; private final SynchronizedStack recycledRequestsAndResponses = new SynchronizedStack<>(); + /* + * Additional time in nanoseconds between sending the first graceful GOAWAY (max stream id) and the final GOAWAY + * (last seen stream id). During this time the server will continue to process new streams on the connection. + * This is to mitigate the race of client-buffered/sent packets for new streams and the final GOAWAY (with last + * seen stream id). By default, Tomcat uses the last computed RTT for this interval, but the RTT might have + * fluctuated due to network or server load conditions, or the client (e.g. nghttp2) might have already buffered + * frames for opening new streams on a connection. + * + * The name "drainTimeout" is taken from Envoy proxy's identical HTTP Connection Manager property and means exactly + * the same. + */ + private long drainTimeout; + @Override public String getHttpUpgradeName(boolean isSSLEnabled) { if (isSSLEnabled) { @@ -409,6 +422,16 @@ public void setDiscardRequestsAndResponses(boolean discardRequestsAndResponses) } + public long getDrainTimeout() { + return drainTimeout; + } + + + public void setDrainTimeout(long drainTimeout) { + this.drainTimeout = drainTimeout; + } + + Request popRequestAndResponse() { Request requestAndResponse = null; if (!discardRequestsAndResponses) { diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index 4ac67284e3bd..b84e2ca45bea 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -140,6 +140,8 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH private volatile int lastNonFinalDataPayload; private volatile int lastWindowUpdate; + // Time between the "graceful" GOAWAY (max stream id) and the final GOAWAY (last seen stream id) + private long drainTimeout = 0; Http2UpgradeHandler(Http2Protocol protocol, Adapter adapter, Request coyoteRequest, SocketWrapperBase socketWrapper) { @@ -173,6 +175,8 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH pingManager.initiateDisabled = protocol.getInitiatePingDisabled(); + drainTimeout = protocol.getDrainTimeout(); + // Initial HTTP request becomes stream 1. if (coyoteRequest != null) { if (log.isTraceEnabled()) { @@ -543,7 +547,7 @@ public void destroy() { void checkPauseState() throws IOException { if (connectionState.get() == ConnectionState.PAUSING) { - if (pausedNanoTime + pingManager.getRoundTripTimeNano() < System.nanoTime()) { + if (pausedNanoTime + pingManager.getRoundTripTimeNano() + drainTimeout < System.nanoTime()) { connectionState.compareAndSet(ConnectionState.PAUSING, ConnectionState.PAUSED); writeGoAwayFrame(maxProcessedStreamId, Http2Error.NO_ERROR.getCode(), null); }