Skip to content

Commit

Permalink
Fix 0RTT test when offloading tasks (java-native-access#548)
Browse files Browse the repository at this point in the history
Motivation:

We need to ensure the session was added to the cache before creating the
second connection as otherwise the test will timeout due not re-using
sessions.

Modifications:

- Add extra method to QuicClientSessionCache that we can use to check if
a session was added
- Spin until the session was added to the cache and only after that try
to connect the second time

Result:

No more timeouts due a race when testing for 0RTT. Fixes
netty/netty-incubator-codec-quic#544
  • Loading branch information
normanmaurer authored Jun 30, 2023
1 parent 52032f9 commit 9dfaba6
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,17 @@ void saveSession(String host, int port, long creationTime, long timeout, byte[]
}
}

// Only used for testing.
boolean hasSession(String host, int port) {
HostPort hostPort = keyFor(host, port);
if (hostPort != null) {
synchronized (sessions) {
return sessions.containsKey(hostPort);
}
}
return false;
}

byte[] getSession(String host, int port) {
HostPort hostPort = keyFor(host, port);
if (hostPort != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,15 +575,6 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
@ParameterizedTest
@MethodSource("newSslTaskExecutors")
public void testConnectWith0RTT(Executor executor) throws Throwable {
if (executor != ImmediateExecutor.INSTANCE) {
// Disable 0RTT test when offloading for now as it sometimes timeout. This is just a workaround and will
// need a proper fix.
// See https://github.com/netty/netty-incubator-codec-quic/issues/544
//
// TODO: remove once fixed.
shutdown(executor);
return;
}
final CountDownLatch readLatch = new CountDownLatch(1);
Channel server = QuicTestUtils.newServer(QuicTestUtils.newQuicServerBuilder(executor,
QuicSslContextBuilder.forServer(
Expand Down Expand Up @@ -647,6 +638,16 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
.connect()
.get();

QuicClientSessionCache cache = ((QuicheQuicSslContext) sslContext).getSessionCache();

// Let's spin until the session shows up in the cache. This is needed as this might happen a bit after
// the connection is already established.
// See https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_CTX_sess_set_new_cb
while (!cache.hasSession("localhost", 9999)) {
// Check again in 100ms.
Thread.sleep(100);
}

quicChannel.close().sync();

if (errorRef.get() != null) {
Expand Down

0 comments on commit 9dfaba6

Please sign in to comment.