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