[
https://issues.apache.org/jira/browse/ZOOKEEPER-2366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15338543#comment-15338543
]
Flavio Junqueira commented on ZOOKEEPER-2366:
---------------------------------------------
Thanks [~cnauroth].
bq. Since we're touching this code, do you think we should add a call to
Thread.currentThread().interrupt()?
The try/catch block wasn't there for the netty cnxn factory, I've added it in
this patch because the bind call does seem to throw some runtime exceptions so
I wanted to make sure we at least log the error in the case something goes
wrong. But, according to the documentation, {{ServerBootstrap.bind}} does not
throw {{InterruptedException}}, or am I reading it wrong?
bq. The Netty bind and close calls are asynchronous, so when this method
returns, it's not guaranteed that the bind and close have completed.
If the bind is asynchronous, then the server will simply not accept client
connection requests until it completes. As for the close, the worst that can
happen is that it accepts a connection request through the old port after the
reconfigure call completes. I don't see a problem, but you let me know if I'm
missing anything.
bq. let's take the opportunity to switch to the SLF4J calling convention with
{} token substitutions
Yep, done.
> 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)
