130 lines
5.7 KiB
Diff
130 lines
5.7 KiB
Diff
From 7f748eb6bfaba5207c89dbd7d5adf50fae847145 Mon Sep 17 00:00:00 2001
|
|
From: Mark Thomas <markt@apache.org>
|
|
Date: Tue, 30 Apr 2019 22:18:12 +0100
|
|
Subject: [PATCH] Expand HTTP/2 timeout handling to connection window
|
|
exhaustion on write.
|
|
|
|
https://github.com/apache/tomcat/commit/7f748eb
|
|
---
|
|
.../coyote/http2/Http2UpgradeHandler.java | 32 +++++++++++++++++--
|
|
java/org/apache/coyote/http2/Stream.java | 27 +++++++++-------
|
|
webapps/docs/changelog.xml | 4 +++
|
|
3 files changed, 50 insertions(+), 13 deletions(-)
|
|
|
|
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
|
|
index 1d8770a..ab0369a 100644
|
|
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
|
|
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
|
|
@@ -794,7 +794,26 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
|
|
}
|
|
if (allocation == 0) {
|
|
try {
|
|
- stream.wait();
|
|
+ // Connection level window is empty. Although this
|
|
+ // request is for a stream, use the connection
|
|
+ // timeout
|
|
+ long writeTimeout = protocol.getWriteTimeout();
|
|
+ if (writeTimeout < 0) {
|
|
+ stream.wait();
|
|
+ } else {
|
|
+ stream.wait(writeTimeout);
|
|
+ }
|
|
+ // Has this stream been granted an allocation
|
|
+ int[] value = backLogStreams.get(stream);
|
|
+ if (value[1] == 0) {
|
|
+ // No allocation
|
|
+ // Close the connection. Do this first since
|
|
+ // closing the stream will raise an exception
|
|
+ close();
|
|
+ // Close the stream (in app code so need to
|
|
+ // signal to app stream is closing)
|
|
+ stream.doWriteTimeout();
|
|
+ }
|
|
} catch (InterruptedException e) {
|
|
throw new IOException(sm.getString(
|
|
"upgradeHandler.windowSizeReservationInterrupted", connectionId,
|
|
@@ -985,11 +1004,20 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
|
|
|
|
|
|
private void close() {
|
|
- connectionState.set(ConnectionState.CLOSED);
|
|
+ ConnectionState previous = connectionState.getAndSet(ConnectionState.CLOSED);
|
|
+ if (previous == ConnectionState.CLOSED) {
|
|
+ // Already closed
|
|
+ return;
|
|
+ }
|
|
+
|
|
for (Stream stream : streams.values()) {
|
|
// The connection is closing. Close the associated streams as no
|
|
// longer required.
|
|
stream.receiveReset(Http2Error.CANCEL.getCode());
|
|
+ // Release any streams waiting for an allocation
|
|
+ synchronized (stream) {
|
|
+ stream.notifyAll();
|
|
+ }
|
|
}
|
|
try {
|
|
socketWrapper.close();
|
|
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
|
|
index 2c4f67e..8b87b12 100644
|
|
--- a/java/org/apache/coyote/http2/Stream.java
|
|
+++ b/java/org/apache/coyote/http2/Stream.java
|
|
@@ -219,17 +219,7 @@ class Stream extends AbstractStream implements HeaderEmitter {
|
|
}
|
|
windowSize = getWindowSize();
|
|
if (windowSize == 0) {
|
|
- String msg = sm.getString("stream.writeTimeout");
|
|
- StreamException se = new StreamException(
|
|
- msg, Http2Error.ENHANCE_YOUR_CALM, getIdentifier().intValue());
|
|
- // Prevent the application making further writes
|
|
- streamOutputBuffer.closed = true;
|
|
- // Prevent Tomcat's error handling trying to write
|
|
- coyoteResponse.setError();
|
|
- coyoteResponse.setErrorReported();
|
|
- // Trigger a reset once control returns to Tomcat
|
|
- streamOutputBuffer.reset = se;
|
|
- throw new CloseNowException(msg, se);
|
|
+ doWriteTimeout();
|
|
}
|
|
} else {
|
|
return 0;
|
|
@@ -252,6 +242,21 @@ class Stream extends AbstractStream implements HeaderEmitter {
|
|
}
|
|
|
|
|
|
+ void doWriteTimeout() throws CloseNowException {
|
|
+ String msg = sm.getString("stream.writeTimeout");
|
|
+ StreamException se = new StreamException(
|
|
+ msg, Http2Error.ENHANCE_YOUR_CALM, getIdentifier().intValue());
|
|
+ // Prevent the application making further writes
|
|
+ streamOutputBuffer.closed = true;
|
|
+ // Prevent Tomcat's error handling trying to write
|
|
+ coyoteResponse.setError();
|
|
+ coyoteResponse.setErrorReported();
|
|
+ // Trigger a reset once control returns to Tomcat
|
|
+ streamOutputBuffer.reset = se;
|
|
+ throw new CloseNowException(msg, se);
|
|
+ }
|
|
+
|
|
+
|
|
@Override
|
|
public final void emitHeader(String name, String value) throws HpackException {
|
|
if (log.isDebugEnabled()) {
|
|
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
|
|
index a8abf2d..5665df4 100644
|
|
--- a/webapps/docs/changelog.xml
|
|
+++ b/webapps/docs/changelog.xml
|
|
@@ -362,6 +362,10 @@
|
|
<update>
|
|
Update the internal fork of Commons DBCP 2 to 2.4.0. (markt)
|
|
</update>
|
|
+ <fix>
|
|
+ Expand HTTP/2 timeout handling to include connection window exhaustion
|
|
+ on write. (markt)
|
|
+ </fix>
|
|
</changelog>
|
|
</subsection>
|
|
</section>
|
|
--
|
|
2.19.1
|