HashedWheelTimer is leaking threads, am I doing something wrong?

[Trustin Lee (이희승)]

A HashedWheelTimer creates only one thread.  If you create many
instances of HashedWheelTimer, you will see many threads, and it's
improper use.  There's no thread leak if used properly.

If you want to make it pull a thread from a pool, you can specify a
custom ThreadFactory in the constructor.   However, I don't think this
is related with thread leak.

— Trustin Lee, http://gleamynode.net/

On Wed, Nov 4, 2009 at 1:36 AM, Garry Watkins <garry at dynafocus.com> wrote:
> Ok I fixed item one.
>
> However for item two, shouldn't it just check threads out of the
> thread pool and return the thread when it is done?  We have clients
> with terrible networks, and it really bugs me that it will leak
> threads every time that the network disconnects my server.
>
> Thanks for your help
>
> Garry Watkins
>
> Managing Director
> DynaFOCUS, LLC
> 1097 Rivershore Rd.
> Charleston, SC 29492
>
> mobile:  843-276-2808
> garry at dynafocus.com
>
>
>
> On Nov 3, 2009, at 11:29 AM, Virat Gohil wrote:
>
>> - Show quoted text -
>> 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
>> _______________________________________________
>> netty-users mailing list
>> netty-users at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/netty-users
>
> _______________________________________________
> netty-users mailing list
> netty-users at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-users
>