Memory Leak?

[FunkyELF]

I am writing a little utility as mentioned here.
http://www.jfree.org/phpBB2/viewtopic.php?t=20945

I have files with a bunch of datasets (average is about 120 data sets with 1,600 datapoints each).

Every time I show a new data set, I create a new chart.
I could watch the memory usage go up as I go through the available charts. It would start at around 36 MB and go up from there, about a megabyte every 4 charts.

I solved this problem and now the memory hovers around 36 MB and going from chart to chart the memory will go up or down so I'm pretty sure I have the leak solved.

This is how I solved it. I commented the part the solves the problem

Code: Select all

        this.removeAll();

        // This is what solves the problem.  Comment out these two lines and there will be memory leaks.
        if(chart != null)
            chart.getXYPlot().setDataset(0,null);
        
        chart = ChartFactory.createXYLineChart(Global.currentDataFile.getName(), "Freq (Hz)", "Real", dataset, PlotOrientation.VERTICAL, true, true, false);

It appears that setting the plot to null fixes everything.
Without it, there will be memory leaks.

I have tried to only create the chart once in the constructor and change out the datasets by changing

Code: Select all

        if(chart != null)
            chart.getXYPlot().setDataset(0,null);

to...

Code: Select all

        if(chart != null)
            chart.getXYPlot().setDataset(0,dataset);

...or even

Code: Select all

        if(chart != null){
            chart.getXYPlot().setDataset(0,null);
            chart.getXYPlot().setDataset(0,dataset);
        }

I still get the memory leaks.

What should I be doing?

If I should be creating a new chart every time, why do I need to call chart.getXYPlot().setDataset(0,null); to not get any memory leaks. Is there a way to create a new chart every time and not get memory leaks without explicitly calling that? Shouldn't the this.removeAll(); fix everything?

If I should be re-using the same chart and just switching out the datasets, what am I doing wrong?

Thanks for the help in advance.

~Eric

[FunkyELF]

Also....

After I did that little fix by setting the dataset to null, I can still induce memory errors by using the mouse and zooming back and forth. Select a rectangle, zoom back to original, select a rectangle, zoom back to original over and over again eats up memory that I can't seem to get back.

~Eric

[david.gilbert]

You really need to put together a self-contained demo app that reproduces this problem...otherwise I'll have no chance to trace the memory leak (in particular, to find out if it is something in JFreeChart, or something in your application code).

[FunkyELF]

I took a look into the source code for XYPlot and saw what was happening when I called setDataSet(null); and it looked like it was removing change listeners.

So I tried the following code and it fixed it just the same as running setDataSet(null);

Code: Select all

        if(chart != null && chart.getXYPlot().getDataset() != null){
            chart.getXYPlot().getDataset(0).removeChangeListener(chart.getXYPlot());
        }

So it appears to be a swing issue. Without either explicitly removing the change listener from the XYDataSet or indirectly removing it by calling setDataSet(null) on the XYPlot object there will be memory leaks.

Shouldn't I be able to remove a JFreeChart from a JPanel by calling this.removeAll(); and have all the resources associated with the JFreeChart garbage collected?

[Developer Dude]

I took a look into the source code for XYPlot and saw what was happening when I called setDataSet(null); and it looked like it was removing change listeners.

So I tried the following code and it fixed it just the same as running setDataSet(null);

Code: Select all

        if(chart != null && chart.getXYPlot().getDataset() != null){
            chart.getXYPlot().getDataset(0).removeChangeListener(chart.getXYPlot());
        }

So it appears to be a swing issue. Without either explicitly removing the change listener from the XYDataSet or indirectly removing it by calling setDataSet(null) on the XYPlot object there will be memory leaks.

Shouldn't I be able to remove a JFreeChart from a JPanel by calling this.removeAll(); and have all the resources associated with the JFreeChart garbage collected?

