Make sure handlerAdded() is called before forwarding the buffer content of the removed handler

- Added a test case that reproduces the problem in ReplayingDecoderTest
- Call newHandler.handlerAdded() *before* oldHandler.handlerRemoved() to ensure newHandlerAdded() is called before forwarding the buffer content of the old handler in replace0().
This commit is contained in:
Trustin Lee 2013-04-24 19:25:43 +09:00
parent 99b999760a
commit 1b3d7f5325
2 changed files with 14 additions and 9 deletions

View File

@ -22,7 +22,6 @@ import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundByteHandlerAdapter; import io.netty.channel.ChannelInboundByteHandlerAdapter;
import io.netty.channel.embedded.EmbeddedByteChannel; import io.netty.channel.embedded.EmbeddedByteChannel;
import org.junit.Ignore;
import org.junit.Test; import org.junit.Test;
import static org.junit.Assert.*; import static org.junit.Assert.*;
@ -65,7 +64,6 @@ public class ReplayingDecoderTest {
} }
@Test @Test
@Ignore("needs a fix")
public void testReplacement() throws Exception { public void testReplacement() throws Exception {
EmbeddedByteChannel ch = new EmbeddedByteChannel(new BloatedLineDecoder()); EmbeddedByteChannel ch = new EmbeddedByteChannel(new BloatedLineDecoder());

View File

@ -442,25 +442,32 @@ final class DefaultChannelPipeline implements ChannelPipeline {
return ctx.handler(); return ctx.handler();
} }
private void replace0(DefaultChannelHandlerContext ctx, String newName, private void replace0(DefaultChannelHandlerContext oldCtx, String newName,
DefaultChannelHandlerContext newCtx) { DefaultChannelHandlerContext newCtx) {
checkMultiplicity(newCtx); checkMultiplicity(newCtx);
DefaultChannelHandlerContext prev = ctx.prev; DefaultChannelHandlerContext prev = oldCtx.prev;
DefaultChannelHandlerContext next = ctx.next; DefaultChannelHandlerContext next = oldCtx.next;
newCtx.prev = prev; newCtx.prev = prev;
newCtx.next = next; newCtx.next = next;
// Finish the replacement of oldCtx with newCtx in the linked list.
// Note that this doesn't mean events will be sent to the new handler immediately
// because we are currently at the event handler thread and no more than one handler methods can be invoked
// at the same time (we ensured that in replace().)
prev.next = newCtx; prev.next = newCtx;
next.prev = newCtx; next.prev = newCtx;
if (!ctx.name().equals(newName)) { if (!oldCtx.name().equals(newName)) {
name2ctx.remove(ctx.name()); name2ctx.remove(oldCtx.name());
} }
name2ctx.put(newName, newCtx); name2ctx.put(newName, newCtx);
// remove old and add new // Invoke newHandler.handlerAdded() first (i.e. before oldHandler.handlerRemoved() is invoked)
callHandlerRemoved(ctx, newCtx, newCtx); // because callHandlerRemoved() will trigger inboundBufferUpdated() or flush() on newHandler and those
// event handlers must be called after handlerAdded().
callHandlerAdded(newCtx); callHandlerAdded(newCtx);
callHandlerRemoved(oldCtx, newCtx, newCtx);
} }
private static void checkMultiplicity(ChannelHandlerContext ctx) { private static void checkMultiplicity(ChannelHandlerContext ctx) {