Expand HTTP/2 timeout handling to connection window exhaustion on write.

apache · May 3, 2019 · 7f748eb · 7f748eb
1 parent 151b447
commit 7f748eb
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 13 deletions.
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -801,7 +801,26 @@ int reserveWindowSize(Stream stream, int reservation, boolean block) throws IOEx
                 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,
@@ -1024,11 +1043,20 @@ private Stream createLocalStream(Request request) {
 
 
     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
@@ -283,17 +283,7 @@ final synchronized int reserveWindowSize(int reservation, boolean block)
                     }
                     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
@@ -316,6 +306,21 @@ final synchronized int reserveWindowSize(int reservation, boolean block)
     }
 
 
+    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
     public final void emitHeader(String name, String value) throws HpackException {
         if (log.isDebugEnabled()) {

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -171,6 +171,10 @@
         for possible use with an asynchronous workflow like CompletableFuture.
         (remm)
       </update>
+      <fix>
+        Expand HTTP/2 timeout handling to include connection window exhaustion
+        on write. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">