From 58cc6aec86590efca01e03b46225e8609ead623c Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Fri, 12 Aug 2011 14:03:48 +0900 Subject: [PATCH] NETTY-431 HashedWheelTimer's TimerTask may execute after call to Timeout.cancel() * Replaced a volatile boolean flag and system date access with an atomic integer flag. --- .../jboss/netty/util/HashedWheelTimer.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/jboss/netty/util/HashedWheelTimer.java b/src/main/java/org/jboss/netty/util/HashedWheelTimer.java index a41c9a5820..63264b2ce3 100644 --- a/src/main/java/org/jboss/netty/util/HashedWheelTimer.java +++ b/src/main/java/org/jboss/netty/util/HashedWheelTimer.java @@ -472,11 +472,15 @@ public class HashedWheelTimer implements Timer { private final class HashedWheelTimeout implements Timeout { + private static final int ST_INIT = 0; + private static final int ST_CANCELLED = 1; + private static final int ST_EXPIRED = 2; + private final TimerTask task; final long deadline; volatile int stopIndex; volatile long remainingRounds; - private volatile boolean cancelled; + private final AtomicInteger state = new AtomicInteger(ST_INIT); HashedWheelTimeout(TimerTask task, long deadline) { this.task = task; @@ -492,26 +496,24 @@ public class HashedWheelTimer implements Timer { } public void cancel() { - if (isExpired()) { + if (!state.compareAndSet(ST_INIT, ST_CANCELLED)) { + // TODO return false return; } - - cancelled = true; - - // Might be called more than once, but doesn't matter. + wheel[stopIndex].remove(this); } public boolean isCancelled() { - return cancelled; + return state.get() == ST_CANCELLED; } public boolean isExpired() { - return cancelled || System.currentTimeMillis() > deadline; + return state.get() != ST_INIT; } public void expire() { - if (cancelled) { + if (!state.compareAndSet(ST_INIT, ST_EXPIRED)) { return; }