Java webapp memory leak when using ScheduledExecutorService

Question

My Tomcat 7 is reporting that there may be a memory leak in my webapp

SEVERE: The web application [/mywebapp] appears to have started a 
thread named [pool-1-thread-1] but has failed to stop it. This is 
very likely to create a  memory leak.

I have a long running task in my webapp that gets initialized when the webapp is started.

public class MyContextListener implements ServletContextListener{
Scheduler scheduler = null;

public MyContextListener(){
    scheduler = new Scheduler();
}

@Override
public void contextDestroyed(ServletContextEvent arg0) {
    scheduler.stop();
}

@Override
public void contextInitialized(ServletContextEvent arg0) {
    scheduler.start();
}

}

.. and my Scheduler.java

public class Scheduler {
private final ScheduledExecutorService fScheduler;

public Scheduler() {
    fScheduler = Executors.newScheduledThreadPool(1);
}


public void start(){
    fScheduler.scheduleWithFixedDelay(new Runnable() {

        @Override
        public void run() {
            //Perform some task
        }
    }, 1, 240, TimeUnit.MINUTES);
}

public void stop(){
    fScheduler.shutdownNow();
}

}

Even though I calling scheduler.stop(); when shutting down the server, its still reporting there could be a memory leak.

This app is deployed on jelastic.com and I find that once it is started, it runs well for around two days and then the tasks don't seem to be running. There is no exceptions or errors in the logs too.

Am I doing anything wrong here ? Is there really a potential memory leak ?

Answer 1

Calling fScheduler.shutdownNow(); is not enough:

There are no guarantees beyond best-effort attempts to stop processing actively executing tasks.

From JavaDoc.

Instead you must explicitly wait for the tasks that are currently running:

fScheduler.shutdownNow();
fScheduler.awaitTermination(10, TimeUnit.SECONDS);

Answer 2

I believe you should not call the shutdown from the Listener but from the Servlet directly.

contextDestroyed() of the listener is too late for the executor service. As stated in the javadoc All servlets and filters will have been destroyed before any ServletContextListeners are notified of context destruction.

whereas overriding the servlet destroy() should be OK as according to the javadoc This method gives the servlet an opportunity to clean up any resources that are being held (for example, memory, file handles, threads...

@Override
public void destroy(  ) {


     fScheduler.shutdownNow();
     fScheduler.awaitTermination(10, TimeUnit.SECONDS);

     super.destroy(  );
}