]> gerrit.simantics Code Review - simantics/sysdyn.git/commitdiff
JFreeChart implementation is leaking memory
authorMarko Luukkainen <marko.luukkainen@semantum.fi>
Wed, 8 Dec 2021 17:56:56 +0000 (19:56 +0200)
committerMarko Luukkainen <marko.luukkainen@semantum.fi>
Wed, 8 Dec 2021 17:56:56 +0000 (19:56 +0200)
Old Plot properties were not diposed when new Plot was set
ChartComposites did not dispose active chart.
Setting object references to null in Plot.dispose() implementations

gitlab #86

bundles/org.simantics.jfreechart/src/org/simantics/jfreechart/chart/AbstractPlot.java
bundles/org.simantics.jfreechart/src/org/simantics/jfreechart/chart/ChartComposite.java
bundles/org.simantics.jfreechart/src/org/simantics/jfreechart/chart/ChartComposite2.java
bundles/org.simantics.jfreechart/src/org/simantics/jfreechart/chart/PiePlot.java

index 600de403702b604e40117753303e94b2acfaadb0..2d5daec8770c87a87862cec81eb14a6398879446 100644 (file)
@@ -58,9 +58,15 @@ public abstract class AbstractPlot implements IPlot {
 
             for(IDataset dataset : currentProperties.datasets)
                 dataset.dispose();
+            currentProperties = null;
         }
-        if(listener != null)
+        if(listener != null) {
             listener.dispose();
+            listener = null;
+        }
+        jfreechart = null;
+        plot = null;
+        
     }
 
     @Override
