Skip to content

Commit

Permalink
Remove QuicConnectEvent (java-native-access#618)
Browse files Browse the repository at this point in the history
Motivation:

How we used QuicConnectEvent to notify about connection migration was
not correct at all. Let's remove it for now as we will introduce
something better once cloudflare/quiche#1660 was
merged into quiche.

Modifications:

Remove QuicConnectEvent

Result:

Get rid of incorrect implementation to prepare for adding a proper one
  • Loading branch information
normanmaurer authored Nov 21, 2023
1 parent 088fc33 commit 2c5e104
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 97 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -1077,8 +1077,6 @@ private boolean connectionSendSegments(SegmentedDatagramPacketAllocator segmente
remote = QuicheSendInfo.getToAddress(sendInfo);
local = QuicheSendInfo.getFromAddress(sendInfo);

fireConnectionEventIfNeeded(oldRemote, remote);

if (size > 0) {
// We have something in the out list already, we need to send this now and so we set the
// segmentSize.
Expand Down Expand Up @@ -1136,16 +1134,6 @@ private boolean connectionSendSegments(SegmentedDatagramPacketAllocator segmente
}
}

private void fireConnectionEventIfNeeded(SocketAddress oldRemote, SocketAddress newRemote) {
// Check if the remote address changed and only in this case fire an event.
// This is required as even tho we check the SendInfo / RecvInfo without this extra check we might
// notify two times (one time in the recv and one time in the send code-path).
if (!oldRemote.equals(newRemote)) {
pipeline().fireUserEventTriggered(
new QuicConnectionEvent(oldRemote, newRemote));
}
}

private static int segmentSize(List<ByteBuf> bufferList) {
assert !bufferList.isEmpty();
int size = bufferList.size();
Expand Down Expand Up @@ -1189,11 +1177,8 @@ private boolean connectionSendSimple() {
}
if (connection.isSendInfoChanged()) {
// Change the cached address
InetSocketAddress oldRemote = remote;
remote = QuicheSendInfo.getToAddress(sendInfo);
local = QuicheSendInfo.getFromAddress(sendInfo);

fireConnectionEventIfNeeded(oldRemote, remote);
}
out.writerIndex(writerIndex + written);
boolean stop = writePacket(new DatagramPacket(out, remote), maxDatagramSize, len);
Expand Down Expand Up @@ -1416,7 +1401,6 @@ void connectionRecv(InetSocketAddress sender, InetSocketAddress recipient, ByteB
if (connection.isRecvInfoChanged()) {
// Update the cached address
remote = sender;
fireConnectionEventIfNeeded(oldRemote, sender);
}
local = recipient;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ private QuicheQuicChannel handleServer(ChannelHandlerContext ctx, InetSocketAddr

putChannel(channel);
ctx.channel().eventLoop().register(channel);
channel.pipeline().fireUserEventTriggered(new QuicConnectionEvent(null, sender));
return channel;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -520,22 +520,13 @@ public void testConnectWithoutTokenValidation(Executor executor) throws Throwabl
@Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
public void testConnectAndGetAddressesAfterClose(Executor executor) throws Throwable {
AtomicReference<QuicChannel> acceptedRef = new AtomicReference<>();
AtomicReference<QuicConnectionEvent> serverEventRef = new AtomicReference<>();
Channel server = QuicTestUtils.newServer(executor,
new ChannelInboundHandlerAdapter() {
@Override
public void channelActive(ChannelHandlerContext ctx) throws Exception {
acceptedRef.set((QuicChannel) ctx.channel());
super.channelActive(ctx);
}

@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
if (evt instanceof QuicConnectionEvent) {
serverEventRef.set((QuicConnectionEvent) evt);
}
super.userEventTriggered(ctx, evt);
}
},
new ChannelInboundHandlerAdapter());
InetSocketAddress address = (InetSocketAddress) server.localAddress();
Expand All @@ -555,9 +546,6 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
assertNotNull(quicChannel.localAddress());
assertNotNull(quicChannel.remoteAddress());

assertNull(serverEventRef.get().oldAddress());
assertEquals(channel.localAddress(), serverEventRef.get().newAddress());

QuicChannel accepted;
while ((accepted = acceptedRef.get()) == null) {
Thread.sleep(50);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,10 @@
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;

import static org.junit.jupiter.api.Assertions.fail;

class QuicChannelValidationHandler extends ChannelInboundHandlerAdapter {

private volatile Throwable cause;

@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
if (evt instanceof QuicConnectionEvent && ((QuicConnectionEvent) evt).oldAddress() != null) {
fail("QuicConnectionEvent indication migration should never happen atm");
}
super.userEventTriggered(ctx, evt);
}

@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
this.cause = cause;
Expand Down

0 comments on commit 2c5e104

Please sign in to comment.