RFR: 8314755: Resource leak: SwingWorker listener keeps strong reference to executor

Christopher Sahnwaldt duke at openjdk.org
Fri Sep 8 15:28:03 UTC 2023


On Sun, 30 Jul 2023 12:50:58 GMT, Christopher Sahnwaldt <duke at openjdk.org> wrote:

> In https://github.com/openjdk/jdk/commit/b8af3d50192f8bc98d83f8102f0fd1989f302e32 the weak reference was accidentally changed from a field to a local variable, which means that the PropertyChangeListener keeps a strong reference to executorService, which is a resource leak

I wrote a test demonstrating the problem that the executor is retained: [SwingWorkerExecutorLeakTest.java](/jcsahnwaldt/SwingWorkerExecutorLeakTest/blob/master/src/main/java/SwingWorkerExecutorLeakTest.java). (It's slightly similar to [6799345/TestShutdown.java](/openjdk/jdk/blob/master/test/jdk/javax/swing/system/6799345/TestShutdown.java).)

I ran the test on JDK 8, 11, 17, and 20 on macOS and always got the same result.

Maybe an even cleaner way to break the reference chain appContext -> listener -> executor would be to remove the listener from the appContext. There's a bit of code in the test demonstrating that it would fix the problem.

In `SwingWorker`, we could simply add the line `appContext.removePropertyChangeListener(AppContext.DISPOSED_PROPERTY_NAME, this)` somewhere in the listener. We could then do away with the weak reference, since the strong reference from the listener to the executor wouldn't cause a resource leak anymore.

I'd be happy top update this PR accordingly. Let me know what you think.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15081#issuecomment-1684661097
PR Comment: https://git.openjdk.org/jdk/pull/15081#issuecomment-1684671932


More information about the client-libs-dev mailing list