HashedWheelTimer is leaking threads, am I doing something wrong?

[Virat Gohil]

On Tue, Nov 3, 2009 at 9:51 PM, Garry Watkins <garry at dynafocus.com> wrote:
>
> Here is the pipeline factory code:
>
> public class PassThroughMasterPipelineFactory implements
> ChannelPipelineFactory {
>    private short unitId;
>    private Properties properties;
>    private ClientBootstrap bootStrap;
>
>    public PassThroughMasterPipelineFactory(short unitId, Properties
> properties, ClientBootstrap bootStrap) {
>        this.unitId = unitId;
>        this.properties = properties;
>        this.bootStrap = bootStrap;
>    }
>
>    public ChannelPipeline getPipeline() throws Exception {
>        ChannelPipeline pipeline = pipeline();
>
> //        pipeline.addLast("logger", new
> LoggingHandler(InternalLogLevel.INFO, false));
>
>        Timer timer = new HashedWheelTimer();
>        pipeline.addLast("reconnect", new PLCDisconnectHandler(bootStrap,
> timer));
>        pipeline.addLast("timeout", new IdleStateHandler(timer, 5, 10, 0));
>
>        UnitronicsLengthFieldBasedDecoder tcpFramer = new
> UnitronicsLengthFieldBasedDecoder();
>        pipeline.addLast("tcpFramer", tcpFramer);
>
>        pipeline.addLast("tcpCodec", new TCPFrameCodec());
>
>        PLCStateMachine stateMachine = new PassThroughStateMachine(unitId);
>        stateMachine.initializeScheduler(properties);
>        pipeline.addLast("handler", new
> PLCStateMachineHandler(stateMachine));
>
>        pipeline.addLast("test", new RequestStatisticHandler());
>
>        return pipeline;
>    }
>
> }
>
>
> Garry Watkins wrote:
>>
>> I have a reconnect handler that handles closed channels and it will try to
>> connect up after 10 seconds.  Every time that it opens the connection back
>> up it creates a new thread.  I am not sure if it is the wheel timer is
>> causing the leak or if there is something else.  I am willing to try and
>> debug this if I can get a pointer in the right direction.
>>
>> Here is what my code looks like:
>>
>> @ChannelPipelineCoverage("one")
>> public class PLCDisconnectHandler extends SimpleChannelUpstreamHandler
>> {
>>     private static final Log LOG =
>> LogFactory.getLog(PLCDisconnectHandler.class);
>>
>>     final ClientBootstrap bootstrap;
>>     private final Timer timer;
>>
>>     public PLCDisconnectHandler(ClientBootstrap bootstrap, Timer timer) {
>>         this.bootstrap = bootstrap;
>>         this.timer = timer;
>>     }
>>
>>     @Override
>>     public void channelOpen(ChannelHandlerContext ctx, ChannelStateEvent
>> e) throws Exception
>>     {
>>         LOG.error("channelOpen");
>>
>>         super.channelOpen(ctx, e);
>>     }
>>
>>     @Override
>>     public void channelConnected(ChannelHandlerContext ctx,
>> ChannelStateEvent e) throws Exception {
>>         LOG.error("channelConnected: " +
>> ctx.getChannel().getRemoteAddress().toString());
>>
>>         super.channelConnected(ctx, e);
>>     }
>>
>>     @Override
>>     public void channelDisconnected(ChannelHandlerContext ctx,
>> ChannelStateEvent e) throws Exception
>>     {
>>         LOG.error("Channel disconnected: " +
>> ctx.getChannel().getRemoteAddress().toString());
>>
>>         super.channelDisconnected(ctx, e);
>>     }
>>
>>     @Override
>>     public void channelClosed(ChannelHandlerContext ctx, ChannelStateEvent
>> e) {
>>         LOG.error("channelClosed");
>>
>>         timer.newTimeout(new TimerTask() {
>>             public void run(Timeout timeout) throws Exception {
>>                 timeout.cancel();
>>                 bootstrap.connect();
>>             }
>>         }, 10, TimeUnit.SECONDS);
>>     }
>>
>>     @Override
>>     public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent
>> e) throws Exception {
>>         if (e.getCause() instanceof ConnectException){
>>             ctx.getChannel().close();
>>         }
>>     }
>> }
>>
>> TIA
>> Garry
>>
>
> --
> View this message in context: http://n2.nabble.com/HashedWheelTimer-is-leaking-threads-am-I-doing-something-wrong-tp3939379p3939502.html
> Sent from the Netty User Group mailing list archive at Nabble.com.
> _______________________________________________
> netty-users mailing list
> netty-users at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-users
>

Hmm,

1. This is the reasons for leaks

>    public ChannelPipeline getPipeline() throws Exception {
>        Timer timer = new HashedWheelTimer();
>        pipeline.addLast("reconnect", new PLCDisconnectHandler(bootStrap,
> timer));
>        pipeline.addLast("timeout", new IdleStateHandler(timer, 5, 10, 0));

You dont actually need a new instance for every pipeline, I suggest
that you create a single instance of timer and share it across all
channels as follows:


 public class PassThroughMasterPipelineFactory implements
 ChannelPipelineFactory {
    private short unitId;
    private Properties properties;
    private ClientBootstrap bootStrap;
    private Timer timer;
    public PassThroughMasterPipelineFactory(short unitId, Properties
 properties, ClientBootstrap bootStrap) {
        this.unitId = unitId;
        this.properties = properties;
        this.bootStrap = bootStrap;
        this.timer =new HashedWheelTimer();
    }
    public ChannelPipeline getPipeline() throws Exception {
        pipeline.addLast("reconnect", new
PLCDisconnectHandler(bootStrap, timer));
        pipeline.addLast("timeout", new IdleStateHandler(timer, 5, 10, 0));
    }

And second:

>>         timer.newTimeout(new TimerTask() {
>>             public void run(Timeout timeout) throws Exception {
>>                 timeout.cancel();
>>                 bootstrap.connect();
>>             }
>>         }, 10, TimeUnit.SECONDS);

It basically creates a new thread every time a connection is closed.

Thanks,

Virat