Skip to content

Commit

Permalink
Http2MultiplexCodec.DefaultHttpStreamChannel.isOpen() / isActive() sh…
Browse files Browse the repository at this point in the history
…oule be false when fireChannelActive() is called

Motivation:

When part of a HTTP/2 StreamChannel the Http2StreamChannel.isOpen() / isActive() should report false within a call to a ChannelInboundHandlers channelInactive() method.

Modifications:

Fullfill promise before call fireChannelInactive()

Result:

Correctly update state / promise before notify handlers. Fixes [netty#7638]
  • Loading branch information
normanmaurer authored and Scottmitch committed Jan 27, 2018
1 parent bed74d8 commit 49d1db4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -886,12 +886,14 @@ public void close(ChannelPromise promise) {
}
}

// The promise should be notified before we call fireChannelInactive().
promise.setSuccess();
closePromise.setSuccess();

pipeline().fireChannelInactive();
if (isRegistered()) {
deregister(unsafe().voidPromise());
}
promise.setSuccess();
closePromise.setSuccess();
} finally {
pendingClosePromise = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import io.netty.util.AttributeKey;

import java.net.InetSocketAddress;
import java.util.concurrent.atomic.AtomicBoolean;

import org.junit.After;
import org.junit.Before;
Expand Down Expand Up @@ -420,6 +421,31 @@ public void writabilityAndFlowControl() {
assertEquals("true,false", inboundHandler.writabilityStates());
}

@Test
public void channelClosedWhenInactiveFired() {
LastInboundHandler inboundHandler = streamActiveAndWriteHeaders(inboundStream);
Http2StreamChannel childChannel = (Http2StreamChannel) inboundHandler.channel();

final AtomicBoolean channelOpen = new AtomicBoolean(false);
final AtomicBoolean channelActive = new AtomicBoolean(false);
assertTrue(childChannel.isOpen());
assertTrue(childChannel.isActive());

childChannel.pipeline().addLast(new ChannelInboundHandlerAdapter() {
@Override
public void channelInactive(ChannelHandlerContext ctx) throws Exception {
channelOpen.set(ctx.channel().isOpen());
channelActive.set(ctx.channel().isActive());

super.channelInactive(ctx);
}
});

childChannel.close().syncUninterruptibly();
assertFalse(channelOpen.get());
assertFalse(channelActive.get());
}

@Ignore("not supported anymore atm")
@Test
public void cancellingWritesBeforeFlush() {
Expand Down

0 comments on commit 49d1db4

Please sign in to comment.