This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new 50015e6 Fix a possible connection stall during concurrent stream write 50015e6 is described below commit 50015e6946ca2d4bd3384aa1106bfc5d0fff6c19 Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Jul 1 19:24:06 2021 +0100 Fix a possible connection stall during concurrent stream write The stall could occur in the following circumstances: - all current streams were in the backlog and received partial allocations - not all streams have acted on their allocation (i.e. the connection window size is currently >0) - one or more window updates arrive for the connection before the connection window size reaches zero - no further window updates are received after the window size reaches zero This could be recreated with a simple HTML page that loaded three large (1MB) images when called with "nghttp -vnsay https://localhost:8443/test/index.html" --- java/org/apache/coyote/http2/Http2UpgradeHandler.java | 14 +++++++++++++- webapps/docs/changelog.xml | 7 +++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index dbdac14..add82e7 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -980,7 +980,17 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH synchronized (this) { long windowSize = getWindowSize(); if (windowSize < 1 && windowSize + increment > 0) { + // Connection window is completed exhausted. Assume there will + // be streams to notify. The overhead is minimal if there are + // none. streamsToNotify = releaseBackLog((int) (windowSize +increment)); + } else if (backLogSize > 0) { + // While windowSize is greater than zero, all of it has already + // been allocated to streams in the backlog (or just about to + // exit the backlog). If any of windowSize was unallocated or + // 'spare', backLogSize would be zero. Therefore, apply this + // addition allocation to the backlog. + streamsToNotify = releaseBackLog(increment); } super.incrementWindowSize(increment); } @@ -1070,7 +1080,9 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH // Loop until we run out of allocation or recipients while (leftToAllocate > 0) { if (recipients.size() == 0) { - backLogStreams.remove(stream); + if (tracker.getUnusedAllocation() == 0) { + backLogStreams.remove(stream); + } return leftToAllocate; } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 7538ad1..709b40d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -126,6 +126,13 @@ <add> Add debug logging for writing an HTTP/2 response via sendfile. (markt) </add> + <fix> + Correct bugs in the HTTP/2 connection flow control management that meant + it was possible for a connection to stall waiting for a connection flow + control window update that had already arrived. Any streams on that + connection that were trying to write when this happened would time out. + (markt) + </fix> </changelog> </subsection> <subsection name="Web applications"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org