gharris1727 commented on code in PR #12290:
URL: https://github.com/apache/kafka/pull/12290#discussion_r1425661185


##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/BlockingConnectorTest.java:
##########
@@ -139,7 +141,8 @@ public void setup() throws Exception {
     public void close() {
         // stop all Connect, Kafka and Zk threads.
         connect.stop();
-        Block.resetBlockLatch();
+        // unblock everything so that we don't leak threads after each test run
+        Block.reset();

Review Comment:
   I think that should take place in a test then, not in the cleanup.
   
   The reason I bring this up is that if I were to assert that the 
clients/threads are all stopped immediately after Block.reset() (as implemented 
in #14783) there's no synchronization to ensure that cleanup takes place before 
the assertion fires. The "asynchronous cleanup" initiated by Block.reset could 
exceed the lifetime of the test, still leaking the threads but only temporarily.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to