Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8307538

Memory leak in TreeTableView when calling refresh

XMLWordPrintable

    • b21
    • generic
    • generic

      ADDITIONAL SYSTEM INFORMATION :
      any OS

      edit: **the issue only occurs with TreeTableView**

      A DESCRIPTION OF THE PROBLEM :
      Tree/TableView TableRow & TableCells are created and cached, to reuse when scrolling around the table. When the code calls TableView.refresh() method to redraw the layout (eg. because of data change, or painting change, etc.) the old TableRow and TableCell objects are discarded and new ones are created. However there are properties where the old TableRow and TableCell objects (actually the Skins) installed listeners, which are not removed when new TableRow and TableCell objects are created, thus making the old TableRow and TableCell objects reside in memory forever and they cannot be garbage collected.

      REGRESSION : Last worked in version jfx19.0.2

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Create a TableView with some columns and rows. Place a button on the Scene, and the button's action should call tableView.refresh(). Click it several times (you can also do it in a for cycle), and check the memory consumption and memory leak objects using some profiler tool (Eclipse Memory Analyzer for example). You will see, that there will be a lot of TableCell objects held in memory (with a huge table after a few refresh calls there can be tenthousands of TableCell objects held in memory).

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      The discarded TableCell/TableRow objects should be released.
      ACTUAL -
      They are kept in memory

      ---------- BEGIN SOURCE ----------
      import javafx.application.Application;
      import javafx.application.Platform;
      import javafx.beans.property.SimpleStringProperty;
      import javafx.scene.Scene;
      import javafx.scene.control.Button;
      import javafx.scene.control.TreeItem;
      import javafx.scene.control.TreeTableColumn;
      import javafx.scene.control.TreeTableView;
      import javafx.scene.layout.BorderPane;
      import javafx.stage.Stage;
      public class MemLeakTest
      {
          public static class Person
          {
              public Person(String firstname, String lastname)
              {
                  this.firstname = firstname;
                  this.lastname = lastname;
              }

              public String firstname;
              public String lastname;
          }

          public static class MainApp extends Application
          {
              @Override
              public void start(Stage primaryStage) throws Exception
              {
                  BorderPane pane = new BorderPane();
                  Scene scene = new Scene(pane, 800, 600);

                  TreeTableView<Person> personTreeTableView = new TreeTableView<>();

                  Button refresh = new Button("Refresh");
                  refresh.setOnAction(event ->
                  {
                      personTreeTableView.refresh();
                  });
                  pane.setBottom(refresh);

                  TreeTableColumn<Person, String> column1 = new TreeTableColumn<>("First name");
                  column1.setCellValueFactory(param -> new SimpleStringProperty(param.getValue().getValue().firstname));

                  TreeTableColumn<Person, String> column2 = new TreeTableColumn<>("Last name");
                  column2.setCellValueFactory(param -> new SimpleStringProperty(param.getValue().getValue().lastname));

                  personTreeTableView.getColumns().addAll(column1, column2);

                  TreeItem<Person> root = new TreeItem<>(null);
                  personTreeTableView.setShowRoot(false);

                  root.getChildren().add(new TreeItem<>(new Person("Eric Maxim", "Choupo-Moting")));
                  root.getChildren().add(new TreeItem<>(new Person("Thomas", "Müller")));
                  root.getChildren().add(new TreeItem<>(new Person("Sadio", "Mané")));
                  root.getChildren().add(new TreeItem<>(new Person("Manuel", "Neuer")));
                  root.getChildren().add(new TreeItem<>(new Person("Yann", "Sommer")));
                  root.getChildren().add(new TreeItem<>(new Person("Jamal", "Musiala")));
                  root.getChildren().add(new TreeItem<>(new Person("Leroy", "Sané")));
                  root.getChildren().add(new TreeItem<>(new Person("Benjamin", "Pavard")));
                  root.getChildren().add(new TreeItem<>(new Person("Kingsley", "Coman")));
                  root.getChildren().add(new TreeItem<>(new Person("Leon", "Goretzka")));
                  root.getChildren().add(new TreeItem<>(new Person("Joshua", "Kimmich")));

                  personTreeTableView.setRoot(root);

                  pane.setCenter(personTreeTableView);

                  primaryStage.setScene(scene);
                  primaryStage.sizeToScene();

                  primaryStage.show();
              }
          }

          public static void main(String... args)
          {
              Platform.startup(() ->
              {
                  try
                  {
                      new MainApp().start(new Stage());
                  }
                  catch (Exception e)
                  {
                      throw new RuntimeException(e);
                  }
              });
          }
      }

      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      I found that com.sun.javafx.scene.control.ListenerHelper is responsible to install the property invalidation/change listeners for the TableRow/TableCell components. Wrapping these listeners into WeakInvalidation/ChangeListener solved the problem, after this change these objects could be released by the GC system.

      package com.sun.javafx.scene.control;

      import java.lang.ref.WeakReference;
      import java.util.ArrayList;
      import java.util.function.Consumer;
      import java.util.function.Function;
      import javafx.beans.InvalidationListener;
      import javafx.beans.Observable;
      import javafx.beans.WeakInvalidationListener;
      import javafx.beans.value.ChangeListener;
      import javafx.beans.value.ObservableValue;
      import javafx.beans.value.WeakChangeListener;
      import javafx.collections.*;
      import javafx.concurrent.Task;
      import javafx.event.Event;
      import javafx.event.EventHandler;
      import javafx.event.EventType;
      import javafx.scene.Node;
      import javafx.scene.Scene;
      import javafx.scene.control.MenuItem;
      import javafx.scene.control.SkinBase;
      import javafx.scene.control.TableColumnBase;
      import javafx.scene.control.TreeItem;
      import javafx.scene.transform.Transform;
      import javafx.stage.Window;

      /**
       * This class provides convenience methods for adding various listeners, both
       * strong and weak, as well as a single {@link #disconnect()} method to remove
       * all listeners.
       * <p>
       * There are two usage patterns:
       * <ul>
       * <li>Client code registers a number of listeners and removes them all at once
       * via {@link #disconnect()} call.
       * <li>Client code registers a number of listeners and removes one via its
       * {@link IDisconnectable} instance.
       * </ul>
       *
       * This class is currently used for clean replacement of {@link Skin}s.
       * We should consider making this class a part of the public API in {@code javax.base},
       * since it proved itself useful in removing listeners and handlers in bulk at the application level.
       */
      public class ListenerHelper implements IDisconnectable {
          private WeakReference<Object> ownerRef;
          private final ArrayList<IDisconnectable> items = new ArrayList<>(4);
          private static Function<SkinBase<?>,ListenerHelper> accessor;

          public ListenerHelper(Object owner) {
              ownerRef = new WeakReference<>(owner);
          }

          public ListenerHelper() {
          }

          public static void setAccessor(Function<SkinBase<?>,ListenerHelper> a) {
              accessor = a;
          }

          public static ListenerHelper get(SkinBase<?> skin) {
              return accessor.apply(skin);
          }

          public IDisconnectable addDisconnectable(Runnable r) {
              IDisconnectable d = new IDisconnectable() {
                  @Override
                  public void disconnect() {
                      items.remove(this);
                      r.run();
                  }
              };
              items.add(d);
              return d;
          }

          @Override
          public void disconnect() {
              for (int i = items.size() - 1; i >= 0; i--) {
                  IDisconnectable d = items.remove(i);
                  d.disconnect();
              }
          }

          private boolean isAliveOrDisconnect() {
              if (ownerRef != null) {
                  if (ownerRef.get() == null) {
                      disconnect();
                      return false;
                  }
              }
              return true;
          }

          // change listeners

          public IDisconnectable addChangeListener(Runnable callback, ObservableValue<?>... props) {
              return addChangeListener(callback, false, props);
          }

          public IDisconnectable addChangeListener(Runnable onChange, boolean fireImmediately, ObservableValue<?>... props) {
              if (onChange == null) {
                  throw new NullPointerException("onChange must not be null.");
              }

              ChLi li = new ChLi() {
                  @Override
                  public void disconnect() {
                      for (ObservableValue p : props) {
                          p.removeListener(this);
                      }
                      items.remove(this);
                  }

                  @Override
                  public void changed(ObservableValue p, Object oldValue, Object newValue) {
                      if (isAliveOrDisconnect()) {
                          onChange.run();
                      }
                  }
              };

              items.add(li);

              for (ObservableValue p : props) {
                  p.addListener(new WeakChangeListener(li));
              }

              if (fireImmediately) {
                  onChange.run();
              }

              return li;
          }

          public <T> IDisconnectable addChangeListener(ObservableValue<T> prop, ChangeListener<T> listener) {
              return addChangeListener(prop, false, listener);
          }

          public <T> IDisconnectable addChangeListener(ObservableValue<T> prop, boolean fireImmediately, ChangeListener<T> listener) {
              if (listener == null) {
                  throw new NullPointerException("Listener must be specified.");
              }

              ChLi<T> li = new ChLi<T>() {
                  @Override
                  public void disconnect() {
                      prop.removeListener(this);
                      items.remove(this);
                  }

                  @Override
                  public void changed(ObservableValue<? extends T> src, T oldValue, T newValue) {
                      if (isAliveOrDisconnect()) {
                          listener.changed(src, oldValue, newValue);
                      }
                  }
              };

              items.add(li);
              prop.addListener(new WeakChangeListener<T>(li));

              if (fireImmediately) {
                  T v = prop.getValue();
                  listener.changed(prop, null, v);
              }

              return li;
          }

          public <T> IDisconnectable addChangeListener(ObservableValue<T> prop, Consumer<T> callback) {
              return addChangeListener(prop, false, callback);
          }

          public <T> IDisconnectable addChangeListener(ObservableValue<T> prop, boolean fireImmediately, Consumer<T> callback) {
              if (callback == null) {
                  throw new NullPointerException("Callback must be specified.");
              }

              ChLi<T> li = new ChLi<T>() {
                  @Override
                  public void disconnect() {
                      prop.removeListener(this);
                      items.remove(this);
                  }

                  @Override
                  public void changed(ObservableValue<? extends T> observable, T oldValue, T newValue) {
                      if (isAliveOrDisconnect()) {
                          callback.accept(newValue);
                      }
                  }
              };

              items.add(li);
              prop.addListener(new WeakChangeListener<T>(li));

              if (fireImmediately) {
                  T v = prop.getValue();
                  callback.accept(v);
              }

              return li;
          }

          // invalidation listeners

          public IDisconnectable addInvalidationListener(Runnable callback, ObservableValue<?>... props) {
              return addInvalidationListener(callback, false, props);
          }

          public IDisconnectable addInvalidationListener(Runnable callback, boolean fireImmediately, ObservableValue<?>... props) {
              if (callback == null) {
                  throw new NullPointerException("Callback must be specified.");
              }

              InLi li = new InLi() {
                  @Override
                  public void disconnect() {
                      for (ObservableValue p : props) {
                          p.removeListener(this);
                      }
                      items.remove(this);
                  }

                  @Override
                  public void invalidated(Observable p) {
                      if (isAliveOrDisconnect()) {
                          callback.run();
                      }
                  }
              };

              items.add(li);

              for (ObservableValue p : props) {
                  p.addListener(new WeakInvalidationListener(li));
              }

              if (fireImmediately) {
                  callback.run();
              }

              return li;
          }

          public <T> IDisconnectable addInvalidationListener(ObservableValue<T> prop, InvalidationListener listener) {
              return addInvalidationListener(prop, false, listener);
          }

          public <T> IDisconnectable addInvalidationListener(ObservableValue<T> prop, boolean fireImmediately, InvalidationListener listener) {
              if (listener == null) {
                  throw new NullPointerException("Listener must be specified.");
              }

              InLi li = new InLi() {
                  @Override
                  public void disconnect() {
                      prop.removeListener(this);
                      items.remove(this);
                  }

                  @Override
                  public void invalidated(Observable observable) {
                      if (isAliveOrDisconnect()) {
                          listener.invalidated(observable);
                      }
                  }
              };

              items.add(li);
              prop.addListener(new WeakInvalidationListener(li));

              if (fireImmediately) {
                  listener.invalidated(prop);
              }

              return li;
          }

          // list change listeners

          public <T> IDisconnectable addListChangeListener(ObservableList<T> list, ListChangeListener<T> listener) {
              if (listener == null) {
                  throw new NullPointerException("Listener must be specified.");
              }

              LiChLi<T> li = new LiChLi<T>() {
                  @Override
                  public void disconnect() {
                      list.removeListener(this);
                      items.remove(this);
                  }

                  @Override
                  public void onChanged(Change<? extends T> ch) {
                      if (isAliveOrDisconnect()) {
                          listener.onChanged(ch);
                      }
                  }
              };

              items.add(li);
              list.addListener(new WeakListChangeListener<T>(li));

              return li;
          }

          // map change listener

          public <K,V> IDisconnectable addMapChangeListener(ObservableMap<K,V> list, MapChangeListener<K,V> listener) {
              if (listener == null) {
                  throw new NullPointerException("Listener must be specified.");
              }

              MaChLi<K,V> li = new MaChLi<K,V>() {
                  @Override
                  public void disconnect() {
                      list.removeListener(this);
                      items.remove(this);
                  }

                  @Override
                  public void onChanged(Change<? extends K, ? extends V> ch) {
                      if (isAliveOrDisconnect()) {
                          listener.onChanged(ch);
                      }
                  }
              };

              items.add(li);
              list.addListener(new WeakMapChangeListener<>(li));

              return li;
          }

          // set change listeners

          public <T> IDisconnectable addSetChangeListener(ObservableSet<T> set, SetChangeListener<T> listener) {
              if (listener == null) {
                  throw new NullPointerException("Listener must be specified.");
              }

              SeChLi<T> li = new SeChLi<T>() {
                  @Override
                  public void disconnect() {
                      set.removeListener(this);
                      items.remove(this);
                  }

                  @Override
                  public void onChanged(Change<? extends T> ch) {
                      if (isAliveOrDisconnect()) {
                          listener.onChanged(ch);
                      }
                  }
              };

              items.add(li);
              set.addListener(new WeakSetChangeListener<T>(li));

              return li;
          }

          // event handlers

          public <T extends Event> IDisconnectable addEventHandler(Object x, EventType<T> t, EventHandler<T> handler) {
              EvHa<T> h = new EvHa<>(handler) {
                  @Override
                  public void disconnect() {
                      if (x instanceof Node n) {
                          n.removeEventHandler(t, this);
                      } else if (x instanceof Window y) {
                          y.removeEventHandler(t, this);
                      } else if (x instanceof Scene y) {
                          y.removeEventHandler(t, this);
                      } else if (x instanceof MenuItem y) {
                          y.removeEventHandler(t, this);
                      } else if (x instanceof TreeItem y) {
                          y.removeEventHandler(t, this);
                      } else if (x instanceof TableColumnBase y) {
                          y.removeEventHandler(t, this);
                      } else if (x instanceof Transform y) {
                          y.removeEventHandler(t, this);
                      } else if (x instanceof Task y) {
                          y.removeEventHandler(t, this);
                      }
                      items.remove(this);
                  }
              };

              items.add(h);

              // we really need an interface here ... "HasEventHandlers"
              if (x instanceof Node y) {
                  y.addEventHandler(t, h);
              } else if (x instanceof Window y) {
                  y.addEventHandler(t, h);
              } else if (x instanceof Scene y) {
                  y.addEventHandler(t, h);
              } else if (x instanceof MenuItem y) {
                  y.addEventHandler(t, h);
              } else if (x instanceof TreeItem y) {
                  y.addEventHandler(t, h);
              } else if (x instanceof TableColumnBase y) {
                  y.addEventHandler(t, h);
              } else if (x instanceof Transform y) {
                  y.addEventHandler(t, h);
              } else if (x instanceof Task y) {
                  y.addEventHandler(t, h);
              } else {
                  throw new IllegalArgumentException("Cannot add event handler to " + x);
              }

              return h;
          }

          // event filters

          public <T extends Event> IDisconnectable addEventFilter(Object x, EventType<T> t, EventHandler<T> handler) {
              EvHa<T> h = new EvHa<>(handler) {
                  @Override
                  public void disconnect() {
                      if (x instanceof Node n) {
                          n.removeEventFilter(t, this);
                      } else if (x instanceof Window y) {
                          y.removeEventFilter(t, this);
                      } else if (x instanceof Scene y) {
                          y.removeEventFilter(t, this);
                      } else if (x instanceof Transform y) {
                          y.removeEventFilter(t, this);
                      } else if (x instanceof Task y) {
                          y.removeEventFilter(t, this);
                      }
                      items.remove(this);
                  }
              };

              items.add(h);

              // we really need an interface here ... "HasEventFilters"
              if (x instanceof Node y) {
                  y.addEventFilter(t, h);
              } else if (x instanceof Window y) {
                  y.addEventFilter(t, h);
              } else if (x instanceof Scene y) {
                  y.addEventFilter(t, h);
              } else if (x instanceof Transform y) {
                  y.addEventFilter(t, h);
              } else if (x instanceof Task y) {
                  y.addEventFilter(t, h);
              } else {
                  throw new IllegalArgumentException("Cannot add event filter to " + x);
              }

              return h;
          }

          //

          private static abstract class ChLi<T> implements IDisconnectable, ChangeListener<T> { }

          private static abstract class InLi implements IDisconnectable, InvalidationListener { }

          private static abstract class LiChLi<T> implements IDisconnectable, ListChangeListener<T> { }

          private static abstract class MaChLi<K,V> implements IDisconnectable, MapChangeListener<K,V> { }

          private static abstract class SeChLi<T> implements IDisconnectable, SetChangeListener<T> { }

          private abstract class EvHa<T extends Event> implements IDisconnectable, EventHandler<T> {
              private final EventHandler<T> handler;

              public EvHa(EventHandler<T> h) {
                  this.handler = h;
              }

              @Override
              public void handle(T ev) {
                  if (isAliveOrDisconnect()) {
                      handler.handle(ev);
                  }
              }
          }
      }


      FREQUENCY : always


        1. Capture0.PNG
          Capture0.PNG
          159 kB
        2. Capture1.PNG
          Capture1.PNG
          246 kB
        3. Capture2.PNG
          Capture2.PNG
          265 kB
        4. MemLeakTest.java
          3 kB

            angorya Andy Goryachev
            webbuggrp Webbug Group
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: