[
https://issues.apache.org/jira/browse/ZOOKEEPER-2366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15295250#comment-15295250
]
Flavio Junqueira commented on ZOOKEEPER-2366:
---------------------------------------------
bq. In case the new port is taken, perhaps its better to leave the old port in
effect ? (rather than to remain without a client port at all like now, which is
expected in the new test)
Maybe so, my concern is that processReconfig-> reconfigure is called upon
processing commit and activate and in the case there is a problem binding to
the port, the current code will simply return successfully from reconfigure
like if the processing of the message had succeeded, which it hasn't. The
behavior sounds wrong to me, does it sound right to you?
bq. Error message in Netty method is less informative. Consider making messages
consistent in both methods.
Sure, I can change it.
bq. I'm not sure simply returning false in Leader.java is sufficient. That
operation may forever remain in the queue ? I'm not sure about this since the
new test somehow works.
Sounds like you have some more insight further down.
bq. Indentation seems a bit weird in the changes in QuorumPeer and in the test.
Ok, will fix
> 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.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)
