Repository: netty/netty
Issue: Enhancements to AbstractTrafficShapingHandler / TrafficCounter

1. do not sleep  101 ms due to issues with java and windows

see: http://www.javamex.com/tutorials/threads/sleep_issues.shtml

in getTimeToWait change 

return wait;

to:

return (wait/10)*10;
1. do not  "waste" a thread in TrafficCounter. use netty's Timer instead.

-- Ron


Issue comments:
PS: if desired I could provide these changes.

Hi @rzorzorzo 

On the first item, I was really unaware of this ! Thanks for spotting this !

On the second item, I could agree but the idea is to be able to change the timer... (traffic handler could changed during runtime due to special request).
How could we do this ? I suppose that using the Timer utilitye from Netty will bring us to assigning again and again a new thread at every check (which is probably worst than having one specific thread per counter making some sleep) ?
On Counter, we need to have such one thread per Handler (one unique for Global, but if needed, one per Channel).

I would be please if you can propose both, but perhaps in 2 separates proposal to be able to accept one or the other ?

@rzorzorzo 
Since I was on my way to check some other elements, I may have done the first point (see #346).

Sorry if you were on your way... ;-)
I will try to make the same on release 3.5 immediately.

However on second proposal, I let you make some proposals... ;-)

Frederic,

thnx for 1.

concerning 2:
TrafficCounter sleeps practically 100% of the time. During sleep, the thread is not returned to the pool. With 100k connections, therefore 100k channels, we will need with the current solution 100k threads. Not feasible.

Netty's HashedWheelTimer requires a single Thread. The same timer, and therefore thread can be shared by all TrafficCounters. The dynamic change of checkIntervall can be supported as with the current implementation.

Maybe the following code sections will help:

fields:

``` java
    private Timer timer;  // replace executor
    private TimerTask timerTask;  // created once in start()
    volatile private Timeout timeout = null; // used in stop() to cancel the timer
```

Constructor:

``` java
    public TrafficCounter(TrafficShapingHandler trafficShapingHandler,
            Timer timer, String name, long checkInterval) {       
        this.timer = timer;
        ...
       }
```

start method

``` java
 public void start() {
...
                      timerTask = new TimerTask()
                {

                    @Override
                    public void run(Timeout timeout) throws Exception
                    {
                        if (!monitorActive.get()) {
                                        return;
                                   }
                        trafficMonitoring.run();
                                      TrafficCounter.this.timeout = timer.newTimeout(timerTask, checkInterval.get(), TimeUnit.MILLISECONDS);                        
                    }

                };
                timeout = timer.newTimeout(timerTask, checkInterval.get(), TimeUnit.MILLISECONDS);
...
```

stop method

``` java
    public void stop() {
...
              if (timeout != null)
                timeout.cancel();
```

-- Ron

@rzorzorzo @fredericBregier  this makes a lot of sense

@rzorzorzo I will spend some time to your proposal, see if I understand first, then if I can propose a new version once I had tested it.
Thank you !!

@rzorzorzo and @normanmaurer 
I finally make the change as proposed. I tested it intensively. I find no issue (as for now of course).
I even optimize a bit more the traffic handler:
- there were one thread created to unlock traffic from time to time when needed => Timer
- in the original proposal, 2 objects were created: the TimerTask and the internal TrafficCOunter object => only the timer task now

I hope this is what you guess both of you !

See proposal in Issue #348

Cool.. I will review later today.

Thank you @normanmaurer !
And thank you too @rzorzorzo for these proposals!

I believe we can close this issue then.


Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/jboss/netty/handler/traffic/AbstractTrafficShapingHandler.java b/src/main/java/org/jboss/netty/handler/traffic/AbstractTrafficShapingHandler.java
index 2ec65af93b4..fe0fdd2ed30 100644
--- a/src/main/java/org/jboss/netty/handler/traffic/AbstractTrafficShapingHandler.java
+++ b/src/main/java/org/jboss/netty/handler/traffic/AbstractTrafficShapingHandler.java
@@ -15,7 +15,7 @@
  */
 package org.jboss.netty.handler.traffic;
 
-import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.jboss.netty.channel.Channel;
@@ -30,7 +30,9 @@
 import org.jboss.netty.util.DefaultObjectSizeEstimator;
 import org.jboss.netty.util.ExternalResourceReleasable;
 import org.jboss.netty.util.ObjectSizeEstimator;
-import org.jboss.netty.util.internal.ExecutorUtil;
+import org.jboss.netty.util.Timeout;
+import org.jboss.netty.util.Timer;
+import org.jboss.netty.util.TimerTask;
 
 /**
  * AbstractTrafficShapingHandler allows to limit the global bandwidth
@@ -41,6 +43,10 @@
  * the method doAccounting of this handler.<br>
  * <br>
  *
+ * An {@link ObjectSizeEstimator} can be passed at construction to specify what
+ * is the size of the object to be read or write accordingly to the type of
+ * object. If not specified, it will used the {@link DefaultObjectSizeEstimator} implementation.<br><br>
+ *
  * If you want for any particular reasons to stop the monitoring (accounting) or to change
  * the read/write limit or the check interval, several methods allow that for you:<br>
  * <ul>
@@ -79,10 +85,14 @@ public abstract class AbstractTrafficShapingHandler extends
     private ObjectSizeEstimator objectSizeEstimator;
 
     /**
-     * Executor to associated to any TrafficCounter
+     * Timer to associated to any TrafficCounter
      */
