On Mon, 23 Mar 2020 07:26:46 GMT, Ambarish Rapte <ara...@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`). ------------- PR: https://git.openjdk.java.net/jfx/pull/147
