JDK-8293606 : Labeled: memory leak when moving the graphic to another parent
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: openjfx19
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2022-09-09
  • Updated: 2022-09-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
Relates :  
Description
similar to JDK-8293444: when re-using the a labeled's graphic somewhere else in a scenegraph (and also removing the labeled from the scenegraph), the labeled cannot be gc'ed. 

The technical reason is a listener to a property of the graphic that is installed by the skin: it's manually removed on change of the graphics property and on dispose (since JDK-8247576) of the skin - none of which happens here. 

The technical fix is to use a weak listener. 

A deeper problem might be that now we have a labeled pointing to a graphic that's no longer contained in its own hierarchy.  So if kept in the scenegraph, its skin will still react as if it were and f.i. compute the labeled's sizing based on the graphic.

My suggestion: fix the technical problem here (will do, since I overlooked the issue when fixing the leak on replacing the skin ;) and tackle the deeper problem in a follow-up.

Test for use in SkinLabeledCleanupTest, failing/passing before/after the  suggested fix

    @Test
    public void testMoveGraphic() {
        Node graphic = labeled.getGraphic();
        WeakReference<Labeled> weakRef = new WeakReference<>(labeled);
        root.getChildren().setAll(labeled);
        stage.show();
        root.getChildren().setAll(graphic);
        Toolkit.getToolkit().firePulse();
        labeled = null;
        attemptGC(weakRef);
        assertNull(weakRef.get());
    }

Note: doing the same/similar in JDK-8293444 (for ScrollPane's listener on a property of its content) is probably not enough to fix the memory leak in that case because ScrollPaneSkin dispose is not yet implemented correctly (and it's still excluded from the cross-skin tests like SkinMemoryLeakTest). Didn't dig, but suspect that there's also a deeper problem as described above.