[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