On Thu, 29 Sep 2022 23:00:17 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

> Fixed memory leak by using weak listeners and making sure to undo everything 
> that has been done to MenuBar/Skin in dispose().

modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java
 line 328:

> 326:         mouseEventHandler = t -> {
> 327:             if (getSkinnable() != null) {
> 328:                 Bounds containerScreenBounds = 
> container.localToScreen(container.getLayoutBounds());

this null check is wrong: skinnable is guaranteed to never be null - except 
after dispose, at which time it's illegal to access any of the skin's 
methods/fields/state. So checking here is smearing over a precondition 
violation of the caller (most probably a dangling handler/listener) which has 
to be fixed.

Note we deliberately removed all null checks in all (cleaned) skins :) If we 
see any exceptions, the procedure is to 

- write a test exposing the exception (it's failing), this test belongs into 
SkinCleanTest
- find and fix the violator
- see the test passing

There might be other exceptions as well (like IOOB or similar) which require a 
similar procedure, see the available tests. Actually, I think it's a good idea 
to actively look out for possible macroscopic misbehavior if handlers/listeners 
are not removed properly. SkinCleanupTest has examples.

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

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

Reply via email to