ZOOKEEPER-3158:firstConnect.countDown() will not be executed where sendThread.primeConnection() has thrown an exception

[maoling]

more details in ZOOKEEPER-3158

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2409/

[eolivelli]

Good.
What about adding a test case to demonstrate the problem and prevent regressions in the future?

[lvfangmin]

Agreed it’s better to have a test case to cover this.

[lvfangmin]

nit: remove tailing white space.

[maoling]

@eolivelli @lvfangmin
it was a issue found by p3c which is like findbug.
it's not very easy to add a UT and easy to check the correctness,WDYT?

[eolivelli]

@maoling Actually wakeupCnxn cannot really throw exceptions (I did not check the code when I reviewed the diff), so I think the adding a test is very difficult and so a big deal.

[eolivelli]

+1 (non binding)

[dineshappavoo]

This change LGTM 👍 . One followup, I am not familiar with the code flow, Just checking is it not possible to throw an exception in sendThread.primeConnection() in the try block(Some mock objects or so)

[maoling]

@eolivelli @dineshappavoo
you're right. I was mislead by void primeConnection() throws IOException which don't throw an IOException.
But,Follow the paradigm,put the firstConnect.countDown() into finally block is better?

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2488/