[GitHub] [helix] kaisun2000 commented on a change in pull request #1227: Implement thread leakage check for each test class

[GitBox <gi...@apache.org>]

kaisun2000 commented on a change in pull request #1227:
URL: https://github.com/apache/helix/pull/1227#discussion_r467242429



##########
File path: helix-core/src/test/java/org/apache/helix/common/ZkTestBase.java
##########
@@ -719,6 +720,12 @@ public void cleanupLiveInstanceOwners() {
       clientMap.clear();
     }
     _liveInstanceOwners.clear();
+
+    boolean status = TestHelper.afterClassCheck(name);
+    // Assert here does not work.
+    if (!status) {
+      System.out.println("---------- Test Class " + name + " thread leakage detected! ---------------");

Review comment:
       This is something I spent a lot of time yesterday  Can't make it work this way. See comment 725 "Assert does not work". 
   
   On the hand, after testing /running this diff many time. I found this may not be a good idea to fail it outright. The reasons are:
   
   - closing Zk Session, Cancel() timer thread or executors by our code and till the thread there are shutdown are async. There are some time delta before these threads tearing down from cancel() time. And it is out of our control. For example, closing Zk session via Zookeeper client library. But the sendThread and eventThread associated with this session is not teared down before Zookeeper return from close() and hand the execution to our code. 
   - Some threads created from stream() etc java functional framework, such as forkjoinwork, or pool- can happen. We don't have control of their life time. 
   
   Thus, sometimes some of these to-be-closed threads would appear here. So relying on human examination here may not be a bad idea for now. Let me know what is your take?
   




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org