8189280: Memory leak in SwingNode if Stage is not shown

Reviewed-by: kcr

4 files changed, 443 insertions, 39 deletions

diff --git a/modules/swing/src/main/java/com/sun/javafx/embed/swing/Disposer.java b/modules/swing/src/main/java/com/sun/javafx/embed/swing/Disposer.java
new file mode 100644
index 000000000..c56f3a133
--- /dev/null
+++ b/modules/swing/src/main/java/com/sun/javafx/embed/swing/Disposer.java
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package com.sun.javafx.embed.swing;
+
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
+import java.util.Hashtable;
+
+/**
+ * This class is used for registering and disposing the native
+ * data associated with java objects.
+ *
+ * The object can register itself by calling the addRecord method and
+ * providing a descendant of the DisposerRecord class with overridden
+ * dispose() method.
+ *
+ * When the object becomes unreachable, the dispose() method
+ * of the associated DisposerRecord object will be called.
+ *
+ * @see DisposerRecord
+ */
+public class Disposer implements Runnable {
+    private static final ReferenceQueue queue = new ReferenceQueue();
+    private static final Hashtable records = new Hashtable();
+    private static Disposer disposerInstance;
+
+    static {
+        disposerInstance = new Disposer();
+
+        ThreadGroup tg = Thread.currentThread().getThreadGroup();
+        java.security.AccessController.doPrivileged(
+            new java.security.PrivilegedAction() {
+                public Object run() {
+                    /* The thread must be a member of a thread group
+                     * which will not get GCed before VM exit.
+                     * Make its parent the top-level thread group.
+                     */
+                    ThreadGroup tg = Thread.currentThread().getThreadGroup();
+                    for (ThreadGroup tgn = tg;
+                         tgn != null;
+                         tg = tgn, tgn = tg.getParent());
+                    Thread t =
+                        new Thread(tg, disposerInstance, "SwingNode Disposer");
+                    t.setContextClassLoader(null);
+                    t.setDaemon(true);
+                    t.setPriority(Thread.MAX_PRIORITY);
+                    t.start();
+                    return null;
+                }
+            }
+        );
+    }
+
+    /**
+     * Registers the object and the native data for later disposal.
+     * @param target Object to be registered
+     * @param rec the associated DisposerRecord object
+     * @see DisposerRecord
+     */
+    public static WeakReference addRecord(Object target, DisposerRecord rec) {
+        WeakReference ref = new WeakReference(target, queue);
+        disposerInstance.records.put(ref, rec);
+        return ref;
+    }
+
+    public void run() {
+        while (true) {
+            try {
+                Object obj = queue.remove();
+                ((Reference)obj).clear();
+                DisposerRecord rec = (DisposerRecord)records.remove(obj);
+                rec.dispose();
+                obj = null;
+                rec = null;
+            } catch (Exception e) {
+                System.out.println("Exception while removing reference: " + e);
+                e.printStackTrace();
+            }
+        }
+    }
+}
diff --git a/modules/swing/src/main/java/com/sun/javafx/embed/swing/DisposerRecord.java b/modules/swing/src/main/java/com/sun/javafx/embed/swing/DisposerRecord.java
new file mode 100644
index 000000000..14a27fcb5
--- /dev/null
+++ b/modules/swing/src/main/java/com/sun/javafx/embed/swing/DisposerRecord.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package com.sun.javafx.embed.swing;
+
+/**
+ * This class is used to hold the resource to be disposed.
+ */
+public interface DisposerRecord {
+    public void dispose();
+}
diff --git a/modules/swing/src/main/java/javafx/embed/swing/SwingNode.java b/modules/swing/src/main/java/javafx/embed/swing/SwingNode.java
index 1c45e9103..218848993 100644
--- a/modules/swing/src/main/java/javafx/embed/swing/SwingNode.java
+++ b/modules/swing/src/main/java/javafx/embed/swing/SwingNode.java
@@ -47,6 +47,7 @@ import java.awt.Component;
 import java.awt.Cursor;
 import java.awt.EventQueue;
 import java.awt.Toolkit;
+import java.lang.ref.WeakReference;
 import java.awt.dnd.DragGestureEvent;
 import java.awt.dnd.DragGestureListener;
 import java.awt.dnd.DragGestureRecognizer;
@@ -68,6 +69,8 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.locks.ReentrantLock;
+import com.sun.javafx.embed.swing.Disposer;
+import com.sun.javafx.embed.swing.DisposerRecord;
 import com.sun.javafx.geom.BaseBounds;
 import com.sun.javafx.geom.transform.BaseTransform;
 import com.sun.javafx.jmx.MXNodeAlgorithm;
