RFR: 8319779: SystemMenu: memory leak due to listener never being removed [v16]

John Hendrikx jhendrikx at openjdk.org
Thu Jun 13 07:24:26 UTC 2024


On Thu, 9 May 2024 19:48:19 GMT, Johan Vos <jvos at openjdk.org> wrote:

>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add more type info

The new test passed with both the old and the new version (I reverted the code, aside from leaving `setMenuBindings` `protected`).  Is the test testing the right thing?

(If relevant, this was done on a Windows platform)

tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java line 287:

> 285: 
> 286:     @Test
> 287:     public void testJDK8309935() {

minor: Not sure what our policies are, but I find test names like this pretty useless, perhaps a short indication what this is doing?

tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java line 346:

> 344:         });
> 345:         Util.runAndWait(() -> {
> 346:         });

I checked the code that `runAndWait` would do when you pass in 0 runnables, and it basically does no waiting at all (less than a microsecond for sure).  If this is really essential (I removed it and the test still passed) then I think a `sleep` might be better.

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

Changes requested by jhendrikx (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1283#pullrequestreview-2114917800
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1637684395
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1637687030


More information about the openjfx-dev mailing list