[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: RFR: 8175358: Memory leak when moving MenuButton into another Scene
From: Ajit Ghaisas <ajit.ghaisas () oracle ! com>
Date: 2020-04-29 7:31:52
Message-ID: A1A39292-999D-49E8-A426-23EB499FFEE8 () oracle ! com
[Download RAW message or body]
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 <fastegal@openjdk.java.net> wrote:
>
> On Mon, 27 Apr 2020 11:57:58 GMT, Ajit Ghaisas <aghaisas@openjdk.org> 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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic