HashedWheelTimer is leaking threads, am I doing something wrong?

[Garry Watkins]

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