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

Alexander Shraer commented on ZOOKEEPER-2366:
---------------------------------------------

Two comments about the test:
1. It still checks follower's client port change, but not leader's client port 
change, which we may also want to do as suggested in the comments above. 
2. There's a try block without a catch. 
  try (ServerSocket ss2 = new ServerSocket()) {
+                ss2.bind(new InetSocketAddress(getLoopbackAddress(), 
oldClientPort));
+            }

Is this intentional ? The test also throws an exception.
Maybe the intention is to bind the old port and then unbind when ss2 goes out 
of scope. Is this release instantanious or could it
fail the following code that needs the old port vacant ? Maybe we should just 
remove this try block ?

Another question: could Thread.currentThread().interrupt() somehow crash the 
server ? 


> Reconfiguration of client port causes a socket leak
> ---------------------------------------------------
>
>                 Key: ZOOKEEPER-2366
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2366
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: quorum
>    Affects Versions: 3.5.0
>            Reporter: Timothy Ward
>            Assignee: Flavio Junqueira
>            Priority: Blocker
>             Fix For: 3.5.2
>
>         Attachments: ZOOKEEPER-2366.patch, ZOOKEEPER-2366.patch, 
> ZOOKEEPER-2366.patch, ZOOKEEPER-2366.patch, ZOOKEEPER-2366.patch, 
> zookeeper.patch
>
>
> The NIOServerCnxnFactory reconfigure method can leak server sockets, and 
> hence make ports unusable until the JVM restarts:
> The first line of the method takes a reference to the current 
> ServerSocketChannel and then the next line replaces it. The subsequent 
> interactions with the server socket can fail (for example if the 
> reconfiguration tries to bind to an in-use port). If they fail *before* the  
> call to oldSS.close() then oldSS is *never* closed. This holds that port open 
> forever, and prevents the user from rolling back to the previous port!
> The code from reconfigure is shown below:
>  ServerSocketChannel oldSS = ss;        
>         try {
>            this.ss = ServerSocketChannel.open();
>            ss.socket().setReuseAddress(true);
>            LOG.info("binding to port " + addr);
>            ss.socket().bind(addr);
>            ss.configureBlocking(false);
>            acceptThread.setReconfiguring();
>            oldSS.close();           
>            acceptThread.wakeupSelector();
>            try {
>                         acceptThread.join();
>                  } catch (InterruptedException e) {
>                          LOG.error("Error joining old acceptThread when 
> reconfiguring client port " + e.getMessage());
>                  }
>            acceptThread = new AcceptThread(ss, addr, selectorThreads);
>            acceptThread.start();
>         } catch(IOException e) {
>            LOG.error("Error reconfiguring client port to " + addr + " " + 
> e.getMessage());
>         }



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to