-
Bug
-
Resolution: Not an Issue
-
P4
-
8, 20
-
generic
-
generic
A DESCRIPTION OF THE PROBLEM :
SwingWorker.getWorkersExecutorService() creates an ExecutorService and stores a reference to the ExecutorService in the AppContext map. When the AppContext is disposed, it removes the reference to the ExecutorService from the map, and a listener defined in getWorkersExecutorService() shuts down the ExecutorService. But the listener retains a strong reference to the ExecutorService, and the AppContext retains a strong reference to the listener, which is a resource leak.
In https://github.com/openjdk/jdk/commit/b8af3d50192f8bc98d83f8102f0fd1989f302e32 a weak reference was accidentally changed from a field to a local variable, which means that the listener keeps a strong reference to the ExecutorService.
STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
See https://github.com/jcsahnwaldt/SwingWorkerExecutorLeakTest/blob/master/src/main/java/SwingWorkerExecutorLeakTest.java
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
The test should run without an exception.
ACTUAL -
The test throws an exception.
---------- BEGIN SOURCE ----------
// compile and run with --add-exports=java.desktop/sun.awt=ALL-UNNAMED
import sun.awt.SunToolkit;
import sun.awt.AppContext;
import java.awt.Toolkit;
import javax.swing.SwingWorker;
import java.lang.ref.WeakReference;
import java.util.concurrent.ExecutorService;
import java.beans.PropertyChangeListener;
public class SwingWorkerExecutorLeakTest {
private static AppContext appContext;
public static void main(String[] args) throws Exception {
// AppContext must be in new thread group, otherwise dispose() throws
Thread thread = new Thread(new ThreadGroup("Test"), "Test") {
public void run() {
appContext = ((SunToolkit)Toolkit.getDefaultToolkit()).createNewAppContext();
new SwingWorker<Void, Void>() {
protected Void doInBackground() {
return null;
}
}.execute(); // calls SwingWorker.getWorkersExecutorService()
}
};
thread.start();
thread.join();
// SwingWorker.getWorkersExecutorService() stored the executor in the AppContext map
WeakReference<ExecutorService> executor = new WeakReference<>((ExecutorService)appContext.get(SwingWorker.class));
appContext.dispose();
// dispose() cleared the AppContext map
if (appContext.get(SwingWorker.class) != null) throw new AssertionError();
// dispose() called the listener defined in SwingWorker.getWorkersExecutorService(),
// which called shutdown() on the executor
if (! executor.get().isShutdown()) throw new AssertionError();
// but the listener retains a strong reference to the executor,
// and dispose() doesn't clear the list of listeners
// reference chain: appContext -> listener -> executor
if (false) {
// this would make the executor unreachable
appContext = null;
}
if (false) {
// this would make the executor unreachable
PropertyChangeListener listener = appContext.getPropertyChangeListeners(AppContext.DISPOSED_PROPERTY_NAME)[0];
appContext.removePropertyChangeListener(AppContext.DISPOSED_PROPERTY_NAME, listener);
listener = null;
}
System.gc();
if (executor.get() != null) {
throw new RuntimeException("executor created by SwingWorker.getWorkersExecutorService() is still strongly referenced");
}
}
}
---------- END SOURCE ----------
FREQUENCY : always
SwingWorker.getWorkersExecutorService() creates an ExecutorService and stores a reference to the ExecutorService in the AppContext map. When the AppContext is disposed, it removes the reference to the ExecutorService from the map, and a listener defined in getWorkersExecutorService() shuts down the ExecutorService. But the listener retains a strong reference to the ExecutorService, and the AppContext retains a strong reference to the listener, which is a resource leak.
In https://github.com/openjdk/jdk/commit/b8af3d50192f8bc98d83f8102f0fd1989f302e32 a weak reference was accidentally changed from a field to a local variable, which means that the listener keeps a strong reference to the ExecutorService.
STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
See https://github.com/jcsahnwaldt/SwingWorkerExecutorLeakTest/blob/master/src/main/java/SwingWorkerExecutorLeakTest.java
EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
The test should run without an exception.
ACTUAL -
The test throws an exception.
---------- BEGIN SOURCE ----------
// compile and run with --add-exports=java.desktop/sun.awt=ALL-UNNAMED
import sun.awt.SunToolkit;
import sun.awt.AppContext;
import java.awt.Toolkit;
import javax.swing.SwingWorker;
import java.lang.ref.WeakReference;
import java.util.concurrent.ExecutorService;
import java.beans.PropertyChangeListener;
public class SwingWorkerExecutorLeakTest {
private static AppContext appContext;
public static void main(String[] args) throws Exception {
// AppContext must be in new thread group, otherwise dispose() throws
Thread thread = new Thread(new ThreadGroup("Test"), "Test") {
public void run() {
appContext = ((SunToolkit)Toolkit.getDefaultToolkit()).createNewAppContext();
new SwingWorker<Void, Void>() {
protected Void doInBackground() {
return null;
}
}.execute(); // calls SwingWorker.getWorkersExecutorService()
}
};
thread.start();
thread.join();
// SwingWorker.getWorkersExecutorService() stored the executor in the AppContext map
WeakReference<ExecutorService> executor = new WeakReference<>((ExecutorService)appContext.get(SwingWorker.class));
appContext.dispose();
// dispose() cleared the AppContext map
if (appContext.get(SwingWorker.class) != null) throw new AssertionError();
// dispose() called the listener defined in SwingWorker.getWorkersExecutorService(),
// which called shutdown() on the executor
if (! executor.get().isShutdown()) throw new AssertionError();
// but the listener retains a strong reference to the executor,
// and dispose() doesn't clear the list of listeners
// reference chain: appContext -> listener -> executor
if (false) {
// this would make the executor unreachable
appContext = null;
}
if (false) {
// this would make the executor unreachable
PropertyChangeListener listener = appContext.getPropertyChangeListeners(AppContext.DISPOSED_PROPERTY_NAME)[0];
appContext.removePropertyChangeListener(AppContext.DISPOSED_PROPERTY_NAME, listener);
listener = null;
}
System.gc();
if (executor.get() != null) {
throw new RuntimeException("executor created by SwingWorker.getWorkersExecutorService() is still strongly referenced");
}
}
}
---------- END SOURCE ----------
FREQUENCY : always
- links to
-
Review openjdk/jdk/15081