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

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