Hi Jeanette,
Thanks for spotting additional memory leaks as part of code review.
I have filed JDK-8244081 <https://bugs.openjdk.java.net/browse/JDK-8244081>
& JDK-8244075 <https://bugs.openjdk.java.net/browse/JDK-8244075> to address
them.
Regards,
Ajit
> On 27-Apr-2020, at 8:17 PM, Jeanette Winzenburg <[email protected]>
> wrote:
>
> On Mon, 27 Apr 2020 11:57:58 GMT, Ajit Ghaisas <[email protected]> wrote:
>
>> Issue : https://bugs.openjdk.java.net/browse/JDK-8175358
>>
>> Root cause : When a MenuItem is removed from a Scene, if any accelerator has
>> been set on MenuItem, it does not get
>> removed from Scene's list of accelerators.
>> Fix : If Scene changes for a Menu, remove the registered accelerators from
>> Scene.
>>
>> Testing : Added a unit test that fails before the fix and passes with it.
>
> fix looks good to me :)
>
> Seeing the code, I think we have two follow-up issues (not introduced here,
> they just jump to visibility in the light
> of reading the code and recent fixes around memory leaks :)
>
> - the listener to the control's sceneProperty is never removed, introducing a
> memory leak, a fix could be similar to that
> of the recently [fixed
> JDK-8236840](https://bugs.openjdk.java.net/browse/JDK-8236840)
>
> - we have the exact same issue with accelerators in a contextMenu of a
> control that's removed from the scene (the
> accelerators are still active, as can be seen in adapting your test method).
> Not sure which collaborator should be
> responsible for the cleanup, could be the helper class, the control, or ?
>
> -------------
>
> PR: https://git.openjdk.java.net/jfx/pull/199
