On Fri, 25 Sep 2020 18:29:35 GMT, yosbits 
<github.com+7517141+yos...@openjdk.org> wrote:

>> beware: all listeners that are manually added by the skin (as opposed to 
>> those added via skin.registerXX) must be
>> manually removed again! Also there must be tests added to guard against 
>> memory leaks and side-effects when switching
>> the skin. For guidance, see 
>> [JDK-8241364](https://bugs.openjdk.java.net/browse/JDK-8241364) and related 
>> (fixed and
>> open) issues.
>
> @kleopatra
> It's a very good point, but the original source code seems to have a memory 
> leak problem due to an incomplete
> implementation of dispose (). Checking with the profile tool, it looks like 
> you need a cleanup of over 20 references. I
> think this is a problem that should be fixed in another issue.

agreed that cleanup of the existing leaks (and misbehavior due to missing 
listener removal is off scope for this :) -
but you must not add new leaks/side-effects in this fix: guarding this change 
against additional misbehavior definitely
is in scope, IMO.

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

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

Reply via email to