8227619: Potential memory leak in javafx.scene.control.ListView

Reviewed-by: kcr, aghaisas
openjdk · Feb 11, 2020 · e986459 · e986459
1 parent a74137a
commit e986459
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/...trols/src/main/java/com/sun/javafx/scene/control/SelectedItemsReadOnlyObservableList.java b/...trols/src/main/java/com/sun/javafx/scene/control/SelectedItemsReadOnlyObservableList.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, 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
@@ -27,6 +27,7 @@
 import javafx.collections.ListChangeListener;
 import javafx.collections.ObservableList;
 import javafx.collections.ObservableListBase;
+import javafx.collections.WeakListChangeListener;
 
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
@@ -46,6 +47,8 @@ public abstract class SelectedItemsReadOnlyObservableList<E> extends ObservableL
         itemsListChanged = true;
         itemsListChange = c;
     };
+    private final WeakListChangeListener weakItemsListListener =
+            new WeakListChangeListener(itemsListListener);
 
     private final Supplier<Integer> modelSizeSupplier;
 
@@ -120,11 +123,11 @@ public int size() {
     // Used by ListView and TableView to allow for improved handling.
     public void setItemsList(ObservableList<E> itemsList) {
         if (this.itemsList != null) {
-            this.itemsList.removeListener(itemsListListener);
+            this.itemsList.removeListener(weakItemsListListener);
         }
         this.itemsList = itemsList;
         if (itemsList != null) {
-            itemsList.addListener(itemsListListener);
+            itemsList.addListener(weakItemsListListener);
         }
     }
 

diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2020, 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
@@ -28,6 +28,7 @@
 import com.sun.javafx.scene.control.VirtualScrollBar;
 import com.sun.javafx.scene.control.behavior.ListCellBehavior;
 import com.sun.javafx.tk.Toolkit;
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -1975,4 +1976,28 @@ public void testEventIndicesOnSelectRange() {
         assertEquals("List item at index 1 should be selected", 1, (int) sm.getSelectedIndices().get(0));
         assertEquals("List item at index 2 should be selected", 2, (int) sm.getSelectedIndices().get(1));
     }
+
+    @Test
+    public void testListViewLeak() {
+        ObservableList<String> items = FXCollections.observableArrayList();
+        WeakReference<ListView<String>> listViewRef = new WeakReference<>(new ListView<>(items));
+        attemptGC(listViewRef, 10);
+        assertNull("ListView has a leak.", listViewRef.get());
+    }
+
+    private void attemptGC(WeakReference<ListView<String>> weakRef, int n) {
+        for (int i = 0; i < n; i++) {
+            System.gc();
+            System.runFinalization();
+
+            if (weakRef.get() == null) {
+                break;
+            }
+            try {
+                Thread.sleep(50);
+            } catch (InterruptedException e) {
+                fail("InterruptedException occurred during Thread.sleep()");
+            }
+        }
+    }
 }