Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/700#discussion_r234871213
--- Diff:
zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
---
@@ -209,6 +209,8 @@ public void run() {
LOG.warn("Ignoring unexpected exception", e);
}
}
+ } catch (IOException e) {
+ LOG.error("Exception when running accept thread. Unable to
register selector?", e);
--- End diff --
If we are binding a port that's already in use here then I think we'll have
a problem because the expectation here is the acceptor thread will always be
available unless explicitly being shut down by caller (for this reason we
caught but ignored all checked and unchecked exception.). The problem is the
control flow does not reach higher level from within this acceptor thread -
thus if we have an instantiated but stopped `NIOServerCnxnFactory` due to
acceptor thread exceptions, the entire zk process could end up in a weird
state. Previous code does not have this issue because it tries to bind port
early and complain if it can't such that caller would catch the issue earlier
before the acceptor threads started. This should be easily testable if we spin
up a ZK server with an unavailable port with this fix and see what happens.
---
