Problem with disposal of ChartComposite object

[paulb]

I had a problem with using JFreeChart in an RCP Application. However, it should apply for all uses with SWT.

I have several TimeSeries which are plotted in an XYStepChart using TimeSeriesCollection. Data is added to the TimeSeries on a regular basis. This is independent of whether the chart is visible or not.
The chart is displayed in an editor (RCP) using ChartComposite. So each time the editor is openened, a ChartComposite is created using the JFreeChart object.
The problem was that after closing and reopening the editor the chart would not be correctly updated with the TimeSeries data that was added while it was not visible and also not according to newly added data (again, data is added regularly, whether the chart is visible (i.e. as a composite) or not).
I figured out that the reason for this was in the disposal of the ChartComposite object (which is disposed when its parent is disposed, i.e., when the editor is closed). The ChartComposite is registered as a listener to the respective JFreeChart which is in turn a listener of the data sets. After disposing the ChartComposite the update of the TimeSeries caused an exception because of the notifications to the still registered ChartComposite object.
Interestingly, manually unregistering the ChartComposite from the JFreeChart object before disposal did not help. The reason was that - I don't know why - the ChartComposite appeared twice in the list of listeners in the JFreeChart object. So unregistering it twice was the solution.

I would consider this a bug (and especially I'm not sure if it is "guaranteed" that the ChartComposite is registered as a listener twice - or maybe more often in some cases) or am I doing anything wrong? Also, shouldn't the ChartComposite unregister itself as a listener when it is disposed (I'm not sure if that is possible, though..).

Thanks for the help.
-- Paul

[david.gilbert]

Thanks for reporting this. I'm not all that familiar with the SWT code, but looking through the constructor I can see the cause of the ChartComposite being registered twice as a listener. It gets registered first in the call:

Code: Select all

this.setChart(jfreechart);

...then again later on in the constructor:

Code: Select all

        if (this.chart != null) {
            this.chart.addChangeListener(this);
            ...
        }

That's wrong, but I'll have to study the code some more to work out which one should be removed.

Regarding whether or not the ChartComposite should deregister itself when it is disposed - yes, I think it should (and ChartPanel probably suffers the same bug, I'll have a look at that too).

I opened this bug report to track it:

http://sourceforge.net/tracker/index.ph ... tid=115494

[paulb]

Thanks for the quick reply. I didn't have the chance to take a closer look at the ChartComposite code, yesterday. Also I am not so familiar with the JFreeChart internals.
Thanks for now and I'll keep you updated if I find out more.

[paulb]

A temporary workaround for those who don't want to change the JFreeChart code is to register as a disposal listener of the ChartComposite object. Even though it's relatively obvious, some sample code:

Code: Select all

class ...  implements DisposeListener {
	ChartComposite chartComposite;
	JFreeChart chart;

	public ..  {
		//...
		chart = ...
		chartComposite = new ChartComposite(..., chart, ...);
		chartComposite.addDisposeListener(this);
	}
	
	public void widgetDisposed(DisposeEvent e) {		// called when chartComposite is disposed
		// dirty double removal of possible listener roles of chartComposite
		chart.removeChangeListener(chartComposite);
		chart.removeProgressListener(chartComposite);
		chart.removeChangeListener(chartComposite);
		chart.removeProgressListener(chartComposite);
	}

}

[heprom]

Hi, this problem is now fixed in the svn tree. Will be in jfreechart 1.10.

Thanks for your feedback.

[beckchr]

Hi, this problem is now fixed in the svn tree. Will be in jfreechart 1.10.

Thanks for your feedback.

Hi, we should remove the block
if (this.chart != null) {
....
}
completely from the constructor, as the same code appears in setChart(), which is called anyway.