This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 244c938bc09649c246126c6be0b18029396e42fb
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Aug 18 20:50:01 2020 +0100
Improve handling of flow control errors.
---
java/org/apache/coyote/http2/Stream.java | 1 +
.../coyote/http2/WindowAllocationManager.java | 38 ++++++++++++----------
.../apache/coyote/http2/TestHttp2Section_5_1.java | 4 +--
webapps/docs/changelog.xml | 5 +++
4 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 11e662b..00220dc 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -729,6 +729,7 @@ class Stream extends AbstractStream implements
HeaderEmitter {
se.getError()));
}
state.sendReset();
+ cancelAllocationRequests();
handler.sendStreamReset(se);
} catch (IOException ioe) {
ConnectionException ce = new ConnectionException(
diff --git a/java/org/apache/coyote/http2/WindowAllocationManager.java
b/java/org/apache/coyote/http2/WindowAllocationManager.java
index f286430..973ce59 100644
--- a/java/org/apache/coyote/http2/WindowAllocationManager.java
+++ b/java/org/apache/coyote/http2/WindowAllocationManager.java
@@ -17,6 +17,7 @@
package org.apache.coyote.http2;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.Response;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.res.StringManager;
@@ -184,24 +185,27 @@ class WindowAllocationManager {
// to stream.notify(). Additional notify() calls may trigger
// unexpected timeouts.
waitingFor = NONE;
- if (stream.getCoyoteResponse().getWriteListener() == null) {
- // Blocking, so use notify to release StreamOutputBuffer
- if (log.isDebugEnabled()) {
-
log.debug(sm.getString("windowAllocationManager.notified",
- stream.getConnectionId(),
stream.getIdentifier()));
+ Response response = stream.getCoyoteResponse();
+ if (response != null) {
+ if (response.getWriteListener() == null) {
+ // Blocking, so use notify to release
StreamOutputBuffer
+ if (log.isDebugEnabled()) {
+
log.debug(sm.getString("windowAllocationManager.notified",
+ stream.getConnectionId(),
stream.getIdentifier()));
+ }
+ stream.notify();
+ } else {
+ // Non-blocking so dispatch
+ if (log.isDebugEnabled()) {
+
log.debug(sm.getString("windowAllocationManager.dispatched",
+ stream.getConnectionId(),
stream.getIdentifier()));
+ }
+ response.action(ActionCode.DISPATCH_WRITE, null);
+ // Need to explicitly execute dispatches on the
StreamProcessor
+ // as this thread is being processed by an
UpgradeProcessor
+ // which won't see this dispatch
+ response.action(ActionCode.DISPATCH_EXECUTE, null);
}
- stream.notify();
- } else {
- // Non-blocking so dispatch
- if (log.isDebugEnabled()) {
-
log.debug(sm.getString("windowAllocationManager.dispatched",
- stream.getConnectionId(),
stream.getIdentifier()));
- }
-
stream.getCoyoteResponse().action(ActionCode.DISPATCH_WRITE, null);
- // Need to explicitly execute dispatches on the
StreamProcessor
- // as this thread is being processed by an UpgradeProcessor
- // which won't see this dispatch
-
stream.getCoyoteResponse().action(ActionCode.DISPATCH_EXECUTE, null);
}
}
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
index 3538516..00c0495 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
@@ -374,8 +374,7 @@ public class TestHttp2Section_5_1 extends Http2TestBase {
parser.readFrame(true);
Assert.assertTrue(output.getTrace(),
- output.getTrace().contains("5-RST-[" +
- Http2Error.REFUSED_STREAM.getCode() + "]"));
+ output.getTrace().contains("5-RST-[" +
Http2Error.REFUSED_STREAM.getCode() + "]"));
output.clearTrace();
// Connection window is zero.
@@ -386,5 +385,6 @@ public class TestHttp2Section_5_1 extends Http2TestBase {
sendWindowUpdate(3, (1 << 31) - 1);
parser.readFrame(true);
+ Assert.assertEquals("3-RST-[" +
Http2Error.FLOW_CONTROL_ERROR.getCode() + "]\n", output.getTrace());
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index de49ffc..d72196e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,11 @@
closed error if errors occur after a stream has been reset by the
client. (markt)
</fix>
+ <fix>
+ Improve handling of HTTP/2 stream level flow control errors and notidy
+ the stream immediately if it is waiting for an allocation when the flow
+ control error occurs. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
