You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/09/13 15:38:21 UTC
[tomcat] branch master updated: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=63682 HTTP/2 hang
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
The following commit(s) were added to refs/heads/master by this push:
new d3d1dc6 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63682 HTTP/2 hang
d3d1dc6 is described below
commit d3d1dc644ba83ca24c13879cc7d2ca1a8df298bf
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Sep 13 16:37:55 2019 +0100
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63682 HTTP/2 hang
Fix a potential hang when using the asynchronous Servlet API to write
the response body and the stream and/or connection window reaches 0
bytes in size.
---
java/org/apache/coyote/http2/Stream.java | 11 ++--
.../coyote/http2/WindowAllocationManager.java | 59 ++++++++++++++--------
webapps/docs/changelog.xml | 5 ++
3 files changed, 51 insertions(+), 24 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 3de2b9f..65d636d 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -958,10 +958,15 @@ class Stream extends AbstractStream implements HeaderEmitter {
}
final synchronized boolean isReady() {
- if (getWindowSize() > 0 && handler.getWindowSize() > 0 && !dataLeft) {
- return true;
- } else {
+ // Bug 63682
+ // Only want to return false if the window size is zero AND we are
+ // already waiting for an allocation.
+ if (getWindowSize() > 0 && allocationManager.isWaitingForStream() ||
+ handler.getWindowSize() > 0 && allocationManager.isWaitingForConnection() ||
+ dataLeft) {
return false;
+ } else {
+ return true;
}
}
diff --git a/java/org/apache/coyote/http2/WindowAllocationManager.java b/java/org/apache/coyote/http2/WindowAllocationManager.java
index da7aebe..ee865b0 100644
--- a/java/org/apache/coyote/http2/WindowAllocationManager.java
+++ b/java/org/apache/coyote/http2/WindowAllocationManager.java
@@ -65,40 +65,40 @@ class WindowAllocationManager {
}
void waitForStream(long timeout) throws InterruptedException {
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("windowAllocationManager.waitFor.stream",
+ //if (log.isDebugEnabled()) {
+ log.info(sm.getString("windowAllocationManager.waitFor.stream",
stream.getConnectionId(), stream.getIdentifier(), Long.toString(timeout)));
- }
+ //}
waitFor(STREAM, timeout);
}
void waitForConnection(long timeout) throws InterruptedException {
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("windowAllocationManager.waitFor.connection",
+ //if (log.isDebugEnabled()) {
+ log.info(sm.getString("windowAllocationManager.waitFor.connection",
stream.getConnectionId(), stream.getIdentifier(), Long.toString(timeout)));
- }
+ //}
waitFor(CONNECTION, timeout);
}
void waitForStreamNonBlocking() {
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("windowAllocationManager.waitForNonBlocking.stream",
+ //if (log.isDebugEnabled()) {
+ log.info(sm.getString("windowAllocationManager.waitForNonBlocking.stream",
stream.getConnectionId(), stream.getIdentifier()));
- }
+ //}
waitForNonBlocking(STREAM);
}
void waitForConnectionNonBlocking() {
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("windowAllocationManager.waitForNonBlocking.connection",
+ //if (log.isDebugEnabled()) {
+ log.info(sm.getString("windowAllocationManager.waitForNonBlocking.connection",
stream.getConnectionId(), stream.getIdentifier()));
- }
+ //}
waitForNonBlocking(CONNECTION);
}
@@ -119,6 +119,23 @@ class WindowAllocationManager {
}
+ boolean isWaitingForStream() {
+ return isWaitingFor(STREAM);
+ }
+
+
+ boolean isWaitingForConnection() {
+ return isWaitingFor(CONNECTION);
+ }
+
+
+ private boolean isWaitingFor(int waitTarget) {
+ synchronized (stream) {
+ return (waitingFor & waitTarget) > 0;
+ }
+ }
+
+
private void waitFor(int waitTarget, long timeout) throws InterruptedException {
synchronized (stream) {
if (waitingFor != NONE) {
@@ -154,10 +171,10 @@ class WindowAllocationManager {
private void notify(int notifyTarget) {
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("windowAllocationManager.notify", stream.getConnectionId(),
+ //if (log.isDebugEnabled()) {
+ log.info(sm.getString("windowAllocationManager.notify", stream.getConnectionId(),
stream.getIdentifier(), Integer.toString(waitingFor), Integer.toString(notifyTarget)));
- }
+ //}
synchronized (stream) {
if ((notifyTarget & waitingFor) > NONE) {
@@ -169,17 +186,17 @@ class WindowAllocationManager {
waitingFor = NONE;
if (stream.getCoyoteResponse().getWriteListener() == null) {
// Blocking, so use notify to release StreamOutputBuffer
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("windowAllocationManager.notified",
+ //if (log.isDebugEnabled()) {
+ log.info(sm.getString("windowAllocationManager.notified",
stream.getConnectionId(), stream.getIdentifier()));
- }
+ //}
stream.notify();
} else {
// Non-blocking so dispatch
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("windowAllocationManager.dispatched",
+ //if (log.isDebugEnabled()) {
+ log.info(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
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index bec625b..c223a4c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -77,6 +77,11 @@
<subsection name="Coyote">
<changelog>
<fix>
+ <bug>63682</bug>: Fix a potential hang when using the asynchronous
+ Servlet API to write the response body and the stream and/or connection
+ window reaches 0 bytes in size. (markt)
+ </fix>
+ <fix>
<bug>63690</bug>: Use the average of the current and previous sizes when
calculating overhead for HTTP/2 <code>DATA</code> and
<code>WINDOW_UPDATE</code> frames to avoid false positives as a result
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org