RFR: 8236840: Memory leak when switching ButtonSkin

Kevin Rushforth kcr at openjdk.java.net
Tue Mar 24 13:28:16 UTC 2020


On Mon, 23 Mar 2020 23:40:39 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> ButtonSkin adds a `ChangeListener` to `Control.sceneProperty()` which results in leaking the `ButtonSkin` itself when
>> the `Button`'s skin is changed to a new `ButtonSkin`.  Using a `WeakChangeListener` instead of `ChangeListener` solves
>> the issue.
>> Please take a look.
>
> In general, there are two approaches to avoiding listener-related memory leaks. One is to use a WeakListener; the other
> is to explicitly remove the listener when the object is removed or otherwise no longer needed.
> Using a WeakListener is certainly easier, but runs the risk of the listener being removed too early and not cleaning up
> after itself. I'm not suggesting that's the case here, but it is worth looking at. The one thing I would ask you to
> take a look at is whether it would matter if the old skin didn't call `setDefaultButton(oldScene, false)` when removed
> (and similarly `setCancelButton`).

@aghaisas can you also review?

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

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


More information about the openjfx-dev mailing list