tisonkun commented on code in PR #430: URL: https://github.com/apache/curator/pull/430#discussion_r974852099
########## curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java: ########## @@ -190,6 +190,12 @@ public void close() throws IOException close(closeMode); } + // for testing + void closeOnDemand() throws IOException + { + internalClose(closeMode, false); + } Review Comment: I think it's a design issue we can resolve in another thread. Generally, I'd prefer `close` to be idempotent but the original code isn't. Your test case can fail if: 1. Checking state != CLOSE. 2. Internally closed. 3. Outside closed. And it happens in the CI environment once. Your suggestion can be reasonable but I don't want to spend too much time on this bug fix PR. A package-level visible method is for internal usage :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@curator.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org