Can a scheduled future cause a memory leak?

Question

I think I have a memory leak in my Android live wallpaper. Whenever I rotate the screen, the amount of memory garbage collected increases by 50kb and doesn't go back down. I think it may be caused by a scheduled future, so I'm going to present a scenario to see if that's the case.

Let's say you have a class (let's call it Foo) that has the following members.

private ScheduledFuture<?> future;
private final ScheduledExecutorService scheduler = Executors
        .newSingleThreadScheduledExecutor();

private final Runnable runnable = new Runnable() {
    public void run() {
        // Do stuff
    }
};

And now you set a scheduled future

future = scheduler.scheduleAtFixedRate(runnable, delay, speed,
                TimeUnit.MILLISECONDS);

The future holds a reference to the runnable, and the runnable holds a reference to the parent Foo object. I'm not sure if this is the case, but could this fact mean that if nothing in the program holds a reference to Foo, the garbage collector still cannot collect it because there is a scheduled future? I'm not too good at multithreading, so I don't know if the code I've shown means the scheduled task will live longer than the object, meaning it won't end up being garbage collected.

If this scenario will not result in preventing Foo from being garbage collection, I just need to be told that with a simple explanation. If it does prevent Foo from being garbage collected, then how do I fix it? Do have to do future.cancel(true); future = null;? Is the future = null part unnecessary?

Answer 1

Although this question is answered long back but after reading this article I thought of posting new answer with explanation.

Can a scheduled future cause a memory leak? --- YES

ScheduledFuture.cancel() or Future.cancel() in general does not notify its Executor that it has been cancelled and it stays in the Queue until its time for execution has arrived. It's not a big deal for simple Futures but can be a big problem for ScheduledFutures. It can stay there for seconds, minutes, hours, days, weeks, years or almost indefinitely depending on the delays it has been scheduled with.

Here is an example with the worst case scenario. The runnable and everything it's referencing will stay in the Queue for Long.MAX_VALUE milliseconds even after its Future has been cancelled!

public static void main(String[] args) {
    ScheduledThreadPoolExecutor executor 
        = new ScheduledThreadPoolExecutor(1);

    Runnable task = new Runnable() {
        @Override
        public void run() {
            System.out.println("Hello World!");
        }
    };

    ScheduledFuture future 
        = executor.schedule(task, 
            Long.MAX_VALUE, TimeUnit.MILLISECONDS);

    future.cancel(true);
}

You can see this by using a Profiler or calling the ScheduledThreadPoolExecutor.shutdownNow() method which will return a List with one element in it (it's the Runnable that got cancelled).

The solution for this problem is to either write your own Future implementation or to call the purge() method every now and then. In case of custom Executor factory solution is:

public static ScheduledThreadPoolExecutor createSingleScheduledExecutor() {
    final ScheduledThreadPoolExecutor executor 
        = new ScheduledThreadPoolExecutor(1);

    Runnable task = new Runnable() {
        @Override
        public void run() {
            executor.purge();
        }
    };

    executor.scheduleWithFixedDelay(task, 30L, 30L, TimeUnit.SECONDS);

    return executor;
}

Answer 2

The future holds a reference to the runnable, and the runnable holds a reference to the parent Foo object. I'm not sure if this is the case, but could this fact mean that if nothing in the program holds a reference to Foo, the garbage collector still cannot collect it because there is a scheduled future?

Its a bad idea to make Foo some sort of transient object that you create often because you should shutdown the ScheduledExecutorService scheduler when your app closes. Thus you should make Foo a pseudo-singleton.

The Garbage collector understands cycles so once you shutdown Foo's executor service you will most likely not have memory issues.

Answer 3

• Either your run method relies on the enclosing Foo class and therefore can't live independently. In that case I don't see how you could have your Foo gc'ed and keep your runnable "alive" to be run by the executor
• or your run method is static in the sense that it does not depend on the state of your Foo class, in which case you could make it static and it will prevent the problem you are experiencing.

You don't seem to handle interruption in your Runnable. That means that even if you call future.cancel(true) your Runnable will continue to run, which as you determined could be a cause for your leak.

There are several ways to make a Runnable "interrupt-friendly". Either you call a method that throws InterruptedException (like Thread.sleep() or a blocking IO method) and it will throw an InterruptedException when the future is cancelled. You can catch that exception and exit the run method promptly after having cleaned up what needs to be cleaned up and restoring the interrupted state:

public void run() {
    while(true) {
        try {
            someOperationThatCanBeInterrupted();
        } catch (InterruptedException e) {
            cleanup(); //close files, network connections etc.
            Thread.currentThread().interrupt(); //restore interrupted status
        }
    }
}    

If you don't call any such methods, the standard idiom is:

public void run() {
    while(!Thread.currentThread().isInterrupted()) {
        doYourStuff();
    }
    cleanup();
}

In that case, you should try to make sure that the condition in the while is checked regularly.

With those changes, when you call future.cancel(true), an interrupt signal will be sent to the thread executing your Runnable which will exit what it is doing, making your Runnable and your Foo instance eligible for GC.