HashedWheelTimer is leaking threads, am I doing something wrong?

Garry Watkins garry at dynafocus.com
Sat Nov 7 13:32:09 EST 2009


When the timeout occurs, how does it run the timeouthandler?  Does it  
do it from a thread pool, or does it run on the hashedWheelTimer's  
thread?

Another quick question.  I am writing a piece of PLC Communications  
software based on a request and response cycle, and I have one handler  
that needs to act as a codec because it needs to maintain state  
between the request and response.

My setup is like this

PLC device => (upstream starts) LengthBasedFramer => TCPFrameCODEC  
(checks to see if response.transid =  lastRequest.transid) = >  
BusinessLogicHandler (downstream starts) => TCPFrameCodec (increment  
lastRequest.transid) => PLC device

Is this the best way to do this?

Thanks

Garry Watkins

Managing Director
DynaFOCUS, LLC
1097 Rivershore Rd.
Charleston, SC 29492

mobile:  843-276-2808
garry at dynafocus.com



On Nov 7, 2009, at 10:17 AM, Trustin Lee (이희승) wrote:

> 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
>>
>
> _______________________________________________
> netty-users mailing list
> netty-users at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-users




More information about the netty-users mailing list