@@ -83,7 +89,17 @@ public abstract class AbstractPlot implements IPlot {
 
     protected abstract Plot newPlot();
     protected void setPlotProperties(PlotProperties properties) {
-       this.currentProperties = properties;
+               if (currentProperties != null) {
+                       for (IAxis axis : currentProperties.ranges)
+                               axis.dispose();
+
+                       for (IAxis axis : currentProperties.domains)
+                               axis.dispose();
+
+                       for (IDataset dataset : currentProperties.datasets)
+                               dataset.dispose();
+               }
+               this.currentProperties = properties;
     }
     protected abstract void getOtherProperties(ReadGraph graph, PlotProperties properties) throws DatabaseException;
 
index e9088797da3c65def9f3dbc94f411bfafafb6d0c..53cdf1c4cbae51ed482c6a07ef6e342ab68dfc72 100644 (file)
@@ -17,6 +17,8 @@ import org.eclipse.jface.layout.GridDataFactory;
 import org.eclipse.jface.layout.GridLayoutFactory;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.awt.SWT_AWT;
+import org.eclipse.swt.events.DisposeEvent;
+import org.eclipse.swt.events.DisposeListener;
 import org.eclipse.swt.widgets.Composite;
 import org.jfree.chart.ChartPanel;
 import org.jfree.chart.JFreeChart;
@@ -66,6 +68,14 @@ public class ChartComposite extends Composite {
         } catch (DatabaseException e) {
             e.printStackTrace();
         }
+        
+        this.addDisposeListener(new DisposeListener() {
+                       
+                       @Override
+                       public void widgetDisposed(DisposeEvent e) {
+                               doDispose();
+                       }
+               });
     }
 
     /**
@@ -78,6 +88,37 @@ public class ChartComposite extends Composite {
         super(parent, style | SWT.NO_BACKGROUND | SWT.EMBEDDED);
         CreateContent(chartResource);
     }
+    
+    public void doDispose() {
+       if (chart != null) {
+               chart.dispose();
+               chart = null;
+       }
+    }
+    
+    /**
+     * This query does not implement equals or hashCode by purpose. The result is not reusable.
+     * 
+     * @author luukkainen
+     *
+     */
+    protected static class ChartRead implements Read<IJFreeChart> {
+               private Resource chartResource;
+               
+               public ChartRead(Resource chartResource) {
+                       this.chartResource = chartResource;
+               }
+               
+               @Override
+               public IJFreeChart perform(ReadGraph graph) throws DatabaseException {
+                       // Adapt chartResource to a chart (XY, pie, bar, ...)
+            if(graph.isInstanceOf(chartResource, JFreeChartResource.getInstance(graph).Chart)) {
+               return  graph.adapt(chartResource, IJFreeChart.class);
+            } else {
+                return null;
+            }
+               }
+       }
 
     /**
      * Creates and displays the chart defined in chartResource
@@ -91,22 +132,7 @@ public class ChartComposite extends Composite {
         frame = SWT_AWT.new_Frame(composite);
 
         // Add a listener displaying the contents of the chart. Chart is re-drawn if the definition changes
-        Simantics.getSession().asyncRequest(new Read<IJFreeChart>() {
-
-            @Override
-            public IJFreeChart perform(ReadGraph graph) throws DatabaseException {
-                // Adapt chartResource to a chart (XY, pie, bar, ...)
-                if(graph.isInstanceOf(chartResource, JFreeChartResource.getInstance(graph).Chart)) {
-                    if(chart != null)
-                        chart.dispose();
-                    chart = graph.adapt(chartResource, IJFreeChart.class);
-                    return chart;
-                } else {
-                    return null;
-                }
-            }
-
-        } , new AsyncListener<IJFreeChart>() {
+        Simantics.getSession().asyncRequest(new ChartRead(chartResource), new AsyncListener<IJFreeChart>() {
 
             @Override
             public boolean isDisposed() {
@@ -117,7 +143,9 @@ public class ChartComposite extends Composite {
             public void execute(AsyncReadGraph graph, IJFreeChart chart) {
                 if(chart == null || composite.isDisposed())
                     return;
-                
+                if (ChartComposite.this.chart != null)
+                       ChartComposite.this.chart.dispose();
+                ChartComposite.this.chart = chart;
                 JFreeChart jfreeChart = chart.getChart();
                 // Display the result chart
                 if (composite.isDisposed())
index 32ef7c346623e9b7b00ccf47648518f6f8beb61d..0efffbc3d38b43dfc5b88ed7aede36ee7ecd95d2 100644 (file)
@@ -113,18 +113,41 @@ public class ChartComposite2 extends SWTAWTComponent {
                return chart;
        }
        
+       @Override
+       public void doDispose() {
+               if (chart != null) {
+                       chart.dispose();
+                       chart = null;
+               }
+               super.doDispose();
+       }
+       
        protected Read<IJFreeChart> getChartQuery(final Resource chartResource) {
-               return new Read<IJFreeChart>() {
-            @Override
-            public IJFreeChart perform(ReadGraph graph) throws DatabaseException {
-                // Adapt chartResource to a chart (XY, pie, bar, ...)
-                if(graph.isInstanceOf(chartResource, JFreeChartResource.getInstance(graph).Chart)) {
-                       return  graph.adapt(chartResource, IJFreeChart.class);
-                } else {
-                    return null;
-                }
+               return new ChartRead(chartResource);
+       }
+       
+       /**
+        * This query does not implement equals or hashCode by purpose. The result is not reusable.
+        * 
+        * @author luukkainen
+        *
+        */
+       protected static class ChartRead implements Read<IJFreeChart> {
+               private Resource chartResource;
+               
+               public ChartRead(Resource chartResource) {
+                       this.chartResource = chartResource;
+               }
+               
+               @Override
+               public IJFreeChart perform(ReadGraph graph) throws DatabaseException {
+                       // Adapt chartResource to a chart (XY, pie, bar, ...)
+            if(graph.isInstanceOf(chartResource, JFreeChartResource.getInstance(graph).Chart)) {
+               return  graph.adapt(chartResource, IJFreeChart.class);
+            } else {
+                return null;
             }
-        };
+               }
        }
 
     /**
index f06bdbea6f863c201ada5449d737cab913ba0715..af78ae14fbb9da91a99c4aac22952ce5412c5d12 100644 (file)
-/*******************************************************************************\r
- * Copyright (c) 2007, 2011 Association for Decentralized Information Management in\r
- * Industry THTH ry.\r
- * All rights reserved. This program and the accompanying materials\r
- * are made available under the terms of the Eclipse Public License v1.0\r
- * which accompanies this distribution, and is available at\r
- * http://www.eclipse.org/legal/epl-v10.html\r
- *\r
- * Contributors:\r
- *     VTT Technical Research Centre of Finland - initial API and implementation\r
- *******************************************************************************/\r
-package org.simantics.jfreechart.chart;\r
-\r
-import java.awt.Color;\r
-import java.awt.Font;\r
-import java.util.HashMap;\r
-\r
-import org.jfree.chart.labels.StandardPieSectionLabelGenerator;\r
-import org.jfree.chart.labels.StandardPieToolTipGenerator;\r
-import org.jfree.chart.plot.DefaultDrawingSupplier;\r
-import org.jfree.chart.plot.Plot;\r
-import org.jfree.data.general.Dataset;\r
-import org.jfree.data.general.DatasetChangeEvent;\r
-import org.jfree.data.general.DatasetChangeListener;\r
-import org.jfree.ui.RectangleInsets;\r
-import org.simantics.databoard.Bindings;\r
-import org.simantics.db.ReadGraph;\r
-import org.simantics.db.Resource;\r
-import org.simantics.db.exception.DatabaseException;\r
-import org.simantics.sysdyn.JFreeChartResource;\r
-\r
-/**\r
- * Class representing a PiePlot in JFreeChart ontology\r
- * \r
- * @author Teemu Lempinen\r
- *\r
- */\r
-public class PiePlot extends AbstractPlot {\r
-\r
-    protected org.jfree.data.general.PieDataset pieDataset;\r
-    private DatasetChangeListener changeListener;\r
-    \r
-    public PiePlot(ReadGraph graph, Resource resource) {\r
-        super(graph, resource);\r
-    }\r
-\r
-    /**\r
-     * Pie plot class with a stricter equals condition\r
-     * @author Teemu Lempinen\r
-     *\r
-     */\r
-    protected static class MyPiePlot extends org.jfree.chart.plot.PiePlot {\r
-\r
-        private static final long serialVersionUID = -5917620061541212934L;\r
-\r
-        @Override\r
-        public boolean equals(Object obj) {\r
-            boolean result = super.equals(obj);\r
-            if(result == true) {\r
-                org.jfree.chart.plot.PiePlot that = (org.jfree.chart.plot.PiePlot) obj;\r
-                if (this.getDataset() != that.getDataset()) {\r
-                    return false; // Normally plot does not check this. We need this to properly update the charts\r
-                }\r
-            }\r
-\r
-            return result;\r
-        }\r
-\r
-    }\r
-\r
-    @Override\r
-    protected Plot newPlot() {\r
-        MyPiePlot plot = new MyPiePlot();\r
-        plot.setToolTipGenerator(new StandardPieToolTipGenerator());\r
-        return plot;\r
-    }\r
-\r
-    @Override\r
-    protected void getOtherProperties(ReadGraph graph, PlotProperties properties) throws DatabaseException {\r
-        Boolean labelsVisible = graph.getPossibleRelatedValue(resource, JFreeChartResource.getInstance(graph).Plot_visibleLabels, Bindings.BOOLEAN);\r
-        properties.otherProperties.put("labelsVisible", labelsVisible);\r
-        \r
-        Boolean useFilter = graph.getPossibleRelatedValue(resource, JFreeChartResource.getInstance(graph).Filter_used, Bindings.BOOLEAN);\r
-        Double  filterFraction = graph.getPossibleRelatedValue(resource, JFreeChartResource.getInstance(graph).Filter_fraction, Bindings.DOUBLE);\r
-        properties.otherProperties.put("useFilter", useFilter);\r
-        properties.otherProperties.put("filterFraction", filterFraction);\r
-    }\r
-    \r
-    @SuppressWarnings({ "unchecked", "rawtypes" })\r
-    @Override\r
-    protected void setPlotProperties(PlotProperties properties) {\r
-        if(!(plot instanceof org.jfree.chart.plot.PiePlot))\r
-            return;\r
-        \r
-        final org.jfree.chart.plot.PiePlot piePlot = (org.jfree.chart.plot.PiePlot)plot;\r
-        \r
-        if(!properties.datasets.isEmpty()) {\r
-            // We assume that a pie plot has only one dataset\r
-            final IDataset ds = properties.datasets.get(0);\r
-            Dataset dataset = ((PieDataset)ds).getDataset();\r
-            \r
-            if(dataset == null)\r
-                return;\r
-\r
-            if(pieDataset != null && changeListener != null) {\r
-                pieDataset.removeChangeListener(changeListener);\r
-            }\r
-            \r
-            pieDataset = (org.jfree.data.general.PieDataset)dataset;\r
-            piePlot.setDataset(pieDataset);\r
-            \r
-            if (pieDataset instanceof FilteredDataset) {\r
-               FilteredDataset f = (FilteredDataset)pieDataset;\r
-               Boolean useFilter = (Boolean)properties.otherProperties.get("useFilter");\r
-                Double filterFraction = (Double)properties.otherProperties.get("filterFraction");\r
-                if (useFilter != null && filterFraction != null) {\r
-                       f.setFiltering(useFilter);\r
-                       f.setFilterFraction(filterFraction*0.01);\r
-                       f.updateFiltered();\r
-                } else {\r
-                       f.setFiltering(false);\r
-                }\r
-            }\r
-            \r
-            Boolean labelsVisible = (Boolean)properties.otherProperties.get("labelsVisible");\r
-            if(Boolean.FALSE.equals(labelsVisible))\r
-                piePlot.setLabelGenerator(null);\r
-            else if(piePlot.getLabelGenerator() == null)\r
-                piePlot.setLabelGenerator(new StandardPieSectionLabelGenerator());\r
-            \r
-            changeListener = new DatasetChangeListener() {\r
-                \r
-                               @Override\r
-                public void datasetChanged(DatasetChangeEvent event) {\r
-                    HashMap<Comparable<?>, Color> colorMap = ((PieDataset)ds).getColorMap();\r
-                    HashMap<Comparable<?>, Boolean> explodedMap = ((PieDataset)ds).getExplodedMap();\r
-                    \r
-                    for(Object o : piePlot.getDataset().getKeys()) {\r
-                        if(o instanceof Comparable) {\r
-                            Comparable<?> key = (Comparable<?>)o;\r
-                            if(explodedMap.containsKey(key) && explodedMap.get(key)) {\r
-                                piePlot.setExplodePercent(key, 0.3);\r
-\r
-                            } else {\r
-                                piePlot.setExplodePercent(key, 0);\r
-                            }\r
-                        }\r
-                    }\r
-                    \r
-                    for(Comparable<?> name : explodedMap.keySet()) {\r
-                        Boolean exploded = explodedMap.get(name);\r
-                        if(Boolean.TRUE.equals(exploded))\r
-                            piePlot.setExplodePercent(name, 0.3);\r
-                    } \r
-                    piePlot.clearSectionPaints(false);\r
-                    piePlot.setDrawingSupplier(new DefaultDrawingSupplier());\r
-                    for(Comparable<?> name : colorMap.keySet())\r
-                        piePlot.setSectionPaint(name, colorMap.get(name));\r
-                }\r
-            };\r
-            \r
-            pieDataset.addChangeListener(changeListener);\r
-        }\r
-        \r
-        // Cleaner look: no outline borders\r
-        piePlot.setInsets(new RectangleInsets(0,0,0,0), false);\r
-        piePlot.setOutlineVisible(false);\r
-        piePlot.setLabelBackgroundPaint(Color.WHITE);\r
-        piePlot.setLabelFont(new Font("helvetica", Font.PLAIN, 11));\r
-        super.setPlotProperties(properties);\r
-    }\r
-\r
-}\r
+/*******************************************************************************
+ * Copyright (c) 2007, 2011 Association for Decentralized Information Management in
+ * Industry THTH ry.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     VTT Technical Research Centre of Finland - initial API and implementation
+ *******************************************************************************/
+package org.simantics.jfreechart.chart;
+
+import java.awt.Color;
+import java.awt.Font;
+import java.util.HashMap;
+
+import org.jfree.chart.labels.StandardPieSectionLabelGenerator;
+import org.jfree.chart.labels.StandardPieToolTipGenerator;
+import org.jfree.chart.plot.DefaultDrawingSupplier;
+import org.jfree.chart.plot.Plot;
+import org.jfree.data.general.Dataset;
+import org.jfree.data.general.DatasetChangeEvent;
+import org.jfree.data.general.DatasetChangeListener;
+import org.jfree.ui.RectangleInsets;
+import org.simantics.databoard.Bindings;
+import org.simantics.db.ReadGraph;
+import org.simantics.db.Resource;
+import org.simantics.db.exception.DatabaseException;
+import org.simantics.sysdyn.JFreeChartResource;
+
+/**
+ * Class representing a PiePlot in JFreeChart ontology
+ * 
+ * @author Teemu Lempinen
+ *
+ */
+public class PiePlot extends AbstractPlot {
+
+    protected org.jfree.data.general.PieDataset pieDataset;
+    private DatasetChangeListener changeListener;
+    
+    public PiePlot(ReadGraph graph, Resource resource) {
+        super(graph, resource);
+    }
+
+    /**
+     * Pie plot class with a stricter equals condition
+     * @author Teemu Lempinen
+     *
+     */
+    protected static class MyPiePlot extends org.jfree.chart.plot.PiePlot {
+
+        private static final long serialVersionUID = -5917620061541212934L;
+
+        @Override
+        public boolean equals(Object obj) {
+            boolean result = super.equals(obj);
+            if(result == true) {
+                org.jfree.chart.plot.PiePlot that = (org.jfree.chart.plot.PiePlot) obj;
+                if (this.getDataset() != that.getDataset()) {
+                    return false; // Normally plot does not check this. We need this to properly update the charts
+                }
+            }
+
+            return result;
+        }
+
+    }
+
+    @Override
+    protected Plot newPlot() {
+        MyPiePlot plot = new MyPiePlot();
+        plot.setToolTipGenerator(new StandardPieToolTipGenerator());
+        return plot;
+    }
+    
+    @Override
+    public void dispose() {
+       pieDataset = null;
+       changeListener = null;
+       super.dispose();
+    }
+
+    @Override
+    protected void getOtherProperties(ReadGraph graph, PlotProperties properties) throws DatabaseException {
+        Boolean labelsVisible = graph.getPossibleRelatedValue(resource, JFreeChartResource.getInstance(graph).Plot_visibleLabels, Bindings.BOOLEAN);
+        properties.otherProperties.put("labelsVisible", labelsVisible);
+        
+        Boolean useFilter = graph.getPossibleRelatedValue(resource, JFreeChartResource.getInstance(graph).Filter_used, Bindings.BOOLEAN);
+        Double  filterFraction = graph.getPossibleRelatedValue(resource, JFreeChartResource.getInstance(graph).Filter_fraction, Bindings.DOUBLE);
+        properties.otherProperties.put("useFilter", useFilter);
+        properties.otherProperties.put("filterFraction", filterFraction);
+    }
+    
+    @SuppressWarnings({ "unchecked", "rawtypes" })
+    @Override
+    protected void setPlotProperties(PlotProperties properties) {
+        if(!(plot instanceof org.jfree.chart.plot.PiePlot))
+            return;
+        
+        final org.jfree.chart.plot.PiePlot piePlot = (org.jfree.chart.plot.PiePlot)plot;
+        
+        if(!properties.datasets.isEmpty()) {
+            // We assume that a pie plot has only one dataset
+            final IDataset ds = properties.datasets.get(0);
+            Dataset dataset = ((PieDataset)ds).getDataset();
+            
+            if(dataset == null)
+                return;
+
+            if(pieDataset != null && changeListener != null) {
+                pieDataset.removeChangeListener(changeListener);
+            }
+            
+            pieDataset = (org.jfree.data.general.PieDataset)dataset;
+            piePlot.setDataset(pieDataset);
+            
+            if (pieDataset instanceof FilteredDataset) {
+               FilteredDataset f = (FilteredDataset)pieDataset;
+               Boolean useFilter = (Boolean)properties.otherProperties.get("useFilter");
+                Double filterFraction = (Double)properties.otherProperties.get("filterFraction");
+                if (useFilter != null && filterFraction != null) {
+                       f.setFiltering(useFilter);
+                       f.setFilterFraction(filterFraction*0.01);
+                       f.updateFiltered();
+                } else {
+                       f.setFiltering(false);
+                }
+            }
+            
+            Boolean labelsVisible = (Boolean)properties.otherProperties.get("labelsVisible");
+            if(Boolean.FALSE.equals(labelsVisible))
+                piePlot.setLabelGenerator(null);
+            else if(piePlot.getLabelGenerator() == null)
+                piePlot.setLabelGenerator(new StandardPieSectionLabelGenerator());
+            
+            changeListener = new DatasetChangeListener() {
+                
+                               @Override
+                public void datasetChanged(DatasetChangeEvent event) {
+                    HashMap<Comparable<?>, Color> colorMap = ((PieDataset)ds).getColorMap();
+                    HashMap<Comparable<?>, Boolean> explodedMap = ((PieDataset)ds).getExplodedMap();
+                    
+                    for(Object o : piePlot.getDataset().getKeys()) {
+                        if(o instanceof Comparable) {
+                            Comparable<?> key = (Comparable<?>)o;
+                            if(explodedMap.containsKey(key) && explodedMap.get(key)) {
+                                piePlot.setExplodePercent(key, 0.3);
+
+                            } else {
+                                piePlot.setExplodePercent(key, 0);
+                            }
+                        }
+                    }
+                    
+                    for(Comparable<?> name : explodedMap.keySet()) {
+                        Boolean exploded = explodedMap.get(name);
+                        if(Boolean.TRUE.equals(exploded))
+                            piePlot.setExplodePercent(name, 0.3);
+                    } 
+                    piePlot.clearSectionPaints(false);
+                    piePlot.setDrawingSupplier(new DefaultDrawingSupplier());
+                    for(Comparable<?> name : colorMap.keySet())
+                        piePlot.setSectionPaint(name, colorMap.get(name));
+                }
+            };
+            
+            pieDataset.addChangeListener(changeListener);
+        }
+        
+        // Cleaner look: no outline borders
+        piePlot.setInsets(new RectangleInsets(0,0,0,0), false);
+        piePlot.setOutlineVisible(false);
+        piePlot.setLabelBackgroundPaint(Color.WHITE);
+        piePlot.setLabelFont(new Font("helvetica", Font.PLAIN, 11));
+        super.setPlotProperties(properties);
+    }
+
+}