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

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 3c11af965a
commit ddfe888173
2 changed files with 19 additions and 16 deletions

View File

@ -455,6 +455,12 @@ final class DefaultChannelHandlerContext implements ChannelHandlerContext, Resou
@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;
@ -462,23 +468,9 @@ final class DefaultChannelHandlerContext implements ChannelHandlerContext, Resou
EventExecutor executor = 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()) {
findAndInvokeClose(promise);
} else {
findAndInvokeDisconnect(promise);
}
findAndInvokeDisconnect(promise);
} else {
safeExecute(executor, () -> {
// 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()) {
findAndInvokeClose(promise);
} else {
findAndInvokeDisconnect(promise);
}
}, promise, null);
safeExecute(executor, () -> findAndInvokeDisconnect(promise), promise, null);
}
return promise;
}

View File

@ -236,6 +236,17 @@ public class EmbeddedChannelTest {
assertNull(handler.pollEvent());
}
@Test
public void testHasNoDisconnectSkipDisconnect() throws InterruptedException {
EmbeddedChannel channel = new EmbeddedChannel(false, new ChannelHandler() {
@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();