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:56:43 UTC

[tomcat] branch 8.5.x updated (cc10604 -> 5ee1614)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from cc10604  eTags vary. Force HTTP/2 tests to use a constant value.
     new d57e1ca  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63682 HTTP/2 hang
     new 5ee1614  Clean-up. Remove unused code. Align with 9.0.x.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 java/org/apache/coyote/http2/Stream.java           | 22 ++++++++--------------
 .../coyote/http2/WindowAllocationManager.java      | 17 +++++++++++++++++
 webapps/docs/changelog.xml                         |  5 +++++
 3 files changed, 30 insertions(+), 14 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/02: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63682 HTTP/2 hang

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit d57e1ca2fc25c9234caed8d74ec741585ff59256
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                | 12 +++++++++---
 .../apache/coyote/http2/WindowAllocationManager.java    | 17 +++++++++++++++++
 webapps/docs/changelog.xml                              |  5 +++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index a25edfe..6b6eb93 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -937,12 +937,18 @@ public class Stream extends AbstractStream implements HeaderEmitter {
         }
 
         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) {
                 writeInterest = true;
                 return false;
+            } else {
+                return true;
             }
+
         }
 
         synchronized boolean isRegisteredForWrite() {
diff --git a/java/org/apache/coyote/http2/WindowAllocationManager.java b/java/org/apache/coyote/http2/WindowAllocationManager.java
index da7aebe..f286430 100644
--- a/java/org/apache/coyote/http2/WindowAllocationManager.java
+++ b/java/org/apache/coyote/http2/WindowAllocationManager.java
@@ -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) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a0e538e..75c640c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -62,6 +62,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


[tomcat] 02/02: Clean-up. Remove unused code. Align with 9.0.x.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 5ee1614cfe9ee6511d77c422957eadc486db1dc8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Sep 13 16:56:09 2019 +0100

    Clean-up. Remove unused code. Align with 9.0.x.
---
 java/org/apache/coyote/http2/Stream.java | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 6b6eb93..32de3fe 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -376,7 +376,7 @@ public class Stream extends AbstractStream implements HeaderEmitter {
             headerState = HEADER_STATE_REGULAR;
         }
 
-        switch(name) {
+        switch (name) {
         case ":method": {
             if (coyoteRequest.method().isNull()) {
                 coyoteRequest.method().setString(value);
@@ -777,7 +777,6 @@ public class Stream extends AbstractStream implements HeaderEmitter {
         private volatile boolean closed = false;
         private volatile StreamException reset = null;
         private volatile boolean endOfStreamSent = false;
-        private volatile boolean writeInterest = false;
 
         /* The write methods are synchronized to ensure that only one thread at
          * a time is able to access the buffer. Without this protection, a
@@ -943,21 +942,10 @@ public class Stream extends AbstractStream implements HeaderEmitter {
             if (getWindowSize() > 0 && allocationManager.isWaitingForStream() ||
                     handler.getWindowSize() > 0 && allocationManager.isWaitingForConnection() ||
                     dataLeft) {
-                writeInterest = true;
                 return false;
             } else {
                 return true;
             }
-
-        }
-
-        synchronized boolean isRegisteredForWrite() {
-            if (writeInterest) {
-                writeInterest = false;
-                return true;
-            } else {
-                return false;
-            }
         }
 
         @Override


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org