RFR: 8294589: MenuBarSkin: memory leak when changing skin [v15]

[Andy Goryachev]

On Thu, 1 Dec 2022 17:48:59 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> You are right: some weak listeners remain, I did not want to re-write the whole thing for a fear of introducing regression and to keep the changes to a minimum.
>> 
>> The second `ListenerHelper` (line 293) gets disconnected in dispose(), or when the skin is collected (since the skin may not be explicitly uninstalled, but instead the whole component or `Pane` might be removed from the scene and discarded.
>
>> but instead the whole component or Pane might be removed from the scene and discarded.
> 
> If that happens, weak reference won't make a difference there either. Removing an entire branch from the Scene and not referring to it anymore will do correct clean-up even without a call to dispose and without the use of weak references.
> 
> The only use case I see is that we still don't trust the Skin lifecycle to be adhered to and "just in case" are using weak references in case somehow dispose is not called. I would much prefer to see an actual reason to use it, and then adding a comment as to why this weak reference is needed so that in 6 months time we're not scratching our heads as to why a weak reference is needed here.

just to be sure, which weak listener are you referring to?

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

PR: https://git.openjdk.org/jfx/pull/906