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

Fix #239: IdleStateHandler starts two timers

Browse Source 
1) Do not rely on ChannelPipeline.isAttached() to ensure initialize() is
called once.
2) Fix a race condition where initialize() can schedule timeouts after
destroy() is called.
This commit is contained in:
Trustin Lee 2012-03-22 15:48:08 +09:00
parent 5ecadf930e
commit bab2ebc32d
1 changed files with 41 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
src/main/java/org/jboss/netty/handler/timeout/IdleStateHandler.java
View File

@ -222,15 +222,7 @@ public class IdleStateHandler extends SimpleChannelUpstreamHandler
}
public void beforeAdd(ChannelHandlerContext ctx) throws Exception {
if (ctx.getPipeline().isAttached()) {
// channelOpen event has been fired already, which means
// this.channelOpen() will not be invoked.
// We have to initialize here instead.
initialize(ctx);
} else {
// channelOpen event has not been fired yet.
// this.channelOpen() will be invoked and initialization will occur there.
}
initialize(ctx);
}
public void afterAdd(ChannelHandlerContext ctx) throws Exception {
@ -245,16 +237,6 @@ public class IdleStateHandler extends SimpleChannelUpstreamHandler
// NOOP
}
@Override
public void channelOpen(ChannelHandlerContext ctx, ChannelStateEvent e)
throws Exception {
// This method will be invoked only if this handler was added
// before channelOpen event is fired. If a user adds this handler
// after the channelOpen event, initialize() will be called by beforeAdd().
initialize(ctx);
ctx.sendUpstream(e);
}
@Override
public void channelClosed(ChannelHandlerContext ctx, ChannelStateEvent e)
throws Exception {
@ -281,8 +263,13 @@ public class IdleStateHandler extends SimpleChannelUpstreamHandler
}
private void initialize(ChannelHandlerContext ctx) {
State state = new State();
ctx.setAttachment(state);
State state = state(ctx);
// Avoid the case where destroy() is called before scheduling timeouts.
// See: https://github.com/netty/netty/issues/143
if (state.destroyed) {
return;
}
state.lastReadTime = state.lastWriteTime = System.currentTimeMillis();
if (readerIdleTimeMillis > 0) {
@ -303,27 +290,39 @@ public class IdleStateHandler extends SimpleChannelUpstreamHandler
}
private void destroy(ChannelHandlerContext ctx) {
State state = (State) ctx.getAttachment();
State state;
// Check if the state was set before, it may not if the destroy method was called before the
// channelOpen(...) method.
//
// See #143
if (state != null) {
if (state.readerIdleTimeout != null) {
state.readerIdleTimeout.cancel();
state.readerIdleTimeout = null;
}
if (state.writerIdleTimeout != null) {
state.writerIdleTimeout.cancel();
state.writerIdleTimeout = null;
}
if (state.allIdleTimeout != null) {
state.allIdleTimeout.cancel();
state.allIdleTimeout = null;
}
synchronized (ctx) {
state = state(ctx);
state.destroyed = true;
}
if (state.readerIdleTimeout != null) {
state.readerIdleTimeout.cancel();
state.readerIdleTimeout = null;
}
if (state.writerIdleTimeout != null) {
state.writerIdleTimeout.cancel();
state.writerIdleTimeout = null;
}
if (state.allIdleTimeout != null) {
state.allIdleTimeout.cancel();
state.allIdleTimeout = null;
}
}
private State state(ChannelHandlerContext ctx) {
State state;
synchronized (ctx) {
// FIXME: It could have been better if there is setAttachmentIfAbsent().
state = (State) ctx.getAttachment();
if (state != null) {
return state;
}
state = new State();
ctx.setAttachment(state);
}
return state;
}
protected void channelIdle(
@ -448,5 +447,7 @@ public class IdleStateHandler extends SimpleChannelUpstreamHandler
volatile long lastWriteTime;
volatile Timeout allIdleTimeout;
volatile boolean destroyed;
}
}