andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Ensure channel handler close() is not skipped in !hasDisconnect case (#9098)

Browse Source 
Motivation

The optimization in #8988 didn't correctly handle the specific case
where the channel hasDisconnect == false, and a
ChannelOutboundHandlerAdapter subclass overrides only the close(ctx,
promise) method without also overriding the disconnect(ctx, promise)
method.

Modifications

Adjust AbstractChannelHandler.disconnect(...) method to divert to
close(...) in !hasDisconnect case before computing target context for
the event.

Result

Fixes #9092
This commit is contained in:
Nick Hill 2019-04-28 01:41:51 -07:00 committed by Norman Maurer
parent 2d33d1493e
commit 00a9a25f29
2 changed files with 18 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java
View File

@ -555,6 +555,11 @@ abstract class AbstractChannelHandlerContext implements ChannelHandlerContext, R
@Override
public ChannelFuture disconnect(final ChannelPromise promise) {
if (!channel().metadata().hasDisconnect()) {
// Translate disconnect to close if the channel has no notion of disconnect-reconnect.
// So far, UDP/IP is the only transport that has such behavior.
return close(promise);
}
if (isNotValidPromise(promise, false)) {
// cancelled
return promise;
@ -563,22 +568,12 @@ abstract class AbstractChannelHandlerContext implements ChannelHandlerContext, R
final AbstractChannelHandlerContext next = findContextOutbound(MASK_DISCONNECT);
EventExecutor executor = next.executor();
if (executor.inEventLoop()) {
// Translate disconnect to close if the channel has no notion of disconnect-reconnect.
// So far, UDP/IP is the only transport that has such behavior.
if (!channel().metadata().hasDisconnect()) {
next.invokeClose(promise);
} else {
next.invokeDisconnect(promise);
}
next.invokeDisconnect(promise);
} else {
safeExecute(executor, new Runnable() {
@Override
public void run() {
if (!channel().metadata().hasDisconnect()) {
next.invokeClose(promise);
} else {
next.invokeDisconnect(promise);
}
next.invokeDisconnect(promise);
}
}, promise, null);
}

11
transport/src/test/java/io/netty/channel/embedded/EmbeddedChannelTest.java
View File

@ -281,6 +281,17 @@ public class EmbeddedChannelTest {
assertNull(handler.pollEvent());
}
@Test
public void testHasNoDisconnectSkipDisconnect() throws InterruptedException {
EmbeddedChannel channel = new EmbeddedChannel(false, new ChannelOutboundHandlerAdapter() {
@Override
public void close(ChannelHandlerContext ctx, ChannelPromise promise) throws Exception {
promise.tryFailure(new Throwable());
}
});
assertFalse(channel.disconnect().isSuccess());
}
@Test
public void testFinishAndReleaseAll() {
ByteBuf in = Unpooled.buffer();