Skip to content

Commit

Permalink
Ensure we produce and send window update frames all the times when da…
Browse files Browse the repository at this point in the history
…ta (java-native-access#471)

is read by a stream.

Motivation:

Due a bug we sometimes missed to produce / send window update frames
when QuicStreamChannel.read() was called by the user after the data was
already feed into the parent QuicChannel. This could lead to deadlocks
as the remote peer never received window update frames and so never
tried to send more data.

Modifications:

- Correctly produce and send window update frames in all cases
- Re-enable tests that sometimes failed due the bug

Result:

No more deadlocks possible
  • Loading branch information
normanmaurer authored Jan 24, 2023
1 parent 2a5c77f commit 7535229
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,13 @@ public void beginRead() {
readPending = true;
if (readable) {
((QuicStreamChannelUnsafe) unsafe()).recv();

// As the stream was readable, and we called recv() ourselves we also need to call
// connectionSendAndFlush(). This is needed as recv() might consume data and so a window update
// frame might be produced. If we miss to call connectionSendAndFlush() we might never send the update
// to the remote peer and so the remote peer might never attempt to send more data.
// See also https://docs.rs/quiche/latest/quiche/struct.Connection.html#method.send.
parent().connectionSendAndFlush();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import io.netty.util.concurrent.ImmediateEventExecutor;
import io.netty.util.concurrent.Promise;
import io.netty.util.concurrent.PromiseNotifier;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -41,14 +40,12 @@

public class QuicWritableTest extends AbstractQuicTest {

@Disabled("Flaky, needs investigation")
@ParameterizedTest
@MethodSource("newSslTaskExecutors")
public void testCorrectlyHandleWritabilityReadRequestedInReadComplete(Executor executor) throws Throwable {
testCorrectlyHandleWritability(executor, true);
}

@Disabled("Flaky, needs investigation")
@ParameterizedTest
@MethodSource("newSslTaskExecutors")
public void testCorrectlyHandleWritabilityReadRequestedInRead(Executor executor) throws Throwable {
Expand Down

0 comments on commit 7535229

Please sign in to comment.