Not add ChannelHandler to ChannelPipeline once the pipeline was destroyed.
Motivation: ChannelPipeline will happily add a handler to a closed Channel's pipeline and will call handlerAdded(...) but will not call handlerRemoved(...). Modifications: Check if pipeline was destroyed and if so not add the handler at all but propergate an exception. Result: Fixes [#6768]
This commit is contained in:
parent
62d9b5b22b
commit
4cc98134da
@ -62,6 +62,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
private Map<EventExecutorGroup, EventExecutor> childExecutors;
|
||||
private MessageSizeEstimator.Handle estimatorHandle;
|
||||
private boolean firstRegistration = true;
|
||||
private boolean destroyed;
|
||||
|
||||
/**
|
||||
* This is the head of a linked list that is processed by {@link #callHandlerAddedForAllHandlers()} and so process
|
||||
@ -127,6 +128,12 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
return channel;
|
||||
}
|
||||
|
||||
private void checkDestroyed(ChannelHandler handler) {
|
||||
if (destroyed) {
|
||||
throw new ChannelPipelineException("Channel already closed, can not add handler " + handler);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public final ChannelPipeline addFirst(String name, ChannelHandler handler) {
|
||||
return addFirst(null, name, handler);
|
||||
@ -136,6 +143,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
public final ChannelPipeline addFirst(EventExecutorGroup group, String name, ChannelHandler handler) {
|
||||
final AbstractChannelHandlerContext newCtx;
|
||||
synchronized (this) {
|
||||
checkDestroyed(handler);
|
||||
checkMultiplicity(handler);
|
||||
name = filterName(name, handler);
|
||||
|
||||
@ -185,6 +193,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
public final ChannelPipeline addLast(EventExecutorGroup group, String name, ChannelHandler handler) {
|
||||
final AbstractChannelHandlerContext newCtx;
|
||||
synchronized (this) {
|
||||
checkDestroyed(handler);
|
||||
checkMultiplicity(handler);
|
||||
|
||||
newCtx = newContext(group, filterName(name, handler), handler);
|
||||
@ -235,6 +244,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
final AbstractChannelHandlerContext newCtx;
|
||||
final AbstractChannelHandlerContext ctx;
|
||||
synchronized (this) {
|
||||
checkDestroyed(handler);
|
||||
checkMultiplicity(handler);
|
||||
name = filterName(name, handler);
|
||||
ctx = getContextOrDie(baseName);
|
||||
@ -295,6 +305,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
final AbstractChannelHandlerContext ctx;
|
||||
|
||||
synchronized (this) {
|
||||
checkDestroyed(handler);
|
||||
checkMultiplicity(handler);
|
||||
name = filterName(name, handler);
|
||||
ctx = getContextOrDie(baseName);
|
||||
@ -505,6 +516,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
|
||||
final AbstractChannelHandlerContext newCtx;
|
||||
synchronized (this) {
|
||||
checkDestroyed(newHandler);
|
||||
checkMultiplicity(newHandler);
|
||||
boolean sameName = ctx.name().equals(newName);
|
||||
if (!sameName) {
|
||||
@ -823,6 +835,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
|
||||
* See: https://github.com/netty/netty/issues/3156
|
||||
*/
|
||||
private synchronized void destroy() {
|
||||
destroyed = true;
|
||||
destroyUp(head.next, false);
|
||||
}
|
||||
|
||||
|
@ -1069,6 +1069,38 @@ public class DefaultChannelPipelineTest {
|
||||
group.shutdownGracefully(0, 0, TimeUnit.SECONDS);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHandlerAfterClose() throws Exception {
|
||||
EmbeddedChannel channel = new EmbeddedChannel();
|
||||
channel.close().syncUninterruptibly();
|
||||
|
||||
assertFalse(channel.isActive());
|
||||
|
||||
final AtomicBoolean addedHandler = new AtomicBoolean();
|
||||
final AtomicBoolean removedHandler = new AtomicBoolean();
|
||||
|
||||
ChannelPipeline pipeline = channel.pipeline();
|
||||
try {
|
||||
pipeline.addLast(new ChannelHandlerAdapter() {
|
||||
@Override
|
||||
public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
|
||||
addedHandler.set(true);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
|
||||
removedHandler.set(true);
|
||||
}
|
||||
});
|
||||
fail();
|
||||
} catch (ChannelPipelineException e) {
|
||||
// expected
|
||||
}
|
||||
|
||||
assertFalse(addedHandler.get());
|
||||
assertFalse(removedHandler.get());
|
||||
}
|
||||
|
||||
@Test(timeout = 3000)
|
||||
public void testVoidPromiseNotify() throws Throwable {
|
||||
ChannelPipeline pipeline1 = new LocalChannel().pipeline();
|
||||
|
Loading…
Reference in New Issue
Block a user