0

Note: This not trying to see if a task that has been submitted to the thread pool has completed, but whether the actual thread has exited.

In order to detect thread leaks I'd like to have code of the form

val start = allThreads()
doStuff()
val end = allThreads()
assert start == end

However, doStuff() may use one or more thread pools, which are actually cleaned up by calling shutdownNow() or similar. There doesn't seem to be a way in a ThreadPoolExecutor to detect whether all threads have terminated. Things like awaitTermination only make sure that all threads have gotten to the point that they will definitely exit, but not that they already have.

Ideally the solution is not hacky such as using reflection to access the internals of the class directly (from which of course we can get all threads then join them all).

UPDATE: The reason for doing this

If you have a large codebase developed over some time you may have a problem with thread leaks. People may not shut down executors appropriately, people may just call new Thread(), people may call third party libraries that start threads and either the code or the library's code doesn't exit those threads. Your builds have failed due to way too many threads running in a single process as that process may run thousands of tests each of which would leak a few threads.

To prevent this, you force your tests to all check that the threads before the test and threads after are the same set. This can be done e.g. by reflection, verifying that every test class inherits from a base class, then in the base class having Before/After to verify threads. The details aren't important but basically we have a thread leak detector that fails the test on leaked threads.

Now if you are properly using a ThreadPoolExecutor and calling shutdownNow, then we do not want the test to fail due to the leak detector. However, just using shutdownNow can cause false positives because even though we successfully called shutdownNow and returned from the rest of the code and are in the After phase where we check current threads, the thread pool threads may still be alive at that time. Therefore we want some way to guarantee that the threads in the pool have already exited before returning, in order to avoid these false positives.

Jamie
  • 3,901
  • 3
  • 23
  • 27
  • `CountDownLatch`? – MadProgrammer Apr 30 '18 at 23:09
  • 2
    Whether a thread is actually destroyed is the very abstraction which a thread pool is designed to hide. What is the benefit of knowing this? – VGR Apr 30 '18 at 23:40
  • If you need to know whether certain tasks have finished executing then perhaps you should use `ExecutorService` instead since it can return `Future` objects. – D.B. May 01 '18 at 04:54
  • @VGR as mentioned, to make sure that things don't leak threads. e.g. missing a call to `shutdownNow()` or raw threads doing something or whatnot. And I could certainly see some flag telling you whether all resources besides jvm memory has been released that doesn't really break the abstraction and is useful enough to be worthwhile. – Jamie May 01 '18 at 14:36
  • @D.B.as mentioned in the question, my goal is not to see if certain tasks have finished running which I am quite aware of already. – Jamie May 01 '18 at 14:36
  • @MadProgrammer how so? Without modifying ThreadPoolExecutor code I don't quite see the solution. – Jamie May 01 '18 at 14:37
  • I’m still not seeing the value here. What does this provide you that waiting for individual tasks’ `Future`s does not? – VGR May 01 '18 at 15:20
  • @VGR You have `bigBlockOfCode` which runs in unit tests. Since you have thousands of unit tests, you want to make sure your tests don't leak threads as otherwise you will likely run into transient issues as the number of threads becomes large. Now ideally `bigBlockOfCode` only uses executors and shuts them down when done, but how do you verify that? Checking if `beforeThreads` and `afterThreads` are the same. But then, you want to make sure your executors don't leave threads running even if for a bit as they may be false positives. – Jamie May 02 '18 at 01:07
  • And, why can’t you shutdown the ExecutorService and then call awaitTermination to accomplish that? – VGR May 02 '18 at 04:35
  • @VGR because based on empirical evidence and a reading of the code, when awaitTermination returns it only guarantees that threads will soon exit, and haven't necessarily exited yet. Which is great for your normal use cases but problematic for my additional one. – Jamie May 02 '18 at 13:42
  • And again I ask: As long as you’re certain your task has finished, why do you need to know whether the thread has terminated? If your task is done, it can’t be causing a leak. – VGR May 02 '18 at 20:44
  • @VGR if you have a giant codebase, you don't know if everything is using executors appropriately. Someone may be just doing `new Thread` and have it launch something that never exits. Or you might use a third party library that you forgot to call `cleanup` on and which would then leak thread. Or you use an executor but forget to call `shutdownNow`. So you want to catch these cases and fail the test then in your tests you compare the threads at start and end. Which may have false positives if your executor returns from shutdownNow without all threads exiting. – Jamie May 03 '18 at 00:41
  • @VGR I have also updated the question with a longer rationale. – Jamie May 03 '18 at 00:50
  • It sounds like you want a way to kill threads whose Runnables are not under your control. As [explained here](https://stackoverflow.com/questions/671049/how-do-you-kill-a-thread-in-java), it cannot be done. – VGR May 03 '18 at 01:03
  • @VGR No just to make sure they are dead (which in ThreadPoolExecutors they will be soon). The posted answer (with modifications) provides a good way to do it by having a ThreadFactory which keeps track of the created threads and then you call `join` on the threads after the call to `shutdownNow` succeeds. – Jamie May 03 '18 at 04:29

1 Answers1

1

Use a Thread factory.

    ThreadFactory tf = runnable -> {
        return new Thread(() -> {
            try {
                runnable.run();
            } finally {
                System.out.println(Thread.currentThread().getName()+": my thread exit");
            }
        });
    };

    ExecutorService svc = Executors.newFixedThreadPool(3, tf);
    Future f = svc.submit(() -> System.out.println(Thread.currentThread().getName()+": some task"));
    f.get();
    svc.shutdown();

    Thread.sleep(2000);
    System.out.println("main done");
user2023577
  • 1,752
  • 1
  • 12
  • 23
  • Hmm interesting, I guess if you keep track of the threads created by the ThreadFactory then you can join them at the end. Wonder about the downsides of it... – Jamie May 01 '18 at 14:41
  • I didn't argue your question, but I can't imagine a reason to want to wait for the death of those threads. Either you misuse the executor service, or there is something very specific you are not telling. The shutdown(Now)() methods are doing what they shouldm and the only reason they would fail is if your tasks are not interruptable or not ending promptly, which you should know another way since you submitted those tasks. – user2023577 May 01 '18 at 21:42
  • shutdownNow only makes it so the threads are in a place where they're about to exit. Now if you are trying to detect thread leaks in a large block of code, if you check for leaks soon after shutting down the executor, you may get false positive. Now, why check for leaks? If you properly use executors always and shut them down after use, then no need. However in a large codebase you want to detect people using raw threads and whatever else they may use and not cleaning them up appropriately. – Jamie May 02 '18 at 01:10
  • @Jamie: Ok. I get it. You are adding runtime complexity to monitor a design time problem. But if you can't control where they will create new exec services, you won't have a chance to put a thread factory either, or intercept anything. Can't ban ban thread creation with a security manager (if even possible), it's too restrictive. Unit tests can be more aggressive, but again, bad code already slip through so insufficient tests will too. Seems to me you need to poll the mxthread bean for thread count and alarm when too high. – user2023577 May 02 '18 at 11:37
  • I'm using Before and After in unit tests to get the thread list before and after and comparing them, so the threads will get checked no matter what. The only point of the thread factory is to make sure the threads exit in a timely manner so that it doesn't cause false positives on the leak detector. – Jamie May 02 '18 at 13:45
  • And since the Before and After are in a base class, and there is a test to make sure that (eventually) all test classes inherit from that base class, it would be hard to accidentally miss the checks. – Jamie May 02 '18 at 13:46