Dynamically Loaded Forms with Cell Factories in JavaFX: Memory Leaks Waiting to Happen?

Question

I am designing a JavaFX app. I have made the decision to load the dialog forms/windows used to collect user input dynamically (from FXML) in response to a user event (eg. clicking a button) and then to "unload" the window when the user is finished with the form and dismisses it.

After each FXML form is loaded, there are listviews, tableviews, and comboboxes that need to be initialized before the dialog is shown to the user. Here is some of the code I use for this purpose:

@FXML // This method is called by the FXMLLoader when 
      //initialization is complete

void initialize() {

    // Initialize your logic here: all @FXML variables will have been injected

    ObservableList<Institution> ilist = Institution.getInstitutionList();
    Callback<ListView<Institution>, ListCell<Institution>> cellfactory =
            new Callback<ListView<Institution>, ListCell<Institution>>() {
                @Override
                public ListCell<Institution> call(ListView<Institution> p) {
                    return new InstitutionListCell();
                }
            };

    cbxInst.setCellFactory(cellfactory);
    cbxInst.setButtonCell(cellfactory.call(null));
    cbxInst.setPromptText(CensusAssistant.RES_STRING_INSTSELECT);
    cbxInst.setItems(ilist);
    cbxInst.setDisable((ilist.size() < 1));


    Callback<ListView<Rotation>, ListCell<Rotation>> rotfactory =
            new Callback<ListView<Rotation>, ListCell<Rotation>>() {
                @Override
                public ListCell<Rotation> call(ListView<Rotation> p) {
                    return new EncRotationListCell();
                }
            };
    :
    :

I don't want to show all the code. The point here is that for every control on my form that uses subclasses of Cell (ListCell, etc.), I am using an anonymous inner class to provide the cell factory.

These anonymous inner classes will be holding references to their enclosing class. My conclusion is that these references will prevent the forms from being garbage collected at any point after the form has been dismissed by the user. I'm worried, then, that if the form is opened and dismissed many times in a session, a memory leak problem will ensue.

Am I on target about this? Will I need to write code that uncouples the cell factories from the controls when the user dismisses the form? Is this a problem with event listeners (or anything that is commonly addressed by using inner classes) in general? Is it wise to always deregister event or action handlers before an object is released?

EDIT:

Many of these same issues have been handled in this post: When exactly is it leak safe to use (anonymous) inner classes?.

I guess I am inclined to believe that for this situation, in which the windows may be repeatedly constructed and dismissed, that I will need to try and null out any potentially long-lived reference to the JavaFX controller class; most especially the cell factories.

Using a memory profiler is on the list of things to do, but I'm still in the throes of application development at a level where I don't feel that I can devote time to this kind of testing. For the time being, I'm just going to try and design forms that "de-initialize" themselves when dismissed. I've also moved the anonymous inner classes from the form controller object to cell factory classes that are implemented as static nested (i.e. top-level) classes. It may be a lot of trouble for not much gain, tho. — scottb, Apr 30 '13 at 20:37
You may find useful next article in this case: http://programmers.stackexchange.com/questions/80084/is-premature-optimization-really-the-root-of-all-evil — Sergey Grinev, Apr 30 '13 at 20:50
The article is apt ... but sometimes I can't stop sweating the small stuff. This is also true, though: there are problems that can be much more easily, quickly, cheaply, and efficiently handled at design time before a project has scaled up than would be possible later, particularly if a major overhaul or retrofit would be required to address an issue. — scottb, Apr 30 '13 at 21:50
2nd approach is valid, but before you start to address it you need to make sure there is an issue. Run profiler. For example you can try NetBeans Profiler -- it shows memory flow in one click if you have NB project. — Sergey Grinev, May 03 '13 at 12:17

score 1 · Accepted Answer · answered May 03 '13 at 23:11

A quick introduction to java garbage collection. Here is an object tree that is probably similar to what you have:

Window
   |-- AnchorPane
            |-- ListView
            |-- Label

I'm not trying to replicate it exactly, it is just an example. Now, if we remove the AnchorPane from the Window, the AnchorPane, ListView and Label will be collected eventually and not be a memory leak. Now if you pass a reference to the AnchorPane to another object then you need to remove both references for the objects to be reclaimed.

I find it easier to try and design a method of developing code which does not invite memory leaks, and then test for the inevitable mistakes.

The best way to think about it is how long an object lives for. If the content of a window is being switched out for different forms, the window will live for longer than the content. Some of your domain objects and services might also live longer than the content. Once you have worked out the relative life times for objects you can follow this rule:

It is OK for short lived objects to reference long lived objects but not the other way around.

In the example, if ListView had a reference to Window (longer lived object) or Label (equal life object) it wouldn't cause a memory leak. Because Window references AnchorPane which is shorter we have to manually remove it from the Window children. For any long to short reference you will have to remove it manually, which is a chance for a bug so sometimes you will have to do this but it is best to minimise these relationships.

For bindings and the scene graph where you don't own the code, it is best to assume you must always manually remove these references since the implementation of what references what might change.

Very informative post. Thanks for taking the time to render it. Anonymous inner class cell factory: the intent is for it to be short lived. Form controller object, longer lived. It would seem prudent to try hard not to use anonymous inner classes for event handlers and control cell factories. — scottb, May 03 '13 at 23:26
Is the form controller really long lived? Typically they are equal. Controls are bound to @FXML annotated fields so you need to throw away both at the same time. I wouldn't worry about this until you can prove it is a problem. Most memory leaks in my experience come from listening to a global observable and then forgetting to remove it. — Andy Till, May 04 '13 at 12:45

Dynamically Loaded Forms with Cell Factories in JavaFX: Memory Leaks Waiting to Happen?

1 Answers1