[
https://issues.apache.org/jira/browse/ZOOKEEPER-2366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15338808#comment-15338808
]
Chris Nauroth commented on ZOOKEEPER-2366:
------------------------------------------
Thanks for the updated patch [~fpj].
The comment about thread interruption referred to this piece of code in
{{NIOServerCnxnFactory}}:
{code}
try {
acceptThread.join();
} catch (InterruptedException e) {
LOG.error("Error joining old acceptThread when reconfiguring
client port {}",
e.getMessage());
}
{code}
Sorry if I was unclear about that. Again, this isn't introduced by your patch,
so feel free to defer this part of the feedback if you prefer.
I was concerned that the asynchronous bind might be a source of flaky tests,
but really the client-side retry logic would just take care of it. I think
you're right that it's not really a problem.
+1 for the patch. I leave it to you to decide whether or not to address the
thread interruption. I'd be +1 either way.
[~shralex], do you have further feedback?
> 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.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)
