Uploaded image for project: 'ZooKeeper'
  1. ZooKeeper
  2. ZOOKEEPER-2737

NettyServerCnxFactory leaks connection if exception happens while writing to a channel.

    Details

      Description

      Found this while debugging occasionally failed unit tests. Currently we do this if exception occurs during writing to a channel with Netty:

      @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();
                      }
                  }
              }
      

      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 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.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          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 <hanm@apache.org>
          Date: 2017-03-24T17:49:09Z

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


          Show
          githubbot ASF GitHub Bot added a comment - 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 <hanm@apache.org> Date: 2017-03-24T17:49:09Z ZOOKEEPER-2737 : close netty connection when exceptions occur during write to channel. To prevent resource leak.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. GitHub Pull Request Build

          +1 @author. The patch does not contain any @author tags.

          +0 tests included. The patch appears to be a documentation patch that doesn't require tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/478//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/478//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/478//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/478//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/478//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/478//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user enixon commented on the issue:

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

          good catch!

          Show
          githubbot ASF GitHub Bot added a comment - Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/207 good catch!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user afine commented on the issue:

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

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user afine commented on the issue: https://github.com/apache/zookeeper/pull/207 +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user phunt commented on the issue:

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

          Ugh, sorry about that.

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user phunt commented on the issue: https://github.com/apache/zookeeper/pull/207 Ugh, sorry about that. +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rakeshadr commented on the issue:

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

          Thanks @hanm, +1 LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user rakeshadr commented on the issue: https://github.com/apache/zookeeper/pull/207 Thanks @hanm, +1 LGTM
          Hide
          hanm Michael Han added a comment -

          Issue resolved by pull request 207
          https://github.com/apache/zookeeper/pull/207

          Show
          hanm Michael Han added a comment - Issue resolved by pull request 207 https://github.com/apache/zookeeper/pull/207
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/zookeeper/pull/207
          Show
          hanm Michael Han added a comment - Committed. Master: https://github.com/apache/zookeeper/commit/d8adc547f9856747905b7d46450f13fa98df147f 3.5: https://github.com/apache/zookeeper/commit/5c356f5a47402c000b5e206a536273afc75de883
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build ZooKeeper-trunk #3334 (See https://builds.apache.org/job/ZooKeeper-trunk/3334/)
          ZOOKEEPER-2737: close netty connection when exceptions occur during w… (hanm: rev d8adc547f9856747905b7d46450f13fa98df147f)

          • (edit) src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build ZooKeeper-trunk #3334 (See https://builds.apache.org/job/ZooKeeper-trunk/3334/ ) ZOOKEEPER-2737 : close netty connection when exceptions occur during w… (hanm: rev d8adc547f9856747905b7d46450f13fa98df147f) (edit) src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java

            People

            • Assignee:
              hanm Michael Han
              Reporter:
              hanm Michael Han
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development