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

[Andy Goryachev]

On Tue, 9 Jan 2024 13:19:53 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 two additional commits since the last revision:
> 
>  - Cleanup test
>  - Add shim class so that we can access the references to com.sun.glass.ui.Menu instances.
>    Add a test to make sure those references are gone.

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 98:

> 96:             active.set(false);
> 97:         }
> 98:         active = new SimpleBooleanProperty(true);

should this be

if(active == null) {
  active = new ...
} else {
 active.set(false);
}

instead?

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 224:

> 222:         mb.textProperty().when(active).addListener(valueModel -> glassMenu.setTitle(parseText(mb)));
> 223:         mb.disableProperty().when(active).addListener(valueModel -> glassMenu.setEnabled(!mb.isDisable()));
> 224:         mb.mnemonicParsingProperty().when(active).addListener(valueModel -> glassMenu.setTitle(parseText(mb)));

is `active` guaranteed to be non-null at this point (and also on L279)?

assuming the code on L98 is fixed and does not clobber the old pointer.

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

> 225:                 if (!JMemoryBuddy.checkCollectable(wr)) {
> 226:                     strongCount++;
> 227:                     assertTrue("Too much refs", strongCount < 2);

"too many" ?

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

> 252:         return menuBar;
> 253:     }
> 254: 

extra newline?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446313415
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446323094
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446325052
PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1446327877