I ran into a very similar problem this morning: I am running a set of benchmarks on the various charting packages and I create 100K charts in a tight loop. I got a heap out of memory error and noticed it was in the dataset add change listener method.

This is not a Swing SDK bug, but this is a common bug in Swing based programs (and any other programs that use listeners - Swing programs use a lot of listeners); if an object adds a listener to an object that is not contained within the listener, then the listened to object holds a reference to the listener and neither object is garbage collected even though both objects have no other references - they hold a reference to each other.

I've been guilty of this myself and it is something to watch out for and not that easy to notice unless you create a lot of these objects.

David, I can repro this very simply: I just create a single dataset and call the ChartFactory.createXYLineChart() method repeatedly in a loop (ChartFactory.createXYLineChart() creates an XYPlot inside a chart and the plot adds a listener to the dataset when it is created so the plot always has a reference to it in the dataset listener list).

I work around it by calling
dataset.removeChangeListener(chart.getXYPlot()); inside the loop after I create the chart and I am finished with it.

[david.gilbert]

...and neither object is garbage collected even though both objects have no other references - they hold a reference to each other.

That's not true, the garbage collector is "smart" enough to detect such cycles and remove the objects.

I just create a single dataset and call the ChartFactory.createXYLineChart() method repeatedly in a loop (ChartFactory.createXYLineChart() creates an XYPlot inside a chart and the plot adds a listener to the dataset when it is created so the plot always has a reference to it in the dataset listener list).

I work around it by calling
dataset.removeChangeListener(chart.getXYPlot()); inside the loop after I create the chart and I am finished with it.

