[jira] [Commented] (ZOOKEEPER-2737) NettyServerCnxFactory leaks connection if exception happens while writing to a channel.

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-2737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15940817#comment-15940817 ] 

ASF GitHub Bot commented on ZOOKEEPER-2737:
-------------------------------------------

GitHub user hanm opened a pull request:

    https://github.com/apache/zookeeper/pull/207

    ZOOKEEPER-2737: close netty connection when exceptions occur during w…

    …rite to channel to prevent resource leak.
    
    I am OK to add some contrived test case to test this but I'd like to do that later if needed, so this fix can get in upcoming releases..

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2737

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/207.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #207
    
----
commit ad3b98418b6739d14003753f913a894dc0868fc1
Author: Michael Han <ha...@apache.org>
Date:   2017-03-24T17:49:09Z

    ZOOKEEPER-2737: close netty connection when exceptions occur during write to channel.
    To prevent resource leak.

----


> NettyServerCnxFactory leaks connection if exception happens while writing to a channel.
> ---------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-2737
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2737
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: server
>    Affects Versions: 3.5.2
>            Reporter: Michael Han
>            Assignee: Michael Han
>            Priority: Critical
>              Labels: connection, netty, server
>
> Found this while debugging occasionally failed unit tests. Currently we do this if exception occurs during writing to a channel with Netty:
> {code}
> @Override
>         public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent e)
>             throws Exception
>         {
>             LOG.warn("Exception caught " + e, e.getCause());
>             NettyServerCnxn cnxn = (NettyServerCnxn) ctx.getAttachment();
>             if (cnxn != null) {
>                 if (LOG.isDebugEnabled()) {
>                     LOG.debug("Closing " + cnxn);
>                     cnxn.close();
>                 }
>             }
>         }
> {code}
> So the connection is only closed when debug mode is enabled. This is problematic as lots of clean up code is abstracted inside the close and without proper close the connection we are leaking resources.
> [Commit log|https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java#L147] indicates the issue exists since day 1 with ZOOKEEPER-733. Note the original patch uploaded to ZOOKEEPER-733 has this close call in right place, and the call gets moved around during iteration of the patches w/o gets noticed.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)