You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@zookeeper.apache.org by PhantomThief <gi...@git.apache.org> on 2018/08/07 00:48:54 UTC

[GitHub] zookeeper pull request #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAd...

GitHub user PhantomThief opened a pull request:

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

    ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressException on conn…

    …ect.

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

    $ git pull https://github.com/PhantomThief/zookeeper master

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

    https://github.com/apache/zookeeper/pull/592.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 #592
    
----
commit 9ddf17813efd4a771b697a7883a1fbafe13b6ccd
Author: w.vela <wa...@...>
Date:   2018-08-07T00:47:03Z

    ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressException on connect.

----


---

[GitHub] zookeeper pull request #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAd...

Posted by lvfangmin <gi...@git.apache.org>.

Github user lvfangmin commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/592#discussion_r208814126
  
    --- Diff: src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java ---
    @@ -280,10 +281,14 @@ void registerAndConnect(SocketChannel sock, InetSocketAddress addr)
         
         @Override
         void connect(InetSocketAddress addr) throws IOException {
    +        // if UnresolvedAddressException throw on connect and it would leak a SocketChannelImpl in cancelledKeys EVERY time.
    +        // it's hard to deal all situations on socks.cancel,
    +        // while an easy way is, ensure DNS resolve successful before register channel.
    +        InetAddress.getByName(addr.getHostName());
    --- End diff --
    
    Can you help me understand why sock.close won't clean up the fd?
    
    If we have to check before register, I would suggest to use explicit check here, like:
    
        if (addr.isUnresolved()) {
            throw new UnresolvedAddressException();
        }
    



---

[GitHub] zookeeper issue #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressEx...

Posted by anmolnar <gi...@git.apache.org>.

Github user anmolnar commented on the issue:

    https://github.com/apache/zookeeper/pull/592
  
    Nice catch @PhantomThief 
    Do you think writing a unit test would worth to cover the issue?


---

[GitHub] zookeeper issue #592: ZOOKEEPER-3112: fix fd leak due to UnresolvedAddressEx...

Posted by PhantomThief <gi...@git.apache.org>.

Github user PhantomThief commented on the issue:

    https://github.com/apache/zookeeper/pull/592
  
    @anmolnar a bad news is, after my test case, it won't fix the fd leak...so I'm adding an another patch.


---