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 0bebe66  Additional h2 concurrency / timeout fix
0bebe66 is described below

commit 0bebe66d921072a3423cece63f3c2f8787d0342c
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu May 30 13:39:05 2019 +0100

    Additional h2 concurrency / timeout fix
---
 .../apache/coyote/http2/Http2UpgradeHandler.java   | 37 ++++++++++++++++++----
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 880d365..4ecb642 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -133,6 +133,24 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
     private final PingManager pingManager = getPingManager();
     private volatile int newStreamsSinceLastPrune = 0;
     // Tracking for when the connection is blocked (windowSize < 1)
+    // The int array must have 3 elements. There are:
+    //   [0] - The number of bytes the Stream requires from the connection
+    //         window. This excludes any allocation that has already been made.
+    //   [1] - The number of bytes that has been allocated from the connection
+    //         window. This excludes any bytes that have been written since the
+    //         allocation was made.
+    //   [2] - 1 if the stream thread has been notified that an allocation has
+    //         been made but has not yet consumed that allocation. 0 in all
+    //         other cases. The purpose of this is to avoid the incorrect
+    //         triggering of a timeout for the following sequence of events:
+    //         window update 1
+    //         allocation 1
+    //         notify 1
+    //         window update 2
+    //         allocation 2
+    //         act on notify 1 (using allocation 1 and 2)
+    //         notify 2
+    //         act on notify 2 (timeout due to no allocation)
     private final Map<AbstractStream,int[]> backLogStreams = new 
ConcurrentHashMap<>();
     private long backLogSize = 0;
 
@@ -765,12 +783,12 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
                         // Has this stream been granted an allocation
                         int[] value = backLogStreams.get(stream);
                         if (value == null) {
-                            value = new int[] { reservation, 0 };
+                            value = new int[] { reservation, 0, 0 };
                             backLogStreams.put(stream, value);
                             backLogSize += reservation;
                             // Add the parents as well
                             AbstractStream parent = stream.getParentStream();
-                            while (parent != null && 
backLogStreams.putIfAbsent(parent, new int[2]) == null) {
+                            while (parent != null && 
backLogStreams.putIfAbsent(parent, new int[3]) == null) {
                                 parent = parent.getParentStream();
                             }
                         } else {
@@ -783,11 +801,13 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
                                     // backlog.
                                     backLogStreams.remove(stream);
                                 } else {
-                                    // This allocation has been used. Reset the
-                                    // allocation to zero. Leave the stream on
-                                    // the backlog as it still has more bytes 
to
-                                    // write.
+                                    // This allocation has been used. Leave the
+                                    // stream on the backlog as it still has
+                                    // more bytes to write.
+                                    // Reset the allocation to zero.
                                     value[1] = 0;
+                                    // Clear the notify in progress marker
+                                    value[2] = 0;
                                 }
                             }
                         }
@@ -931,7 +951,10 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
                 int allocation = entry.getValue()[1];
                 if (allocation > 0) {
                     backLogSize -= allocation;
-                    result.add(entry.getKey());
+                    if (entry.getValue()[2] == 0) {
+                        result.add(entry.getKey());
+                        entry.getValue()[2] = 1;
+                    }
                 }
             }
         }


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

Reply via email to