RFR: 8208088: Memory Leak in ControlAcceleratorSupport [v6]

[RationalityFrontline]

On Mon, 12 Apr 2021 08:40:56 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> The method `ControlAcceleratorSupport.doAcceleratorInstall(final List<? extends MenuItem> items, final Scene scene)` adds a `ChangeListener` on `MenuItem.acceleratorProperty()`. This listener is not removed when the MenuItem is removed from scenegraph.
>> Adding and removing a MenuItem results in multiple copies of the listener added to MenuItem.acceleratorProperty().
>> 
>> Fix is to remove the listener when MenuItem is removed.
>> Fix can be verified by checking the number of instances of `ControlAcceleratorSupport.lambda` using _jvisualvm_. 
>> Without this fix, the number of `ControlAcceleratorSupport.lambda` increase in multiple of number of MenuItems being removed and added back.
>> With fix, the count is always same as number of MenuItems in scenegraph.
>> 
>> Also there is another ListChangeListener added to a `ObservableList<MenuItem> items` in the method `ControlAcceleratorSupport.doAcceleratorInstall(final ObservableList<MenuItem> items, final Scene scene)`. There was a TODO note to remove this listener.
>> This listener is added on `MenuBarButton.getItems()` and not on `Menu.getItems()`.  This `MenuBarButton` is created by `MenuBarSkin` to show a `Menu`. This `MenuBarButton` gets disposed when the related `Menu` is removed from scenegraph, and so the added `ListChangeListener` gets GCed. Hence it is not required to explicitly remove the listener. 
>> Added a comment explaining this behavior in place of the TODO.
>
> Ambarish Rapte has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove indirect test

After updating to jfx 17, I detected memory leak in my application (every controller that has menu items won't get garbage collected after closing its stage), with visualvm I found it was caused by class ControlAcceleratorSupport. This kind of memory leak doesn't happen on jfx 16, so I guess there is something wrong with this PR.

I've created a sample project that could reproduce and verify the memory leak: [jfx-test](https://github.com/RationalityFrontline/jfx-test/tree/ControlAcceleratorSupport) (on git branch ControlAcceleratorSupport).

Run command `gradlew run` and you should see the following ui interface:
![image](https://user-images.githubusercontent.com/69410606/132838128-ac5a3883-2170-441f-b926-37bde77751b0.png)

Clicking button `Call GC and Print MenuItems` will call `System.gc()` and print current ungarbaged menu items to console, clicking menu `Restart Stage` will call `Stage.close()` and launch a new same Stage. 

After clicking `Restart Stage` several times, click `Call GC and Print MenuItems`, you will see lots of ungarbaged menu items. However, by changing jfx version to 16 in `build.gradle.kts`, you will always see only one menu item.

If menu items are set with action listeners, then these listeners also won't be garbage collected, typically these listeners hold references to controllers, which made all closed controllers leaked. This made jfx 17 unusable.

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

PR: https://git.openjdk.java.net/jfx/pull/429