OK, now I see the problem. This single dataset (that is always referenced so can't be garbage collected) keeps references to all its "listener" charts...so even though those charts aren't used any more, they can't be garbage collected (because they are referenced by the dataset). Note how that's different to the scenario you outlined above.

Manually removing the listener as you've done is about all I can suggest at the moment...I can't think of a mechanism that will allow the dataset to know when the chart is finished with (other than maybe adding a dispose() method to the JFreeChart class...but that also relies on the developer remembering to call dispose()). Let me go and see what JTable does in relation to its TableModel, because that's the same relationship as the plot to its dataset...

[Developer Dude]

...and neither object is garbage collected even though both objects have no other references - they hold a reference to each other.

That's not true, the garbage collector is "smart" enough to detect such cycles and remove the objects.

I mispoke: there isn't a cycle; the chart doesn't have a reference to the dataset, but the dataset does hold a reference to chart. That isn't a cyclic reference. Nonetheless, it is enough to keep it in memory and not garbage collected.

OK, now I see the problem. This single dataset (that is always referenced so can't be garbage collected) keeps references to all its "listener" charts...so even though those charts aren't used any more, they can't be garbage collected (because they are referenced by the dataset). Note how that's different to the scenario you outlined above.

Manually removing the listener as you've done is about all I can suggest at the moment...I can't think of a mechanism that will allow the dataset to know when the chart is finished with (other than maybe adding a dispose() method to the JFreeChart class...but that also relies on the developer remembering to call dispose()). Let me go and see what JTable does in relation to its TableModel, because that's the same relationship as the plot to its dataset...

Without learning the code and the architecture I can't make any suggestions about how to remedy this, but I can say that it probably isn't good design for the user of a chart to have to know this inside detail and workaround.

How I handle this problem in Swing components is to either not have a reference to an external object (I made a lot of POJO property editors), or I had one or the other component remove listeners or ask that itself be removed as a listener in a close() method. There were variations of that - but it is a common problem.

My use of JFreeChart would be to generate files which would go into a report for a web front end, or to be included in a PDF file for emailing. The scale would be tens of thousands of reports each night. So obviously this kind of leak would be a problem. Fortunately the workaround is fairly straightforward (as far as I can tell there is no side-effects) - but IMO it is a workaround and I suggest further consideration of a cleaner solution.

[Taqua]

Well, if you hold a reference to the same dataset instance all the time, then you will run into trouble. But for reporting needs, you usually create a new dataset based on data from a database or so each time you run a report.

And if you have a long running global object, then well, you may have to provide a better implementation. One solution would be to implement the dataset interface in a way to have weak-references to your listeners. But as this is a application-specific case, it belongs in your application's domain.

Please remember: the datasets provided by JFreeChart are only default implementations. Each dataset type is defined only by its interface; if the default implementation dont suit your needs, implement your own. Personally, I barely use the default datasets (except in the most simple cases) and implement own optimized versions based on my local application model.

[Developer Dude]

Well, if you hold a reference to the same dataset instance all the time, then you will run into trouble. But for reporting needs, you usually create a new dataset based on data from a database or so each time you run a report.

And if you have a long running global object, then well, you may have to provide a better implementation. One solution would be to implement the dataset interface in a way to have weak-references to your listeners. But as this is a application-specific case, it belongs in your application's domain.

Please remember: the datasets provided by JFreeChart are only default implementations. Each dataset type is defined only by its interface; if the default implementation dont suit your needs, implement your own. Personally, I barely use the default datasets (except in the most simple cases) and implement own optimized versions based on my local application model.

In practice I probably would not have the same dataset for every chart of the same kind - I can't think of a good reason to do that. It would be useful to have the same dataset/model for different views though.

But for a benchmark where I am looking at how chart creation scales, then there is little reason to create a new dataset for each iteration - I am just trying to get a rough idea of how JFreeChart scales.

Still, you can expect that some people would use the default data set implementation for simple data models, and either way people should be aware of this problem which seems to me to be an internal implementation detail of chart creation. I am just suggesting that maybe there should be a method or process in the interface between the chart and the dataset where any dangling listener references are removed, even if it is a one to one reference instead of a one to many as in my case - the former can grow to be as numerous as the latter if the user of the lib is creating thousands of charts at a time.

[david.gilbert]

One solution would be to implement the dataset interface in a way to have weak-references to your listeners. But as this is a application-specific case, it belongs in your application's domain.

Hi Thomas,

What would be the downside, if any, of using WeakReferences in JFreeChart's default dataset implementations? It sounds like exactly the right solution from what I've read so far.

[david.gilbert]

It would be useful to have the same dataset/model for different views though.

Absolutely, so this problem needs addressing. I did some searching for the way that this is handled by JTable, and it sounds like it has the same problem:

http://www.jroller.com/santhosh/entry/l ... _may_cause

So far, using WeakReferences in the dataset implementations seems like a reasonable solution, but I'll do some more reading on that to make sure I'm not missing something.

By the way, I appreciate the time you've taken to explain the problem - it has helped me to understand better what is going on.

[david.gilbert]

Here's another interesting link, although I'm not sure yet what problem is being solved by the WeakRefActionListener wrapper class:

http://www.objectdefinitions.com/odblog ... r-pattern/

[Developer Dude]

In one application I worked on we had a synch manager framework that used weak references to help keep different views aware of changes to various entities that a view might be displaying. For example, an editor might allow the user to change or delete an entity that was opened as a node from a tree view, or there might be a table that contains an entity that is also displayed as a node in a tree view and the user could delete the entity from either view.

We didn't want to pass around the same object to every view - sometimes you want to change an object in an editor before saving or canceling changes and you don't want those changes to show up in other views until committed. So we cloned our entities. Besides, in the editors we kept a copy (also a clone) of the original entity object to compare to the changed entity object.

We didn't want every view to listen to every other view - that would be coupling them too tightly.

The solution was a synch manager that listened for deletions and commits to the server using weak references and the views registered themselves as interested parties for entities and/or actions. It took a while to get it right. Besides bugs, the main mistake we made was listening to all the property changes when we were really only interested in general actions - the final version told a listener of an action and passed in a before and after copy of the entity, then the listener could decide what to do about it.

I should mention that all of our entities had GUIDs and that is how we kept track of them and whether a listener was interested in a specific entity.