@@ -254,25 +257,17 @@ public class SwingNode extends Node {
         if (content != null) {
             lwFrame = new JLightweightFrame();
 
-            lwFrame.addWindowFocusListener(new WindowFocusListener() {
-                @Override
-                public void windowGainedFocus(WindowEvent e) {
-                    SwingFXUtils.runOnFxThread(() -> {
-                        SwingNode.this.requestFocus();
-                    });
-                }
-                @Override
-                public void windowLostFocus(WindowEvent e) {
-                    SwingFXUtils.runOnFxThread(() -> {
-                        ungrabFocus(true);
-                    });
-                }
-            });
+            SwingNodeWindowFocusListener snfListener =
+                                 new SwingNodeWindowFocusListener(this);
+            lwFrame.addWindowFocusListener(snfListener);
 
             jlfNotifyDisplayChanged.invoke(lwFrame, scale);
-            lwFrame.setContent(new SwingNodeContent(content));
+            lwFrame.setContent(new SwingNodeContent(content, this));
             lwFrame.setVisible(true);
 
+            SwingNodeDisposer disposeRec = new SwingNodeDisposer(lwFrame);
+            Disposer.addRecord(this, disposeRec);
+
             SwingFXUtils.runOnFxThread(() -> {
                 locateLwFrame(); // initialize location
 
@@ -688,12 +683,56 @@ public class SwingNode extends Node {
         return alg.processLeafNode(this, ctx);
     }
 
-    private class SwingNodeContent implements LightweightContent {
+    private static class SwingNodeDisposer implements DisposerRecord {
+         JLightweightFrame lwFrame;
+
+         SwingNodeDisposer(JLightweightFrame ref) {
+             this.lwFrame = ref;
+         }
+         public void dispose() {
+             if (lwFrame != null) {
+                 lwFrame.dispose();
+                 lwFrame = null;
+             }
+         }
+    }
+
+    private static class SwingNodeWindowFocusListener implements WindowFocusListener {
+        private WeakReference<SwingNode> swingNodeRef;
+
+        SwingNodeWindowFocusListener(SwingNode swingNode) {
+            this.swingNodeRef = new WeakReference<SwingNode>(swingNode);
+        }
+
+        @Override
+        public void windowGainedFocus(WindowEvent e) {
+            SwingFXUtils.runOnFxThread(() -> {
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    swingNode.requestFocus();
+                }
+            });
+        }
+
+        @Override
+        public void windowLostFocus(WindowEvent e) {
+            SwingFXUtils.runOnFxThread(() -> {
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    swingNode.ungrabFocus(true);
+                }
+            });
+        }
+    }
+
+    private static class SwingNodeContent implements LightweightContent {
         private JComponent comp;
         private volatile FXDnD dnd;
+        private WeakReference<SwingNode> swingNodeRef;
 
-        public SwingNodeContent(JComponent comp) {
+        public SwingNodeContent(JComponent comp, SwingNode swingNode) {
             this.comp = comp;
+            this.swingNodeRef = new WeakReference<SwingNode>(swingNode);
         }
         @Override
         public JComponent getComponent() {
@@ -701,11 +740,17 @@ public class SwingNode extends Node {
         }
         @Override
         public void paintLock() {
-            paintLock.lock();
+            SwingNode swingNode = swingNodeRef.get();
+            if (swingNode != null) {
+                swingNode.paintLock.lock();
+            }
         }
         @Override
         public void paintUnlock() {
-            paintLock.unlock();
+            SwingNode swingNode = swingNodeRef.get();
+            if (swingNode != null) {
+                swingNode.paintLock.unlock();
+            }
         }
 
         // Note: we skip @Override annotation and implement both pre-hiDPI and post-hiDPI versions
@@ -716,15 +761,24 @@ public class SwingNode extends Node {
         }
         //@Override
         public void imageBufferReset(int[] data, int x, int y, int width, int height, int linestride, int scale) {
-            SwingNode.this.setImageBuffer(data, x, y, width, height, linestride, scale);
+            SwingNode swingNode = swingNodeRef.get();
+            if (swingNode != null) {
+                swingNode.setImageBuffer(data, x, y, width, height, linestride, scale);
+            }
         }
         @Override
         public void imageReshaped(int x, int y, int width, int height) {
-            SwingNode.this.setImageBounds(x, y, width, height);
+            SwingNode swingNode = swingNodeRef.get();
+            if (swingNode != null) {
+                swingNode.setImageBounds(x, y, width, height);
+            }
         }
         @Override
         public void imageUpdated(int dirtyX, int dirtyY, int dirtyWidth, int dirtyHeight) {
-            SwingNode.this.repaintDirtyRegion(dirtyX, dirtyY, dirtyWidth, dirtyHeight);
+            SwingNode swingNode = swingNodeRef.get();
+            if (swingNode != null) {
+                swingNode.repaintDirtyRegion(dirtyX, dirtyY, dirtyWidth, dirtyHeight);
+            }
         }
         @Override
         public void focusGrabbed() {
@@ -733,49 +787,68 @@ public class SwingNode extends Node {
                 // so we can't delegate it to another GUI toolkit.
                 if (PlatformUtil.isLinux()) return;
 
-                if (getScene() != null &&
-                        getScene().getWindow() != null &&
-                        getScene().getWindow().impl_getPeer() != null) {
-                    getScene().getWindow().impl_getPeer().grabFocus();
-                    grabbed = true;
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    Scene scene = swingNode.getScene();
+                    if (scene != null &&
+                        scene.getWindow() != null &&
+                        scene.getWindow().impl_getPeer() != null) {
+                        scene.getWindow().impl_getPeer().grabFocus();
+                        swingNode.grabbed = true;
+                    }
                 }
             });
         }
         @Override
         public void focusUngrabbed() {
             SwingFXUtils.runOnFxThread(() -> {
-                ungrabFocus(false);
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    swingNode.ungrabFocus(false);
+                }
             });
         }
         @Override
         public void preferredSizeChanged(final int width, final int height) {
             SwingFXUtils.runOnFxThread(() -> {
-                SwingNode.this.swingPrefWidth = width;
-                SwingNode.this.swingPrefHeight = height;
-                SwingNode.this.impl_notifyLayoutBoundsChanged();
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    swingNode.swingPrefWidth = width;
+                    swingNode.swingPrefHeight = height;
+                    swingNode.impl_notifyLayoutBoundsChanged();
+                }
             });
         }
         @Override
         public void maximumSizeChanged(final int width, final int height) {
             SwingFXUtils.runOnFxThread(() -> {
-                SwingNode.this.swingMaxWidth = width;
-                SwingNode.this.swingMaxHeight = height;
-                SwingNode.this.impl_notifyLayoutBoundsChanged();
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    swingNode.swingMaxWidth = width;
+                    swingNode.swingMaxHeight = height;
+                    swingNode.impl_notifyLayoutBoundsChanged();
+                }
             });
         }
         @Override
         public void minimumSizeChanged(final int width, final int height) {
             SwingFXUtils.runOnFxThread(() -> {
-                SwingNode.this.swingMinWidth = width;
-                SwingNode.this.swingMinHeight = height;
-                SwingNode.this.impl_notifyLayoutBoundsChanged();
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    swingNode.swingMinWidth = width;
+                    swingNode.swingMinHeight = height;
+                    swingNode.impl_notifyLayoutBoundsChanged();
+                }
             });
         }
 
         //@Override
         public void setCursor(Cursor cursor) {
             SwingFXUtils.runOnFxThread(() -> {
-                SwingNode.this.setCursor(SwingCursors.embedCursorToCursor(cursor));
+                SwingNode swingNode = swingNodeRef.get();
+                if (swingNode != null) {
+                    swingNode.setCursor(SwingCursors.embedCursorToCursor(cursor));
+                }
             });
         }
 
@@ -783,7 +856,10 @@ public class SwingNode extends Node {
             // This is a part of AWT API, so the method may be invoked on any thread
             synchronized (SwingNodeContent.this) {
                 if (this.dnd == null) {
-                    this.dnd = new FXDnD(SwingNode.this);
+                    SwingNode swingNode = swingNodeRef.get();
+                    if (swingNode != null) {
+                        this.dnd = new FXDnD(swingNode);
+                    }
                 }
             }
         }
diff --git a/tests/system/src/test/java/test/javafx/embed/swing/SwingNodeMemoryLeakTest.java b/tests/system/src/test/java/test/javafx/embed/swing/SwingNodeMemoryLeakTest.java
new file mode 100644
index 000000000..79579c849
--- /dev/null
+++ b/tests/system/src/test/java/test/javafx/embed/swing/SwingNodeMemoryLeakTest.java
@@ -0,0 +1,190 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package test.javafx.embed.swing;
+
+import javafx.application.Application;
+import javafx.application.Platform;
+import javafx.embed.swing.SwingNode;
+import javafx.scene.Scene;
+import javafx.scene.layout.BorderPane;
+import javafx.scene.layout.HBox;
+import javafx.stage.Stage;
+
+import javax.swing.JLabel;
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import util.Util;
+import static util.Util.TIMEOUT;
+import junit.framework.AssertionFailedError;
+import org.junit.Test;
+import org.junit.BeforeClass;
+import org.junit.AfterClass;
+import static org.junit.Assert.assertEquals;
+
+public class SwingNodeMemoryLeakTest {
+
+    final static int TOTAL_SWINGNODE = 10;
+    static CountDownLatch launchLatch;
+    final static int GC_ATTEMPTS = 10;
+    ArrayList<WeakReference<SwingNode>> weakRefArrSN =
+                                      new ArrayList(TOTAL_SWINGNODE);
+    ArrayList<WeakReference<JLabel>> weakRefArrJL =
+                                      new ArrayList(TOTAL_SWINGNODE);
+
+    @BeforeClass
+    public static void setupOnce() {
+        launchLatch = new CountDownLatch(1);
+        // Start the Application
+        new Thread(() -> Application.launch(SwingNodeMemoryLeakTest.MyApp.class, (String[])null)).start();
+
+        try {
+            if (!launchLatch.await(5 * TIMEOUT, TimeUnit.MILLISECONDS)) {
+                throw new AssertionFailedError("Timeout waiting for Application to launch ("+
+                    (5 * TIMEOUT) + " seconds)");
+            }
+        } catch (InterruptedException ex) {
+            AssertionFailedError err = new AssertionFailedError("Unexpected exception");
+            err.initCause(ex);
+            throw err;
+        }
+    }
+
+    @AfterClass
+    public static void teardownOnce() {
+        Platform.exit();
+    }
+
+    @Test
+    public void testSwingNodeMemoryLeak() {
+        Util.runAndWait(() -> {
+            testSwingNodeObjectsInStage();
+        });
+        attemptGCSwingNode();
+        assertEquals(TOTAL_SWINGNODE, getCleanedUpSwingNodeCount());
+
+        attemptGCJLabel();
+        assertEquals(TOTAL_SWINGNODE, getCleanedUpJLabelCount());
+    }
+
+    private void testSwingNodeObjectsInStage() {
+
+        Stage tempStage[] = new Stage[TOTAL_SWINGNODE];
+
+        for (int i = 0; i < TOTAL_SWINGNODE; i++) {
+            BorderPane root = new BorderPane();
+            SwingNode sw = new SwingNode();
+            JLabel label = new JLabel("SWING");
+            sw.setContent(label);
+
+            weakRefArrSN.add(i, new WeakReference<SwingNode>(sw));
+            weakRefArrJL.add(i, new WeakReference<JLabel>(label));
+
+            root.centerProperty().set(sw);
+
+            Stage stage = new Stage();
+            Scene scene = new Scene(root, 150, 100);
+            stage.setScene(scene);
+
+            tempStage[i] = stage;
+        }
+        if (TOTAL_SWINGNODE != weakRefArrSN.size()) {
+            System.out.println("TOTAL_SWINGNODE != weakRefArr.size()");
+        }
+        assertEquals(0, getCleanedUpSwingNodeCount());
+        assertEquals(0, getCleanedUpJLabelCount());
+        assertEquals(TOTAL_SWINGNODE, weakRefArrSN.size());
+        assertEquals(TOTAL_SWINGNODE, weakRefArrJL.size());
+
+        for (int i = 0; i < TOTAL_SWINGNODE; i++) {
+            if (tempStage[i] != null) {
+                tempStage[i].close();
+                tempStage[i] = null;
+            }
+        }
+    }
+
+    private void attemptGCSwingNode() {
+        // Attempt gc GC_ATTEMPTS times
+        for (int i = 0; i < GC_ATTEMPTS; i++) {
+            System.gc();
+            System.runFinalization();
+            if (getCleanedUpSwingNodeCount() == TOTAL_SWINGNODE) {
+                break;
+            }
+            try {
+                Thread.sleep(250);
+            } catch (InterruptedException e) {
+                System.err.println("InterruptedException occurred during Thread.sleep()");
+            }
+        }
+    }
+
+    private void attemptGCJLabel() {
+        // Attempt gc GC_ATTEMPTS times
+        for (int i = 0; i < GC_ATTEMPTS; i++) {
+            System.gc();
+            System.runFinalization();
+            if (getCleanedUpJLabelCount() == TOTAL_SWINGNODE) {
+                break;
+            }
+            try {
+                Thread.sleep(250);
+            } catch (InterruptedException e) {
+                System.err.println("InterruptedException occurred during Thread.sleep()");
+            }
+        }
+    }
+    private int getCleanedUpSwingNodeCount() {
+        int count = 0;
+        for (WeakReference<SwingNode> ref : weakRefArrSN) {
+            if (ref.get() == null) {
+                count++;
+            }
+        }
+        return count;
+    }
+
+    private int getCleanedUpJLabelCount() {
+        int count = 0;
+        for (WeakReference<JLabel> ref : weakRefArrJL) {
+            if (ref.get() == null) {
+                count++;
+            }
+        }
+        return count;
+    }
+
+    public static class MyApp extends Application {
+        @Override
+        public void start(Stage stage) throws Exception {
+            launchLatch.countDown();
+        }
+    }
+
+}