Github user ivmaykov commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/669#discussion_r233652712 --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java --- @@ -103,71 +105,102 @@ boolean isConnected() { // Assuming that isConnected() is only used to initiate connection, // not used by some other connection status judgement. - return channel != null; + connectLock.lock(); + try { + return channel != null || connectFuture != null; --- End diff -- As the comment above says, the `isConnected()` method is only used in the main loop inside `ClientCnxn$SendThread.run()` to see if a new connection should be initiated. So, this method should return false if a connection attempt is already in progress. This is the case when `connectFuture` is not null. Arguably the method should be called `isConnectedOrConnecting()` but I didn't want to go around refactoring APIs in this diff - can do it if you like.
---