Content of SwingNode not garbage collected when content changed

Viewed 286

I have a JavaFX application that displays a Swing plot via a SwingNode. Due to the way that the swing component is written and my unwillingness to refactor it, I create a new instance of the swing plot each time the user needs to update the data. In other words, whenever the user re-generates the plot, it creates a new Swing component and sets the SwingNode's content to the new component.

It all works fine except that I discovered that the swing components never get garbage collected. They contain a substantial amount of data, so after a while it becomes a pretty severe memory leak.

I've been able to demonstrate the issue with this minimum reproducible example:

public class LeakDemo extends Application {

    //Keep week references to all panels that we've ever generated to see if any
    //of them get collected.
    private Collection<WeakReference<JPanel>> panels = new CopyOnWriteArrayList<>();
    
    @Override
    public void start(Stage primaryStage) throws Exception {
        
        SwingNode node = new SwingNode();
        
        Pane root = new Pane();
        root.getChildren().add(node);
        
        //Kick off a thread that repeatedly creates new JPanels and resets the swing node's content
        new Thread(() -> {
            
            while(true) {
                
                //Lets throw in a little sleep so we can read the output
                try {
                    Thread.sleep(500);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                
                SwingUtilities.invokeLater(() -> {
                    JPanel panel = new JPanel();
                    panels.add(new WeakReference<>(panel));
                    node.setContent(panel);
                });
                
                System.out.println("Panels in memory: " + panels.stream().filter(ref -> ref.get() != null).count());
                
                //I know this doesn't guarantee anything, but prompting a GC gives me more confidence that this
                //truly is a bug.
                System.gc();
            }
            
        }).start();
        
        primaryStage.setScene(new Scene(root, 100, 100));
        
        primaryStage.show();
        
    }
    
    public static void main(String[] args) {
        launch(args);
    }
}

The output of the program is

Panels in memory: 0
Panels in memory: 1
Panels in memory: 2
Panels in memory: 3
Panels in memory: 4
Panels in memory: 5
Panels in memory: 6
Panels in memory: 7
Panels in memory: 8
Panels in memory: 9
Panels in memory: 10

and will continue like that into the thousands.

I tried inspecting a heap dump from jvisualvm, but got pretty lost in the sea of references.

I'm suspicious that this is an issue in JavaFX, but I thought I'd check here before I report it as a bug.

3 Answers

OK, I figured it out.

Short answer

Just wrap the swing content inside a JPanel (or some other JComponent). Then only ever call SwingNode.setContent() once to add the wrapper. When you need to update the swing content, call removeAll() on your wrapper and then add() with the appropriate content.

Long Answer

Thanks to the suggestion in this answer: https://stackoverflow.com/a/66283491/2423283 I was able to determine that the leak is caused by GlassStage which is a non-api class which among other things, keeps a static list of all implementations of GlassStage. The content of a SwingNode gets managed by an instance of EmbeddedScene which is a subtype of GlassStage.

Items are removed from the static list when close() is called on them. SwingNode.setContent() does not close any pre-existing content, but Container.removeAll() does.

Working code

Here's an example of the fixed code:

public class LeakDemoFixed extends Application {

    //Keep week references to all panels that we've ever generated to see if any
    //of them get collected.
    private Collection<WeakReference<JPanel>> panels = new CopyOnWriteArrayList<>();
    
    @Override
    public void start(Stage primaryStage) throws Exception {
        
        SwingNode node = new SwingNode();
        
        //These 2 lines were added
        JComponent swingContent = new JPanel();
        node.setContent(swingContent);
        
        Pane root = new Pane();
        root.getChildren().add(node);
        
        //Kick off a thread that repeatedly creates new JPanels and resets the swing node's content
        new Thread(() -> {
            
            while(true) {
                
                //Lets throw in a little sleep so we can read the output
                try {
                    Thread.sleep(500);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                
                SwingUtilities.invokeLater(() -> {
                    JPanel panel = new JPanel();
                    panels.add(new WeakReference<>(panel));
                    //Removed the line below
                    //node.setContent(panel);
                    
                    //Added these 2 lines
                    swingContent.removeAll();
                    swingContent.add(panel);
                });
                
                System.out.println("Panels in memory: " + panels.stream().filter(ref -> ref.get() != null).count());
                
                //I know this doesn't guarantee anything, but prompting a GC gives me more confidence that this
                //truly is a bug.
                System.gc();
            }
            
        }).start();
        
        primaryStage.setScene(new Scene(root, 100, 100));
        
        primaryStage.show();
        
    }
    
    public static void main(String[] args) {
        launch(args);
    }
}

I didn't reached this question before but here is what I've done to correct the issue. I would like this fix to be integrated into JavaFX for Java8 but I don't know how to get this done and they closed the issue (because of duplicate)

https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8284352

The problem is due to the JLightweightFrame never being released by de SwingNodeDisposer.

The SwingNodeDisposer holds an hard pointer to the JLightweightFrame that prevents its suppression.

I have modified the SwingNode.setContentImpl() function to use a WeakReference instead of an hard pointer and now memory is properly released.

...

    WeakReference<JLightweightFrame> lwFramePtr = new WeakReference<JLightweightFrame>(lwFrame);
    SwingNodeDisposer disposeRec = new SwingNodeDisposer(lwFramePtr);
    Disposer.addRecord(this, disposeRec);

...

This gives good results without having the need to use a single JPanel or to encapsulate it inside a WeakReference.

Related