Skip to content

Commit

Permalink
Revert "Always increment Stream Id on createStream (#13485)" to fix b…
Browse files Browse the repository at this point in the history
…ug which caused sending multiple RST frames for the same id

Motivation:

This reverts commit 4e1a0c3 as it seems to cause a race in some situations. More investigation needs to be done but for now let's revert it.

Modification:

Revert commit 4e1a0c3

Result:

No more race which cause multiple RST frames to be send for the same stream id.
  • Loading branch information
normanmaurer committed Jul 27, 2023
1 parent 0e11934 commit 1129cff
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -738,16 +738,13 @@ public boolean canOpenStream() {
public DefaultStream createStream(int streamId, boolean halfClosed) throws Http2Exception {
State state = activeState(streamId, IDLE, isLocal(), halfClosed);

try {
checkNewStreamAllowed(streamId, state);
} finally {
// Always increment Expected stream ID
incrementExpectedStreamId(streamId);
}
checkNewStreamAllowed(streamId, state);

// Create and initialize the stream.
DefaultStream stream = new DefaultStream(streamId, state);

incrementExpectedStreamId(streamId);

addStream(stream);

stream.activate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,21 +347,6 @@ public void clientLocalIncrementAndGetStreamShouldRespectOverflow() throws Http2
incrementAndGetStreamShouldRespectOverflow(client.local(), MAX_VALUE);
}

@Test
public void incrementAndGetStreamShouldAlwaysIncrement() {
final int streamId = client.local().lastStreamKnownByPeer() + 1;

assertThrows(Http2Exception.class, new Executable() {
@Override
public void execute() throws Throwable {
client.local().createStream(streamId, true);
}
});

int nextStreamID = client.local().incrementAndGetNextStreamId();
assertEquals(streamId, nextStreamID - 3);
}

@Test
public void clientLocalCreateStreamExhaustedSpace() throws Http2Exception {
client.local().createStream(MAX_VALUE, true);
Expand Down

0 comments on commit 1129cff

Please sign in to comment.