You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by lujiefsi <gi...@git.apache.org> on 2018/06/15 07:50:19 UTC

[GitHub] zookeeper pull request #544: ZOOKEEPER-3009 : fix the related bugs in branch...

GitHub user lujiefsi opened a pull request:

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

    ZOOKEEPER-3009 : fix the related bugs in branch-3.4

    The same problem that described in ZOOKEEPER-3009  also exists in branch-3.4, just as @hanm said. I pull a new request to fix it. The patch just make the related code of 3.4 be same as 3.5. please check!!!

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

    $ git pull https://github.com/lujiefsi/zookeeper ZOOKEEPER-3009-3.4

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

    https://github.com/apache/zookeeper/pull/544.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 #544
    
----
commit 82e310428268eedec58b0a88d1af9b9a265c999a
Author: lujie <lu...@...>
Date:   2018-06-15T07:42:07Z

    fix the ZOOKEEPER-3009 in branch-3.4

----


---

[GitHub] zookeeper pull request #544: ZOOKEEPER-3009 : fix the related bugs in branch...

Posted by lujiefsi <gi...@git.apache.org>.
Github user lujiefsi closed the pull request at:

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


---

[GitHub] zookeeper pull request #544: ZOOKEEPER-3009 : fix the related bugs in branch...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/544#discussion_r195958750
  
    --- Diff: src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java ---
    @@ -165,9 +168,13 @@ public void removeCnxn(NIOServerCnxn cnxn) {
                 }
     
                 synchronized (ipMap) {
    -                Set<NIOServerCnxn> s =
    -                        ipMap.get(cnxn.getSocketAddress());
    -                s.remove(cnxn);
    +                InetAddress addr = cnxn.getSocketAddress();
    +            	if (addr != null) {
    --- End diff --
    
    coding style issue: need one more space before the if statement so it aligns with previous statement. Other than this, patch looks good.


---

[GitHub] zookeeper pull request #544: ZOOKEEPER-3009 : fix the related bugs in branch...

Posted by lujiefsi <gi...@git.apache.org>.
Github user lujiefsi commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/544#discussion_r195967959
  
    --- Diff: src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java ---
    @@ -165,9 +168,13 @@ public void removeCnxn(NIOServerCnxn cnxn) {
                 }
     
                 synchronized (ipMap) {
    -                Set<NIOServerCnxn> s =
    -                        ipMap.get(cnxn.getSocketAddress());
    -                s.remove(cnxn);
    +                InetAddress addr = cnxn.getSocketAddress();
    +            	if (addr != null) {
    --- End diff --
    
    tabs led the bad coding style, I have removed it.


---

[GitHub] zookeeper pull request #544: ZOOKEEPER-3009 : fix the related bugs in branch...

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/544#discussion_r204217429
  
    --- Diff: src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java ---
    @@ -129,11 +129,14 @@ public int getLocalPort(){
             return ss.socket().getLocalPort();
         }
     
    -    private void addCnxn(NIOServerCnxn cnxn) {
    +    private void addCnxn(NIOServerCnxn cnxn) throws IOException {
             synchronized (cnxns) {
                 cnxns.add(cnxn);
                 synchronized (ipMap){
    -                InetAddress addr = cnxn.sock.socket().getInetAddress();
    +                InetAddress addr = cnxn.getSocketAddress();
    +                if (addr == null) {
    +                	throw new IOException("Socket of " + cnxn + " has been closed");
    --- End diff --
    
    nit: keep indent.


---

[GitHub] zookeeper issue #544: ZOOKEEPER-3009 : fix the related bugs in branch-3.4

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:

    https://github.com/apache/zookeeper/pull/544
  
    committed to 3.4, thanks @lujiefsi !
    Please close this pull request.


---