-    protected Executor executor;
-
+    protected Timer timer;
+    /**
+     * used in releaseExternalResources() to cancel the timer
+     */
+    volatile private Timeout timeout = null;
+    
     /**
      * Limit in B/s to apply to write
      */
@@ -105,15 +115,16 @@ public abstract class AbstractTrafficShapingHandler extends
      */
     final AtomicBoolean release = new AtomicBoolean(false);
 
-    private void init(ObjectSizeEstimator newObjectSizeEstimator,
-            Executor newExecutor, long newWriteLimit, long newReadLimit, long newCheckInterval) {
-        objectSizeEstimator = newObjectSizeEstimator;
-        executor = newExecutor;
-        writeLimit = newWriteLimit;
-        readLimit = newReadLimit;
-        checkInterval = newCheckInterval;
-        //logger.info("TSH: "+writeLimit+":"+readLimit+":"+checkInterval+":"+isPerChannel());
-    }
+     private void init(ObjectSizeEstimator newObjectSizeEstimator,
+             Timer newTimer, long newWriteLimit, long newReadLimit,
+             long newCheckInterval) {
+         objectSizeEstimator = newObjectSizeEstimator;
+         timer = newTimer;
+         writeLimit = newWriteLimit;
+         readLimit = newReadLimit;
+         checkInterval = newCheckInterval;
+         //logger.warn("TSH: "+writeLimit+":"+readLimit+":"+checkInterval);
+     }
 
     /**
      *
@@ -126,8 +137,8 @@ void setTrafficCounter(TrafficCounter newTrafficCounter) {
     /**
      * Constructor using default {@link ObjectSizeEstimator}
      *
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      * @param writeLimit
      *          0 or a limit in bytes/s
      * @param readLimit
@@ -136,10 +147,9 @@ void setTrafficCounter(TrafficCounter newTrafficCounter) {
      *          The delay between two computations of performances for
      *            channels or 0 if no stats are to be computed
      */
-    public AbstractTrafficShapingHandler(Executor executor, long writeLimit,
+    public AbstractTrafficShapingHandler(Timer timer, long writeLimit,
             long readLimit, long checkInterval) {
-        init(new DefaultObjectSizeEstimator(), executor, writeLimit, readLimit,
-                checkInterval);
+        init(new DefaultObjectSizeEstimator(), timer, writeLimit, readLimit, checkInterval);
     }
 
     /**
@@ -148,8 +158,8 @@ public AbstractTrafficShapingHandler(Executor executor, long writeLimit,
      * @param objectSizeEstimator
      *            the {@link ObjectSizeEstimator} that will be used to compute
      *            the size of the message
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      * @param writeLimit
      *          0 or a limit in bytes/s
      * @param readLimit
@@ -159,26 +169,24 @@ public AbstractTrafficShapingHandler(Executor executor, long writeLimit,
      *            channels or 0 if no stats are to be computed
      */
     public AbstractTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor,
+            ObjectSizeEstimator objectSizeEstimator, Timer timer,
             long writeLimit, long readLimit, long checkInterval) {
-        init(objectSizeEstimator, executor, writeLimit, readLimit,
-                checkInterval);
+        init(objectSizeEstimator, timer, writeLimit, readLimit, checkInterval);
     }
 
     /**
      * Constructor using default {@link ObjectSizeEstimator} and using default Check Interval
      *
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      * @param writeLimit
      *          0 or a limit in bytes/s
      * @param readLimit
      *          0 or a limit in bytes/s
      */
-    public AbstractTrafficShapingHandler(Executor executor, long writeLimit,
+    public AbstractTrafficShapingHandler(Timer timer, long writeLimit,
             long readLimit) {
-        init(new DefaultObjectSizeEstimator(), executor, writeLimit, readLimit,
-                DEFAULT_CHECK_INTERVAL);
+        init(new DefaultObjectSizeEstimator(), timer, writeLimit, readLimit, DEFAULT_CHECK_INTERVAL);
     }
 
     /**
@@ -187,29 +195,27 @@ public AbstractTrafficShapingHandler(Executor executor, long writeLimit,
      * @param objectSizeEstimator
      *            the {@link ObjectSizeEstimator} that will be used to compute
      *            the size of the message
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      * @param writeLimit
      *          0 or a limit in bytes/s
      * @param readLimit
      *          0 or a limit in bytes/s
      */
     public AbstractTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor,
+            ObjectSizeEstimator objectSizeEstimator, Timer timer,
             long writeLimit, long readLimit) {
-        init(objectSizeEstimator, executor, writeLimit, readLimit,
-                DEFAULT_CHECK_INTERVAL);
+        init(objectSizeEstimator, timer, writeLimit, readLimit, DEFAULT_CHECK_INTERVAL);
     }
 
     /**
      * Constructor using default {@link ObjectSizeEstimator} and using NO LIMIT and default Check Interval
      *
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      */
-    public AbstractTrafficShapingHandler(Executor executor) {
-        init(new DefaultObjectSizeEstimator(), executor, 0, 0,
-                DEFAULT_CHECK_INTERVAL);
+    public AbstractTrafficShapingHandler(Timer timer) {
+        init(new DefaultObjectSizeEstimator(), timer, 0, 0, DEFAULT_CHECK_INTERVAL);
     }
 
     /**
@@ -218,25 +224,25 @@ public AbstractTrafficShapingHandler(Executor executor) {
      * @param objectSizeEstimator
      *            the {@link ObjectSizeEstimator} that will be used to compute
      *            the size of the message
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      */
     public AbstractTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor) {
-        init(objectSizeEstimator, executor, 0, 0, DEFAULT_CHECK_INTERVAL);
+            ObjectSizeEstimator objectSizeEstimator, Timer timer) {
+        init(objectSizeEstimator, timer, 0, 0, DEFAULT_CHECK_INTERVAL);
     }
 
     /**
      * Constructor using default {@link ObjectSizeEstimator} and using NO LIMIT
      *
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      * @param checkInterval
      *          The delay between two computations of performances for
      *            channels or 0 if no stats are to be computed
      */
