JDK-8089305 : Memory leak - many "weak" listeners within JavaFX core classes do not implement WeakListener
  • Type: Bug
  • Component: javafx
  • Sub-Component: base
  • Affected Version: 8u31
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-02-24
  • Updated: 2015-08-04
  • Resolved: 2015-08-04
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.
JDK 9
9Fixed
Description
Run the program in the first comment - the memory gets GCed correctly. 
Run the program in the second comment  - the memory grows forever.

The reason is that many listeners used internally in JFX bindings hold a weak reference to the target listener, but do not implement the javafx.beans.WeakListener interface.

Hence they are never removed within the ExpressionHelperBase.trim() method, even though the target listener had been GC-ed.

In this case, it is the StringPropertyBase$Listener class, but it happens on many more places across JavaFX.

The posted example is actually very realistic. In our application, short-lived objects or table cells used to bind to long-lived properties (imagine, for example, order status, which often does not change for days, but when it changes, we need to display the new value). 
After a couple of hours of extensive work with the application (lots of table updates, lots of scrolling), we sometimes had a couple hundreds MB occupied by these empty listeners.  Not only that it consumes memory, the application gets overall slower, as the properties have to scroll through the long lists of listeners. 
One simply can not use bindings in such cases and must register explicit weak listeners. 
I believe that the issue could be fixed simply by implementing the WeakListener interface in those listeners used in bindings, wouldn't it?

Comments
Changeset: 410a2ea780c6 Author: vadim Date: 2015-08-04 12:49 +0300 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/410a2ea780c6
04-08-2015

+1. The change looks good to me, I have also verified that is passed dev-build-lite.sh.
31-07-2015

Forgot to add that trim() implementation is not optimal, especially for artificial test cases. Usually GC cleans WeakReferences in chunks so if there are a lot of cleaned WeakListeners, trim() will copy the rest of the listeners array again and again. Now it finds first cleaned listener and picks not cleaned listeners one by one. There is an existing ExpressionHelperBaseTest which checks most cases so there is no new test for this change.
30-07-2015

Chien, Kevin, Please review this fix: http://cr.openjdk.java.net/~vadim/8089305/webrev.00/ Implementing WeakListener for PropertyBase and BindingHelperObserver allows ExpressionHelperBase.trim() to remove GC'd listeners. The first test sanity checks that invalidating value triggers removing listener. Second test first adds 2 listeners so the ExpressionHelper creates Generic listener holder which calls trim(). Third test will test JDK-8130458.
30-07-2015

That would be great. I'll ask my colleagues to vote for it.
25-02-2015

Target for FX 9, but could consider bringing it into 8u60.
24-02-2015

import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; public class MemoryLeak { public static void main(String[] args) { StringProperty p = new SimpleStringProperty(); while(true) { for(int i=0; i<1000; i++) { new SimpleStringProperty().bind(p); } try { Thread.sleep(10); } catch (InterruptedException e) { e.printStackTrace(); } } } }
24-02-2015

import javafx.beans.InvalidationListener; import javafx.beans.Observable; import javafx.beans.WeakInvalidationListener; import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; public class MemoryOK { public static void main(String[] args) { StringProperty p = new SimpleStringProperty(); while(true) { for(int i=0; i<1000; i++) { p.addListener(new WeakInvalidationListener(new MyListener())); } try { Thread.sleep(10); } catch (InterruptedException e) { e.printStackTrace(); } } } private static class MyListener implements InvalidationListener { @Override public void invalidated(Observable observable) { } } }
24-02-2015