Fixed race sending SETTINGS with INITIAL_WINDOW_SIZE.

Before, the sender was updating the window size after the SETTINGS
frame was written.
This was leading to a race where the receiver saw the updated window
size and sent DATA frames; these were received by the original sender
before it had the chance to update its local window size, causing an
error.
Now, the update of the window size happen just before writing the
SETTINGS frame to avoid this race.

3 files changed, 36 insertions, 11 deletions

diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java
index 30c36d5c55..239a7c0d3c 100644
--- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java
+++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java
@@ -648,7 +648,7 @@ public abstract class FlowControlStrategyTest
     }
 
     @Test
-    public void testServerTwoDataFramesWithStalledSession() throws Exception
+    public void testServerTwoDataFramesWithStalledStream() throws Exception
     {
         // Frames in queue = DATA1, DATA2.
         // Server writes part of DATA1, then stalls.
diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/BufferingFlowControlStrategy.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/BufferingFlowControlStrategy.java
index a17343b67e..95eb0d00e4 100644
--- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/BufferingFlowControlStrategy.java
+++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/BufferingFlowControlStrategy.java
@@ -95,7 +95,7 @@ public class BufferingFlowControlStrategy extends AbstractFlowControlStrategy
             level = sessionLevel.getAndSet(0);
             session.updateRecvWindow(level);
             if (LOG.isDebugEnabled())
-                LOG.debug("Data consumed, updated session recv window by {} for {}", level, session);
+                LOG.debug("Data consumed, updated session recv window by {}/{} for {}", level, maxLevel, session);
             windowFrame = new WindowUpdateFrame(0, level);
         }
         else
@@ -122,7 +122,7 @@ public class BufferingFlowControlStrategy extends AbstractFlowControlStrategy
                     level = streamLevel.getAndSet(0);
                     stream.updateRecvWindow(level);
                     if (LOG.isDebugEnabled())
-                        LOG.debug("Data consumed, updated stream recv window by {} for {}", level, stream);
+                        LOG.debug("Data consumed, updated stream recv window by {}/{} for {}", level, maxLevel, stream);
                     WindowUpdateFrame frame = new WindowUpdateFrame(stream.getId(), level);
                     if (windowFrame == null)
                         windowFrame = frame;
diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java
index c909d0f989..54608fdff6 100644
--- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java
+++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java
@@ -996,6 +996,7 @@ public abstract class HTTP2Session implements ISession, Parser.Listener
                 generator.control(lease, frame);
                 if (LOG.isDebugEnabled())
                     LOG.debug("Generated {}", frame);
+                prepare();
                 return null;
             }
             catch (Throwable x)
@@ -1006,6 +1007,38 @@ public abstract class HTTP2Session implements ISession, Parser.Listener
             }
         }
 
+        /**
+         * <p>Performs actions just before writing the frame to the network.</p>
+         * <p>Some frame, when sent over the network, causes the receiver
+         * to react and send back frames that may be processed by the original
+         * sender *before* {@link #succeeded()} is called.
+         * <p>If the action to perform updates some state, this update may
+         * not be seen by the received frames and cause errors.</p>
+         * <p>For example, suppose the action updates the stream window to a
+         * larger value; the sender sends the frame; the receiver is now entitled
+         * to send back larger data; when the data is received by the original
+         * sender, the action may have not been performed yet, causing the larger
+         * data to be rejected, when it should have been accepted.</p>
+         */
+        private void prepare()
+        {
+            switch (frame.getType())
+            {
+                case SETTINGS:
+                {
+                    SettingsFrame settingsFrame = (SettingsFrame)frame;
+                    Integer initialWindow = settingsFrame.getSettings().get(SettingsFrame.INITIAL_WINDOW_SIZE);
+                    if (initialWindow != null)
+                        flowControl.updateInitialStreamWindow(HTTP2Session.this, initialWindow, true);
+                    break;
+                }
+                default:
+                {
+                    break;
+                }
+            }
+        }
+
         @Override
         public void succeeded()
         {
@@ -1027,14 +1060,6 @@ public abstract class HTTP2Session implements ISession, Parser.Listener
                     }
                     break;
                 }
-                case SETTINGS:
-                {
-                    SettingsFrame settingsFrame = (SettingsFrame)frame;
-                    Integer initialWindow = settingsFrame.getSettings().get(SettingsFrame.INITIAL_WINDOW_SIZE);
-                    if (initialWindow != null)
-                        flowControl.updateInitialStreamWindow(HTTP2Session.this, initialWindow, true);
-                    break;
-                }
                 case PUSH_PROMISE:
                 {
                     // Pushed streams are implicitly remotely closed.