-    public AbstractTrafficShapingHandler(Executor executor, long checkInterval) {
-        init(new DefaultObjectSizeEstimator(), executor, 0, 0, checkInterval);
+    public AbstractTrafficShapingHandler(Timer timer, long checkInterval) {
+        init(new DefaultObjectSizeEstimator(), timer, 0, 0, checkInterval);
     }
 
     /**
@@ -245,20 +251,24 @@ public AbstractTrafficShapingHandler(Executor executor, long checkInterval) {
      * @param objectSizeEstimator
      *            the {@link ObjectSizeEstimator} that will be used to compute
      *            the size of the message
-     * @param executor
-     *          created for instance like Executors.newCachedThreadPool
+     * @param timer
+     *          created once for instance like HashedWheelTimer(10, TimeUnit.MILLISECONDS, 1024)
      * @param checkInterval
      *          The delay between two computations of performances for
      *            channels or 0 if no stats are to be computed
      */
     public AbstractTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor,
+            ObjectSizeEstimator objectSizeEstimator, Timer timer,
             long checkInterval) {
-        init(objectSizeEstimator, executor, 0, 0, checkInterval);
+        init(objectSizeEstimator, timer, 0, 0, checkInterval);
     }
 
     /**
      * Change the underlying limitations and check interval.
+     *
+     * @param newWriteLimit
+     * @param newReadLimit
+     * @param newCheckInterval
      */
     public void configure(long newWriteLimit, long newReadLimit,
             long newCheckInterval) {
@@ -268,17 +278,22 @@ public void configure(long newWriteLimit, long newReadLimit,
 
     /**
      * Change the underlying limitations.
+     *
+     * @param newWriteLimit
+     * @param newReadLimit
      */
     public void configure(long newWriteLimit, long newReadLimit) {
         writeLimit = newWriteLimit;
         readLimit = newReadLimit;
         if (trafficCounter != null) {
-            trafficCounter.resetAccounting(System.currentTimeMillis() + 1);
+            trafficCounter.resetAccounting(System.currentTimeMillis()+1);
         }
     }
 
     /**
      * Change the check interval.
+     *
+     * @param newCheckInterval
      */
     public void configure(long newCheckInterval) {
         checkInterval = newCheckInterval;
@@ -300,45 +315,25 @@ protected void doAccounting(TrafficCounter counter) {
 
     /**
      * Class to implement setReadable at fix time
- */
-    private class ReopenRead implements Runnable {
-        /**
-         * Associated ChannelHandlerContext
-         */
-        private ChannelHandlerContext ctx;
-
-        /**
-         * Time to wait before clearing the channel
-         */
-        private long timeToWait;
-
-        /**
-         * @param ctx
-         *            the associated channelHandlerContext
-         * @param timeToWait
-         */
-        protected ReopenRead(ChannelHandlerContext ctx, long timeToWait) {
+     */
+    private class ReopenReadTimerTask implements TimerTask {
+        ChannelHandlerContext ctx;
+        ReopenReadTimerTask(ChannelHandlerContext ctx) {
             this.ctx = ctx;
-            this.timeToWait = timeToWait;
         }
-
-        /**
-         * Truly run the waken up of the channel
-         */
-        public void run() {
-            try {
-                if (release.get()) {
-                    return;
-                }
-                Thread.sleep(timeToWait);
-            } catch (InterruptedException e) {
-                // interruption so exit
+        public void run(Timeout timeoutArg) throws Exception {
+            //logger.warn("Start RRTT: "+release.get());
+            if (release.get()) {
                 return;
             }
-            // logger.info("WAKEUP!");
+            /*
+            logger.warn("WAKEUP! "+
+                    (ctx != null && ctx.getChannel() != null &&
+                            ctx.getChannel().isConnected()));
+             */
             if (ctx != null && ctx.getChannel() != null &&
                     ctx.getChannel().isConnected()) {
-                //logger.info(" setReadable TRUE: "+timeToWait);
+                //logger.warn(" setReadable TRUE: ");
                 // readSuspended = false;
                 ctx.setAttachment(null);
                 ctx.getChannel().setReadable(true);
@@ -374,17 +369,18 @@ public void messageReceived(ChannelHandlerContext arg0, MessageEvent arg1)
                     return;
                 }
                 // compute the number of ms to wait before reopening the channel
-                long wait = getTimeToWait(readLimit, trafficCounter
-                        .getCurrentReadBytes(), trafficCounter.getLastTime(),
-                        curtime);
-                if (wait > MINIMAL_WAIT) { // At least 10ms seems a minimal time in order to
+                long wait = getTimeToWait(readLimit,
+                        trafficCounter.getCurrentReadBytes(),
+                        trafficCounter.getLastTime(), curtime);
+                if (wait >= MINIMAL_WAIT) { // At least 10ms seems a minimal
+                                            // time in order to
                     Channel channel = arg0.getChannel();
                     // try to limit the traffic
                     if (channel != null && channel.isConnected()) {
                         // Channel version
-                        if (executor == null) {
+                        if (timer == null) {
                             // Sleep since no executor
-                            //logger.info("Read sleep since no executor for "+wait+" ms for "+this);
+                            // logger.warn("Read sleep since no timer for "+wait+" ms for "+this);
                             if (release.get()) {
                                 return;
                             }
@@ -395,11 +391,14 @@ public void messageReceived(ChannelHandlerContext arg0, MessageEvent arg1)
                             // readSuspended = true;
                             arg0.setAttachment(Boolean.TRUE);
                             channel.setReadable(false);
-                            //logger.info("Read will wakeup after "+wait+" ms "+this);
-                            executor.execute(new ReopenRead(arg0, wait));
+                            // logger.warn("Read will wakeup after "+wait+" ms "+this);
+                            TimerTask timerTask = new ReopenReadTimerTask(arg0);
+                            timeout = timer.newTimeout(timerTask, wait,
+                                    TimeUnit.MILLISECONDS);
                         } else {
-                            // should be waiting: but can occurs sometime so as a FIX
-                            //logger.info("Read sleep ok but should not be here: "+wait+" "+this);
+                            // should be waiting: but can occurs sometime so as
+                            // a FIX
+                            // logger.warn("Read sleep ok but should not be here: "+wait+" "+this);
                             if (release.get()) {
                                 return;
                             }
@@ -407,7 +406,7 @@ public void messageReceived(ChannelHandlerContext arg0, MessageEvent arg1)
                         }
                     } else {
                         // Not connected or no channel
-                        //logger.info("Read sleep "+wait+" ms for "+this);
+                        // logger.warn("Read sleep "+wait+" ms for "+this);
                         if (release.get()) {
                             return;
                         }
@@ -432,11 +431,12 @@ public void writeRequested(ChannelHandlerContext arg0, MessageEvent arg1)
                 if (writeLimit == 0) {
                     return;
                 }
-                // compute the number of ms to wait before continue with the channel
-                long wait = getTimeToWait(writeLimit, trafficCounter
-                        .getCurrentWrittenBytes(), trafficCounter.getLastTime(),
-                        curtime);
-                if (wait > MINIMAL_WAIT) {
+                // compute the number of ms to wait before continue with the
+                // channel
+                long wait = getTimeToWait(writeLimit,
+                        trafficCounter.getCurrentWrittenBytes(),
+                        trafficCounter.getLastTime(), curtime);
+                if (wait >= MINIMAL_WAIT) {
                     // Global or Channel
                     if (release.get()) {
                         return;
@@ -449,7 +449,6 @@ public void writeRequested(ChannelHandlerContext arg0, MessageEvent arg1)
             super.writeRequested(arg0, arg1);
         }
     }
-
     @Override
     public void handleDownstream(ChannelHandlerContext ctx, ChannelEvent e)
             throws Exception {
@@ -485,7 +484,9 @@ public void releaseExternalResources() {
             trafficCounter.stop();
         }
         release.set(true);
-        ExecutorUtil.terminate(executor);
+        if (timeout != null) {
+            timeout.cancel();
+        }
     }
 
     @Override
diff --git a/src/main/java/org/jboss/netty/handler/traffic/ChannelTrafficShapingHandler.java b/src/main/java/org/jboss/netty/handler/traffic/ChannelTrafficShapingHandler.java
index 9ec72afd5ac..28389693ab8 100644
--- a/src/main/java/org/jboss/netty/handler/traffic/ChannelTrafficShapingHandler.java
+++ b/src/main/java/org/jboss/netty/handler/traffic/ChannelTrafficShapingHandler.java
@@ -15,8 +15,6 @@
  */
 package org.jboss.netty.handler.traffic;
 
-import java.util.concurrent.Executor;
-
 import org.jboss.netty.channel.ChannelHandlerContext;
 import org.jboss.netty.channel.ChannelPipelineFactory;
 import org.jboss.netty.channel.ChannelStateEvent;
@@ -24,6 +22,7 @@
 import org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor;
 import org.jboss.netty.handler.execution.OrderedMemoryAwareThreadPoolExecutor;
 import org.jboss.netty.util.ObjectSizeEstimator;
+import org.jboss.netty.util.Timer;
 
 /**
  * This implementation of the {@link AbstractTrafficShapingHandler} is for channel
@@ -33,8 +32,8 @@
  * <ul>
  * <li>Add in your pipeline a new ChannelTrafficShapingHandler, before a recommended {@link ExecutionHandler} (like
  * {@link OrderedMemoryAwareThreadPoolExecutor} or {@link MemoryAwareThreadPoolExecutor}).<br>
- * <tt>ChannelTrafficShapingHandler myHandler = new ChannelTrafficShapingHandler(executor);</tt><br>
- * executor could be created using <tt>Executors.newCachedThreadPool();</tt><br>
+ * <tt>ChannelTrafficShapingHandler myHandler = new ChannelTrafficShapingHandler(timer);</tt><br>
+ * timer could be created using <tt>HashedWheelTimer<tt><br>
  * <tt>pipeline.addLast("CHANNEL_TRAFFIC_SHAPING", myHandler);</tt><br><br>
  *
  * <b>Note that this handler has a Pipeline Coverage of "one" which means a new handler must be created
@@ -52,7 +51,7 @@
  * the less precise the traffic shaping will be. It is suggested as higher value something close
  * to 5 or 10 minutes.<br>
  * </li>
- * <li>When you shutdown your application, release all the external resources like the executor
+ * <li>When you shutdown your application, release all the external resources (except the timer internal itself)
  * by calling:<br>
  * <tt>myHandler.releaseExternalResources();</tt><br>
  * </li>
@@ -60,96 +59,53 @@
  */
 public class ChannelTrafficShapingHandler extends AbstractTrafficShapingHandler {
 
-    /**
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     * @param checkInterval
-     */
-    public ChannelTrafficShapingHandler(Executor executor, long writeLimit,
+    public ChannelTrafficShapingHandler(Timer timer, long writeLimit,
             long readLimit, long checkInterval) {
-        super(executor, writeLimit, readLimit, checkInterval);
+        super(timer, writeLimit, readLimit, checkInterval);
     }
 
-    /**
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     */
-    public ChannelTrafficShapingHandler(Executor executor, long writeLimit,
+    public ChannelTrafficShapingHandler(Timer timer, long writeLimit,
             long readLimit) {
-        super(executor, writeLimit, readLimit);
+        super(timer, writeLimit, readLimit);
     }
-    
-    
-    /**
-     * @param executor
-     * @param checkInterval
-     */
-    public ChannelTrafficShapingHandler(Executor executor, long checkInterval) {
-        super(executor, checkInterval);
+
+    public ChannelTrafficShapingHandler(Timer timer, long checkInterval) {
+        super(timer, checkInterval);
     }
 
-    /**
-     * @param executor
-     */
-    public ChannelTrafficShapingHandler(Executor executor) {
-        super(executor);
+    public ChannelTrafficShapingHandler(Timer timer) {
+        super(timer);
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     * @param checkInterval
-     */
     public ChannelTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor,
+            ObjectSizeEstimator objectSizeEstimator, Timer timer,
             long writeLimit, long readLimit, long checkInterval) {
-        super(objectSizeEstimator, executor, writeLimit, readLimit,
+        super(objectSizeEstimator, timer, writeLimit, readLimit,
                 checkInterval);
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     */
     public ChannelTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor,
+            ObjectSizeEstimator objectSizeEstimator, Timer timer,
             long writeLimit, long readLimit) {
-        super(objectSizeEstimator, executor, writeLimit, readLimit);
+        super(objectSizeEstimator, timer, writeLimit, readLimit);
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     * @param checkInterval
-     */
     public ChannelTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor,
+            ObjectSizeEstimator objectSizeEstimator, Timer timer,
             long checkInterval) {
-        super(objectSizeEstimator, executor, checkInterval);
+        super(objectSizeEstimator, timer, checkInterval);
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     */
     public ChannelTrafficShapingHandler(
-            ObjectSizeEstimator objectSizeEstimator, Executor executor) {
-        super(objectSizeEstimator, executor);
+            ObjectSizeEstimator objectSizeEstimator, Timer timer) {
+        super(objectSizeEstimator, timer);
     }
 
-
     @Override
     public void channelClosed(ChannelHandlerContext ctx, ChannelStateEvent e)
             throws Exception {
         if (trafficCounter != null) {
             trafficCounter.stop();
-            trafficCounter = null;
         }
         super.channelClosed(ctx, e);
     }
@@ -162,8 +118,10 @@ public void channelConnected(ChannelHandlerContext ctx, ChannelStateEvent e)
         ctx.getChannel().setReadable(false);
         if (trafficCounter == null) {
             // create a new counter now
-            trafficCounter = new TrafficCounter(this, executor, "ChannelTC" +
-                    ctx.getChannel().getId(), checkInterval);
+            if (timer != null) {
+                trafficCounter = new TrafficCounter(this, timer, "ChannelTC" +
+                        ctx.getChannel().getId(), checkInterval);
+            }
         }
         if (trafficCounter != null) {
             trafficCounter.start();
diff --git a/src/main/java/org/jboss/netty/handler/traffic/GlobalTrafficShapingHandler.java b/src/main/java/org/jboss/netty/handler/traffic/GlobalTrafficShapingHandler.java
index 7f54e28f296..b6f3f708f8a 100644
--- a/src/main/java/org/jboss/netty/handler/traffic/GlobalTrafficShapingHandler.java
+++ b/src/main/java/org/jboss/netty/handler/traffic/GlobalTrafficShapingHandler.java
@@ -15,13 +15,12 @@
  */
 package org.jboss.netty.handler.traffic;
 
-import java.util.concurrent.Executor;
-
 import org.jboss.netty.channel.ChannelHandler.Sharable;
 import org.jboss.netty.handler.execution.ExecutionHandler;
 import org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor;
 import org.jboss.netty.handler.execution.OrderedMemoryAwareThreadPoolExecutor;
 import org.jboss.netty.util.ObjectSizeEstimator;
+import org.jboss.netty.util.Timer;
 
 /**
  * This implementation of the {@link AbstractTrafficShapingHandler} is for global
@@ -31,8 +30,8 @@
  * The general use should be as follow:<br>
  * <ul>
  * <li>Create your unique GlobalTrafficShapingHandler like:<br><br>
- * <tt>GlobalTrafficShapingHandler myHandler = new GlobalTrafficShapingHandler(executor);</tt><br><br>
- * executor could be created using <tt>Executors.newCachedThreadPool();</tt><br>
+ * <tt>GlobalTrafficShapingHandler myHandler = new GlobalTrafficShapingHandler(timer);</tt><br><br>
+ * timer could be created using <tt>HashedWheelTimer<tt><br>
  * <tt>pipeline.addLast("GLOBAL_TRAFFIC_SHAPING", myHandler);</tt><br><br>
  *
  * <b>Note that this handler has a Pipeline Coverage of "all" which means only one such handler must be created
@@ -52,7 +51,7 @@
  * {@link OrderedMemoryAwareThreadPoolExecutor} or {@link MemoryAwareThreadPoolExecutor}).<br>
  * <tt>pipeline.addLast("GLOBAL_TRAFFIC_SHAPING", myHandler);</tt><br><br>
  * </li>
- * <li>When you shutdown your application, release all the external resources like the executor
+ * <li>When you shutdown your application, release all the external resources (except the timer internal itself)
  * by calling:<br>
  * <tt>myHandler.releaseExternalResources();</tt><br>
  * </li>
@@ -64,96 +63,61 @@ public class GlobalTrafficShapingHandler extends AbstractTrafficShapingHandler {
      * Create the global TrafficCounter
      */
     void createGlobalTrafficCounter() {
-        TrafficCounter tc = new TrafficCounter(this, executor, "GlobalTC",
-                checkInterval);
-        setTrafficCounter(tc);
-        tc.start();
+        TrafficCounter tc;
+        if (timer != null) {
+            tc = new TrafficCounter(this, timer, "GlobalTC",
+                    checkInterval);
+            setTrafficCounter(tc);
+            tc.start();
+        }
     }
 
-    /**
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     * @param checkInterval
-     */
-    public GlobalTrafficShapingHandler(Executor executor, long writeLimit,
+    public GlobalTrafficShapingHandler(Timer timer, long writeLimit,
             long readLimit, long checkInterval) {
-        super(executor, writeLimit, readLimit, checkInterval);
+        super(timer, writeLimit, readLimit, checkInterval);
         createGlobalTrafficCounter();
     }
 
-    /**
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     */
-    public GlobalTrafficShapingHandler(Executor executor, long writeLimit,
+    public GlobalTrafficShapingHandler(Timer timer, long writeLimit,
             long readLimit) {
-        super(executor, writeLimit, readLimit);
+        super(timer, writeLimit, readLimit);
         createGlobalTrafficCounter();
     }
-    /**
-     * @param executor
-     * @param checkInterval
-     */
-    public GlobalTrafficShapingHandler(Executor executor, long checkInterval) {
-        super(executor, checkInterval);
+
+    public GlobalTrafficShapingHandler(Timer timer, long checkInterval) {
+        super(timer, checkInterval);
         createGlobalTrafficCounter();
     }
 
-    /**
-     * @param executor
-     */
-    public GlobalTrafficShapingHandler(Executor executor) {
-        super(executor);
+    public GlobalTrafficShapingHandler(Timer timer) {
+        super(timer);
         createGlobalTrafficCounter();
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     * @param checkInterval
-     */
     public GlobalTrafficShapingHandler(ObjectSizeEstimator objectSizeEstimator,
-            Executor executor, long writeLimit, long readLimit,
+            Timer timer, long writeLimit, long readLimit,
             long checkInterval) {
-        super(objectSizeEstimator, executor, writeLimit, readLimit,
+        super(objectSizeEstimator, timer, writeLimit, readLimit,
                 checkInterval);
         createGlobalTrafficCounter();
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     * @param writeLimit
-     * @param readLimit
-     */
     public GlobalTrafficShapingHandler(ObjectSizeEstimator objectSizeEstimator,
-            Executor executor, long writeLimit, long readLimit) {
-        super(objectSizeEstimator, executor, writeLimit, readLimit);
+            Timer timer, long writeLimit, long readLimit) {
+        super(objectSizeEstimator, timer, writeLimit, readLimit);
         createGlobalTrafficCounter();
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     * @param checkInterval
-     */
     public GlobalTrafficShapingHandler(ObjectSizeEstimator objectSizeEstimator,
-            Executor executor, long checkInterval) {
-        super(objectSizeEstimator, executor, checkInterval);
+            Timer timer, long checkInterval) {
+        super(objectSizeEstimator, timer, checkInterval);
         createGlobalTrafficCounter();
     }
 
-    /**
-     * @param objectSizeEstimator
-     * @param executor
-     */
     public GlobalTrafficShapingHandler(ObjectSizeEstimator objectSizeEstimator,
-            Executor executor) {
-        super(objectSizeEstimator, executor);
+            Timer timer) {
+        super(objectSizeEstimator, timer);
         createGlobalTrafficCounter();
     }
+
 }
diff --git a/src/main/java/org/jboss/netty/handler/traffic/TrafficCounter.java b/src/main/java/org/jboss/netty/handler/traffic/TrafficCounter.java
index 32a0a236e02..2a63e5bf9a1 100644
--- a/src/main/java/org/jboss/netty/handler/traffic/TrafficCounter.java
+++ b/src/main/java/org/jboss/netty/handler/traffic/TrafficCounter.java
@@ -15,11 +15,14 @@
  */
 package org.jboss.netty.handler.traffic;
 
-import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.jboss.netty.channel.ChannelHandlerContext;
+import org.jboss.netty.util.Timeout;
+import org.jboss.netty.util.Timer;
+import org.jboss.netty.util.TimerTask;
 
 /**
  * TrafficCounter is associated with {@link AbstractTrafficShapingHandler}.<br>
@@ -97,27 +100,31 @@ public class TrafficCounter {
     /**
      * The associated TrafficShapingHandler
      */
-    private AbstractTrafficShapingHandler trafficShapingHandler;
+    private final AbstractTrafficShapingHandler trafficShapingHandler;
 
     /**
-     * Default Executor
+     * One Timer for all Counter
      */
-    private Executor executor;
-
+    private final Timer timer;  // replace executor
     /**
-     * Is Monitor active
+     * Monitor created once in start()
      */
-    AtomicBoolean monitorActive = new AtomicBoolean();
+    private TimerTask timerTask;
+    /**
+     * used in stop() to cancel the timer
+     */
+    volatile private Timeout timeout = null;
 
     /**
-     * Monitor
+     * Is Monitor active
      */
-    private TrafficMonitoring trafficMonitoring;
+    AtomicBoolean monitorActive = new AtomicBoolean();
 
     /**
      * Class to implement monitoring at fix delay
- */
-    private static class TrafficMonitoring implements Runnable {
+     *
+     */
+    private static class TrafficMonitoringTask implements TimerTask {
         /**
          * The associated TrafficShapingHandler
          */
@@ -132,42 +139,30 @@ private static class TrafficMonitoring implements Runnable {
          * @param trafficShapingHandler
          * @param counter
          */
-        protected TrafficMonitoring(
+        protected TrafficMonitoringTask(
                 AbstractTrafficShapingHandler trafficShapingHandler,
                 TrafficCounter counter) {
             trafficShapingHandler1 = trafficShapingHandler;
             this.counter = counter;
         }
 
-        /**
-         * Default run
-         */
-        public void run() {
-            try {
-                Thread.currentThread().setName(counter.name);
-                for (; counter.monitorActive.get();) {
-                    long check = counter.checkInterval.get();
-                    if (check > 0) {
-                        Thread.sleep(check);
-                    } else {
-                        // Delay goes to 0, so exit
-                        return;
-                    }
-                    long endTime = System.currentTimeMillis();
-                    counter.resetAccounting(endTime);
-                    if (trafficShapingHandler1 != null) {
-                        trafficShapingHandler1.doAccounting(counter);
-                    }
-                }
-            } catch (InterruptedException e) {
-                // End of computations
+        public void run(Timeout timeout) throws Exception {
+            if (!counter.monitorActive.get()) {
+                return;
             }
+            long endTime = System.currentTimeMillis();
+            counter.resetAccounting(endTime);
+            if (trafficShapingHandler1 != null) {
+                trafficShapingHandler1.doAccounting(counter);
+            }
+            timeout = 
+                counter.timer.newTimeout(this, counter.checkInterval.get(), TimeUnit.MILLISECONDS);                        
         }
     }
 
     /**
      * Start the monitoring process
- */
+     */
     public void start() {
         synchronized (lastTime) {
             if (monitorActive.get()) {
@@ -176,16 +171,16 @@ public void start() {
             lastTime.set(System.currentTimeMillis());
             if (checkInterval.get() > 0) {
                 monitorActive.set(true);
-                trafficMonitoring = new TrafficMonitoring(
-                        trafficShapingHandler, this);
-                executor.execute(trafficMonitoring);
+                timerTask = new TrafficMonitoringTask(trafficShapingHandler, this);
+                timeout = 
+                    timer.newTimeout(timerTask, checkInterval.get(), TimeUnit.MILLISECONDS);
             }
         }
     }
 
     /**
      * Stop the monitoring process
- */
+     */
     public void stop() {
         synchronized (lastTime) {
             if (!monitorActive.get()) {
@@ -196,6 +191,9 @@ public void stop() {
             if (trafficShapingHandler != null) {
                 trafficShapingHandler.doAccounting(this);
             }
+            if (timeout != null) {
+                timeout.cancel();
+            }
         }
     }
 
@@ -221,20 +219,20 @@ void resetAccounting(long newLastTime) {
     }
 
     /**
-     * Constructor with the {@link AbstractTrafficShapingHandler} that hosts it, the executorService to use, its
+     * Constructor with the {@link AbstractTrafficShapingHandler} that hosts it, the Timer to use, its
      * name, the checkInterval between two computations in millisecond
      * @param trafficShapingHandler the associated AbstractTrafficShapingHandler
-     * @param executor
-     *            Should be a CachedThreadPool for efficiency
+     * @param timer
+     *            Could be a HashedWheelTimer
      * @param name
      *            the name given to this monitor
      * @param checkInterval
      *            the checkInterval in millisecond between two computations
      */
     public TrafficCounter(AbstractTrafficShapingHandler trafficShapingHandler,
-            Executor executor, String name, long checkInterval) {
+            Timer timer, String name, long checkInterval) {
         this.trafficShapingHandler = trafficShapingHandler;
-        this.executor = executor;
+        this.timer = timer;
         this.name = name;
         lastCumulativeTime = System.currentTimeMillis();
         configure(checkInterval);
diff --git a/src/main/java/org/jboss/netty/handler/traffic/package-info.java b/src/main/java/org/jboss/netty/handler/traffic/package-info.java
index 25321804cb0..50ed87d3d5a 100644
--- a/src/main/java/org/jboss/netty/handler/traffic/package-info.java
+++ b/src/main/java/org/jboss/netty/handler/traffic/package-info.java
@@ -17,26 +17,28 @@
 /**
  * Implementation of a Traffic Shaping Handler and Dynamic Statistics.<br>
  * <br><br>
+ *
+ *
  * <P>The main goal of this package is to allow to shape the traffic (bandwidth limitation),
  * but also to get statistics on how many bytes are read or written. Both functions can
  * be active or inactive (traffic or statistics).</P>
  *
  * <P>Two classes implement this behavior:<br>
  * <ul>
- * <li> <tt>{@link org.jboss.netty.handler.traffic.TrafficCounter}</tt>: this class implements the counters needed by the handlers.
+ * <li> <tt>{@link TrafficCounter}</tt>: this class implements the counters needed by the handlers.
  * It can be accessed to get some extra information like the read or write bytes since last check, the read and write
  * bandwidth from last check...</li><br><br>
  *
- * <li> <tt>{@link org.jboss.netty.handler.traffic.AbstractTrafficShapingHandler}</tt>: this abstract class implements the kernel
+ * <li> <tt>{@link AbstractTrafficShapingHandler}</tt>: this abstract class implements the kernel
  * of the traffic shaping. It could be extended to fit your needs. Two classes are proposed as default
- * implementations: see {@link org.jboss.netty.handler.traffic.ChannelTrafficShapingHandler} and see {@link org.jboss.netty.handler.traffic.GlobalTrafficShapingHandler}
+ * implementations: see {@link ChannelTrafficShapingHandler} and see {@link GlobalTrafficShapingHandler}
  * respectively for Channel traffic shaping and Global traffic shaping.</li><br><br>
  *
  * The insertion in the pipeline of one of those handlers can be wherever you want, but
- * <b>it must be placed before any <tt>{@link org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor}</tt>
- * in your pipeline</b>.<br>
- * <b><i>It is really recommended to have such a</i> <tt>{@link org.jboss.netty.handler.execution.MemoryAwareThreadPoolExecutor}</tt>
- * <i>(either non ordered or </i> <tt>{@link org.jboss.netty.handler.execution.OrderedMemoryAwareThreadPoolExecutor}</tt>
+ * <b>it must be placed before any <tt>{@link MemoryAwareThreadPoolExecutor}</tt>
+ * in your pipeline</b>.</li><br>
+ * <b><i>It is really recommended to have such a</i> <tt>{@link MemoryAwareThreadPoolExecutor}</tt>
+ * <i>(either non ordered or </i> <tt>{@link OrderedMemoryAwareThreadPoolExecutor}</tt>
  * <i>) in your pipeline</i></b>
  * when you want to use this feature with some real traffic shaping, since it will allow to relax the constraint on
  * NioWorker to do other jobs if necessary.<br>
@@ -48,9 +50,9 @@
  * 60KB/s for each channel since NioWorkers are stopping by this handler.<br>
  * When it is used as a read traffic shaper, the handler will set the channel as not readable, so as to relax the
  * NioWorkers.<br><br>
- * An {@link org.jboss.netty.util.ObjectSizeEstimator} can be passed at construction to specify what
+ * An {@link ObjectSizeEstimator} can be passed at construction to specify what
  * is the size of the object to be read or write accordingly to the type of
- * object. If not specified, it will used the {@link org.jboss.netty.util.DefaultObjectSizeEstimator} implementation.<br><br>
+ * object. If not specified, it will used the {@link DefaultObjectSizeEstimator} implementation.<br><br>
  * </ul></P>
  *
  * <P>Standard use could be as follow:</P>
@@ -60,27 +62,27 @@
  * [Global or per Channel] [Write or Read] Limitation in byte/s.</li><br>
  * A value of <tt>0</tt>
  * stands for no limitation, so the traffic shaping is deactivate (on what you specified).<br>
- * You can either change those values with the method <tt>configure</tt> in {@link org.jboss.netty.handler.traffic.AbstractTrafficShapingHandler}.<br>
+ * You can either change those values with the method <tt>configure</tt> in {@link AbstractTrafficShapingHandler}.<br>
  * <br>
  *
  * <li>To activate or deactivate the statistics, you can adjust the delay to a low (suggested not less than 200ms
  * for efficiency reasons) or a high value (let say 24H in millisecond is huge enough to not get the problem)
  * or even using <tt>0</tt> which means no computation will be done.</li><br>
  * If you want to do anything with this statistics, just override the <tt>doAccounting</tt> method.<br>
- * This interval can be changed either from the method <tt>configure</tt> in {@link org.jboss.netty.handler.traffic.AbstractTrafficShapingHandler}
- * or directly using the method <tt>configure</tt> of {@link org.jboss.netty.handler.traffic.TrafficCounter}.<br><br>
+ * This interval can be changed either from the method <tt>configure</tt> in {@link AbstractTrafficShapingHandler}
+ * or directly using the method <tt>configure</tt> of {@link TrafficCounter}.<br><br>
  *
  * </ul></P><br><br>
  *
  * <P>So in your application you will create your own TrafficShapingHandler and set the values to fit your needs.</P>
- * <tt>XXXXXTrafficShapingHandler myHandler = new XXXXXTrafficShapingHandler(executor);</tt><br><br>
- * where executor could be created using <tt>Executors.newCachedThreadPool();</tt> and XXXXX could be either
+ * <tt>XXXXXTrafficShapingHandler myHandler = new XXXXXTrafficShapingHandler(timer);</tt><br><br>
+ * timer could be created using <tt>HashedWheelTimer<tt> and XXXXX could be either
  * Global or Channel<br>
  * <tt>pipeline.addLast("XXXXX_TRAFFIC_SHAPING", myHandler);</tt><br>
  * <tt>...</tt><br>
  * <tt>pipeline.addLast("MemoryExecutor",new ExecutionHandler(memoryAwareThreadPoolExecutor));</tt><br><br>
- * <P>Note that a new {@link org.jboss.netty.handler.traffic.ChannelTrafficShapingHandler} must be created for each new channel,
- * but only one {@link org.jboss.netty.handler.traffic.GlobalTrafficShapingHandler} must be created for all channels.</P>
+ * <P>Note that a new {@link ChannelTrafficShapingHandler} must be created for each new channel,
+ * but only one {@link GlobalTrafficShapingHandler} must be created for all channels.</P>
  *
  * <P>Note also that you can create different GlobalTrafficShapingHandler if you want to separate classes of
  * channels (for instance either from business point of view or from bind address point of view).</P>


Test patch: