tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject [tomcat] branch 8.5.x updated: Expand HTTP/2 timeout handling to connection window exhaustion on write.
Date Fri, 03 May 2019 19:46:19 GMT
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


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 0bcd69c  Expand HTTP/2 timeout handling to connection window exhaustion on write.
0bcd69c is described below

commit 0bcd69c9dd8ae0ff424f2cd46de51583510b7f35
Author: Mark Thomas <markt@apache.org>
AuthorDate: Tue Apr 30 22:18:12 2019 +0100

    Expand HTTP/2 timeout handling to connection window exhaustion on write.
---
 .../apache/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 86f0e93..e5ae91f 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -805,7 +805,26 @@ public class Http2UpgradeHandler extends AbstractStream implements InternalHttpU
                 if (allocation == 0) {
                     if (block) {
                         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,
@@ -1023,11 +1042,20 @@ public class Http2UpgradeHandler extends AbstractStream implements
InternalHttpU
 
 
     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 1c6ed9d..d7b2006 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -280,17 +280,7 @@ public 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, getIdAsInt());
-                        // 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();
                     }
                 } catch (InterruptedException e) {
                     // Possible shutdown / rst or similar. Use an IOException to
@@ -313,6 +303,21 @@ public 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, getIdAsInt());
+        // 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
     @Deprecated
     protected synchronized void doNotifyAll() {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 72c1424..24a4b9a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -113,6 +113,10 @@
         Refactor Hostname validation to improve performance. Patch provided by
         Uwe Hees. (markt)
       </scode>
+      <fix>
+        Expand HTTP/2 timeout handling to include connection window exhaustion
+        on write. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">


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


Mime
View raw message