Skip to content

Commit 0f859c3

Browse files
authored
okhttp: Move connection window update before stream termination logic (grpc#12990)
By RFC 9113, section 6.9, receivers must take frames into account for flow control even if they're errored. This change moves the stream error response logic after connection window updates
1 parent 292a361 commit 0f859c3

2 files changed

Lines changed: 16 additions & 11 deletions

File tree

okhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,19 @@ public void data(boolean inFinished, int streamId, BufferedSource in, int length
864864
// concerned with the window being exceeded at this point.
865865
in.require(length);
866866

867+
// connection window update
868+
// The connection window must be updated even if the stream is in an errored state.
869+
// See RFC 9113, section 6.9
870+
connectionUnacknowledgedBytesRead += paddedLength;
871+
if (connectionUnacknowledgedBytesRead
872+
>= config.flowControlWindow * Utils.DEFAULT_WINDOW_UPDATE_RATIO) {
873+
synchronized (lock) {
874+
frameWriter.windowUpdate(0, connectionUnacknowledgedBytesRead);
875+
frameWriter.flush();
876+
}
877+
connectionUnacknowledgedBytesRead = 0;
878+
}
879+
867880
synchronized (lock) {
868881
StreamState stream = streams.get(streamId);
869882
if (stream == null) {
@@ -887,17 +900,6 @@ public void data(boolean inFinished, int streamId, BufferedSource in, int length
887900
buf.write(in.getBuffer(), length);
888901
stream.inboundDataReceived(buf, length, paddedLength - length, inFinished);
889902
}
890-
891-
// connection window update
892-
connectionUnacknowledgedBytesRead += paddedLength;
893-
if (connectionUnacknowledgedBytesRead
894-
>= config.flowControlWindow * Utils.DEFAULT_WINDOW_UPDATE_RATIO) {
895-
synchronized (lock) {
896-
frameWriter.windowUpdate(0, connectionUnacknowledgedBytesRead);
897-
frameWriter.flush();
898-
}
899-
connectionUnacknowledgedBytesRead = 0;
900-
}
901903
}
902904

903905
@Override

okhttp/src/test/java/io/grpc/okhttp/OkHttpServerTransportTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,9 @@ public void windowUpdate() throws Exception {
10671067
writeDataDirectly(clientWriterSink, FLAG_PADDED | FLAG_END_STREAM, 1, message, 100);
10681068
clientFrameWriter.flush();
10691069
assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue();
1070+
// Receive window update for the padded data size before stream reset
1071+
verify(clientFramesRead).windowUpdate(0, expectedConsumed + 100);
1072+
assertThat(clientFrameReader.nextFrame(clientFramesRead)).isTrue();
10701073
verify(clientFramesRead).rstStream(eq(1), eq(ErrorCode.FLOW_CONTROL_ERROR));
10711074
clientFrameWriter.rstStream(3, ErrorCode.CANCEL);
10721075
pingPong();

0 commit comments

Comments
 (0)