9

I am submitting Callable objects to a ThreadPoolExecutor and they seem to be sticking around in memory.

Looking at the heap dump with the MAT tool for Eclipse see that the Callable objects are being referenced by a FutureTask$Sync's callable variable. That FutureTask$Sync is referenced by a FutureTask's sync variable. That FutureTask is referenced by the FutureTask$Sync's this$0 variable.

I have read around about this (here, here, and on SO) and it seems like the FutureTask that the callable is wrapped in upon the ThreadPoolExecutor's submit() holds a reference to the callable forever.

What I am confused about is how to ensure that the FutureTask gets garbage collected so it doesn't continue to hold the callable in memory, and hold anything the callable might be holding in memory?

Just to give more details about my particular situation, I am trying to implement the ThreadPoolExecutor in a way that allows all of the submitted tasks to be canceled if needed. I have tried several different methods I found on SO and elsewhere, such as completely shutting the executor down (with shutdown(), shutdownNow() etc) and also keeping a list of the futures return by submit() and calling cancel on all them and then clearing the list of futures. Ideally I would like not to have to shut it down, and just cancel() and clear out when needed.

All of these methods don't seem to make a difference. If I submit a callable to the pool, there is a good chance it will end up sticking around.

What am I doing wrong?

Thanks.

Edit:

As requested, here is the constructor for the ThreadPoolExecutor.

public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, BlockingQueue<Runnable> workQueue) {
    super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue);
}

After further testing I can see that if I let the tasks that have been submitted to the ThreadPoolExecutor finish, then there is no leak. If I try to cancel them in anyway such as:

shutdownNow()

Or saving a reference to the future and calling cancel on it later:

Future referenceToCancelLater = submit(task);
...
referenceToCancelLater.cancel(false);

Or by removing them from the queue with methods like:

getQueue.drainTo(someList)

or

getQueue.clear()

or Looping through saved references to the futures and calling:

getQueue.remove(task)

Any of those cases causes the FutureTask to stick around as described above.

So the real question in all of this is how to I properly cancel or remove items from a ThreadPoolExecutor so that the FutureTask is garbage collected and not leaked forever?

cottonBallPaws
  • 21,220
  • 37
  • 123
  • 171
  • 1
    post your code, as of hard remove way: `ThreadPoolExecutor.getQueue().remove(future)` will do the trick – bestsss Feb 07 '11 at 00:15
  • There is some discussion on whether getQueue() should be used that way. Is there really a drawback to doing it that way? – cottonBallPaws Feb 07 '11 at 00:18
  • @bestsss I was hoping I wouldn't get asked that ;) The code has a lot going on and I'm not sure which parts a relevant to this issue. I was hoping to get a general sense of what could be causing a leak in regards to FutureTask. Is there a specific part you want to see? – cottonBallPaws Feb 07 '11 at 00:26
  • @littleFluffyKitty, since you can create the queue, itself (and I do have weird queues w/ stack thread scheduling policy), you can consider it part of the `ThreadPoolExecutor` API, use it at your own will (peril). `ThreadPoolExecutor` uses the queue like that on shutdownNow() for instance. Again, your problem is not the queue. You're leaking smth somewhere else – bestsss Feb 07 '11 at 00:37
  • @littleFluffyKitty, as for code. jmap -histogram to make sure you have actually a lot of futures and not just the Runnable/Callables, you can override decorateTask to make sure the Futures are your own (and be discerned easily to the rest of FutureTask, you want to just extends FutureTask and that would be enough). Then the code you create the ThreadPool, where you call submit. – bestsss Feb 07 '11 at 00:41
  • @bestsss, looking at the histogram, it shows there are the same number of FutureTask and FutureTask$Sync instances as there are the callables still in memory. It also looks like (at least at this moment...) that the ones that get stuck in memory are the ones that are canceled. Maybe I'll try the getQueue().remove() method you mentioned and see if it still keeps them in memory. Since it seems as if the canceling is not being handled correctly. – cottonBallPaws Feb 07 '11 at 00:55
  • @littleFluffyKitty show the c-tor of that `ThreadPoolExecutor`, how do you create, what Queue you use and rest, do you have ANY thread available to process the canceled Futures, they are to be removed from the queue when their time comes, which for scheduled executor queue could be far in the time. – bestsss Feb 07 '11 at 01:24
  • @bestsss, updated the question with the constructor. I'm not sure what you mean by the rest of your last comment though. – cottonBallPaws Feb 07 '11 at 02:15
  • `PauseCancelThreadPoolExecutor`, what's that? You don't use `ThreadPoolExecutor`. Show the class implementation. Since Queue.clear() doesn't remove the references, they are kept somewhere in that mysterious class - PauseCancelThreadPoolExecutor – bestsss Feb 07 '11 at 09:14
  • @littleFluffyKitty, (I see you reverted to `ThreadPoolExecutor`), thus the final question: where do you keep `referenceToCancelLater`, you have to remove it from that structure as well. – bestsss Feb 07 '11 at 17:00
  • @bestsss, sorry for the radio silence. I have tried a couple different ways, and I always removed the reference from the array that held it. At this point I have just resorted to setting a flag in the callable that I can set to cancelled, and looping through and setting those when I want to cancel all of them. Then they just flush out of the queue themselves. It's not great but it seems to be working so far. – cottonBallPaws Feb 08 '11 at 02:18
  • 2
    This issue was filed as JDK bug 6602600 (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6602600). – Flavio Oct 14 '12 at 19:54

