Scheduled Futures leak resources from Managed Executor Services on application stop

[colvinco]

Describe the bug
The futures queue in ManagedScheduledExecutorServiceImpl holds references to scheduled futures, even once they have completed.

The queue is periodically cleaned when new tasks are scheduled, by the private purgeFutures() method, but otherwise they are not actively removed, and it isn't called when applications are shutdown. As purgeFutures is private, applications can't call it themselves.

There's also a deactivate method that aggressively cleans up when the server is shutdown.

Our application does a lot of work via scheduled tasks, and in a test/dev environment we want to stop and start the application without restarting the server. We do have other memory leaks that we're tracking down, but this one is making it harder to see what else is going on.

Steps to Reproduce

1. Create a web.xml with a context listener and a scheduled executor service

<?xml version="1.0" encoding="UTF-8"?><web-app xmlns="http://java.sun.com/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_0.xsd" id="WebApp_ID" version="3.0">
    <listener>
        <listener-class>com.example.FooContextListener</listener-class>
    </listener>

    <resource-env-ref>
        <resource-env-ref-name>wm/fooScheduler</resource-env-ref-name>
        <resource-env-ref-type>java.util.concurrent.ScheduledExecutorService</resource-env-ref-type>
    </resource-env-ref>
</web-app>

2. Create a simple Context Listener class that will submit a number of scheduled tasks to a managed executor

package com.example;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import javax.naming.InitialContext;
import javax.naming.NamingException;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;

public class FooContextListener implements ServletContextListener {

  private ScheduledExecutorService executorService;

  @Override
  public void contextInitialized(ServletContextEvent sce) {
    try {
      executorService =
          (ScheduledExecutorService) new InitialContext().lookup("java:comp/env/wm/fooScheduler");
    } catch (NamingException ex) {
      ex.printStackTrace();
    }

    for (int i = 0; i < 30; i++) {
      final Bar bar = new Bar();
      executorService.schedule(bar, 1L, TimeUnit.MILLISECONDS);
    }

    ServletContextListener.super.contextInitialized(sce);
  }

  @Override
  public void contextDestroyed(ServletContextEvent sce) {
    ServletContextListener.super.contextDestroyed(sce);
  }

  private static class Bar implements Runnable {

    @Override
    public void run() {
      System.out.println("Hello");
    }
  }
}

3. Start Liberty, wait for the Tasks to run (print "Hello" 30 times), then stop the application (not the server). A leak of 30 references to the $Bar class is observable, e.g. in JProfiler:
   
   

You can see that the GC Root is via the futures queue in

open-liberty/dev/com.ibm.ws.concurrent/src/com/ibm/ws/concurrent/internal/ManagedScheduledExecutorServiceImpl.java

Line 63 in aa6a9f8

private final ConcurrentLinkedQueue<ScheduledFuture<?>> futures = new ConcurrentLinkedQueue<ScheduledFuture<?>>();

4. In the contextDestroyed() method insert a call to purgeFutures() with reflection

    try {
      final Method method = executorService.getClass().getDeclaredMethod("purgeFutures");
      method.setAccessible(true);
      method.invoke(executorService);
    } catch (NoSuchMethodException
        | SecurityException
        | IllegalAccessException
        | IllegalArgumentException
        | InvocationTargetException ex) {
      ex.printStackTrace();
    }

5. Repeat the test and there is now no leak:
   

Expected behavior
Resources should be released automatically when an application stops, as managed executors cannot be shutdown by application code, or there needs to be an explicit method to clean up. Waiting for 20 new tasks to be scheduled is unreliable.

Diagnostic information:

• OpenLiberty Version: I'm using 22.0.0.12 currently, but AFAIK it affects all recent versions

[njr-11]

This seems like a reasonable change to make. One way for Liberty to accomplish this automatically would be to use the com.ibm.ws.container.service.metadata.ApplicationMetaDataListener.applicationMetaDataDestroyed notification to trigger purgeFutures().