On Sun, 30 Jul 2023 12:50:58 GMT, Christopher Sahnwaldt <d...@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
