Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Simone Bordet 2015-03-25 19:24:28 +01:00
parent b8623c125f
commit 999177ccc2
3 changed files with 36 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/FlowControlStrategyTest.java
View File

@ -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.

4
jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/BufferingFlowControlStrategy.java
View File

@ -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;

41
jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java
View File

@ -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.