4 Answers4

4

According to this post, you can call purge on the executor.

gsgx
  • 12,020
  • 25
  • 98
  • 149
  • it says "Tries to remove from the work queue all Future tasks that have been cancelled. This method can be useful as a storage reclamation operation, that has no other impact on functionality. Cancelled tasks are never executed, but may accumulate in work queues until worker threads can actively remove them. Invoking this method instead tries to remove them now. However, this method may fail to remove tasks in the presence of interference by other threads." does it mean if the task is already running but falls into deadlock it won't be killed ? – HoaPhan Mar 07 '18 at 11:27
1

As a work around could you do something like:

class ClearingCallable<T> implements Callable<T> {
    Callable<T> delegate;
    ClearingCallable(Callable<T> delegate) {
        this.delegate = delegate;
    }

    T call() {
        try {
            return delegate.call();
        } finally {
            delegate = null;
        }
    }
}
Tom
  • 43,583
  • 4
  • 41
  • 61
  • What would be the delegate in this case? – cottonBallPaws Feb 07 '11 at 00:09
  • 1
    Your callable that's holding state that you want garbage collected. – Tom Feb 07 '11 at 00:10
  • this solves nothing. The problem is not the `ThreadPoolExecutor`, b/c it, alone, doesn't leak executed tasks – bestsss Feb 07 '11 at 00:16
  • It is the FutureTask that holds the Callable that I want to be garbage collected. Looking at the heap, I can't figure out why it isn't being collected. – cottonBallPaws Feb 07 '11 at 00:18
  • 1
    So the problem is that your returned object is being kept hold of? So An alternative papering over the cracks would be just return say a WeakReference and call clear on it afterwards. You're still leaking the small object of course. – Tom Feb 07 '11 at 06:48
1

I couldn't get anything to work so I came up with the following solution. Here is a rough overview: I created an array in the ThreadPoolExecutor that kept track of the runnables that were in the queue. Then when I needed to cancel the queue, I looped through and called a cancel method on each of the runnables. I my case, all of these runnables were a custom class I created and their cancel method simply set a cancelled flag. When the queue brought up the next one to process, in the run of the runnable it would see it was cancelled and skip the actual work.

So all of the runnables then just get flushed out quickly one by one as it sees it was cancelled.

Probably not the greatest solution, but it works for me and it doesn't leak memory.

cottonBallPaws
  • 21,220
  • 37
  • 123
  • 171
  • Hi, I think I am having the same issue you had. Did you figure out how to solve the problem in a cleaner ;) way? Actually, there should exist some Java classes that take care of the deletion of those expired threads. Cheers – Simone Jun 13 '12 at 06:57
  • @Simone unfortunately no, I couldn't find something more built in. I'm still using the above method. It works just fine though, just took some effort to get setup. – cottonBallPaws Jun 13 '12 at 16:22
  • Thanks for the reply. Then, does the cancel method (Runnable class) free the thread allocated memory or did you do it manually? – Simone Jun 14 '12 at 02:42
0

Refer to: https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Future.html

A Future represents the result of an asynchronous computation. If the result not be retrieved using method get then memory leak happen!

If you don't want to consume asynchronous result, using Runnable install Callable.

zhenjing
  • 1