Fixed IEditorPart reference (memory) leaks from ResourceEditorSupport

ResourceEditorSupport.validationRequest gave IEditorPart as an argument
to UnaryRequest which caused the reference to remain in the DB client's
caches for very long times. This consumed the JVM heap rather quickly
with editors that require a lot of memory and never explicitly free
(nullify) it.

refs #6936

Change-Id: I46bfd107d57dea0493653afdb378dc089fa508a2

diff --git a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/DefaultExplorerSelectionListener.java b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/DefaultExplorerSelectionListener.java
index 407fbff3ba8fa750ae823c4148aa71c131d9b129..713b28297e67856ba0841f263aeb12a9f0028aa3 100644 (file)
--- a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/DefaultExplorerSelectionListener.java
+++ b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/DefaultExplorerSelectionListener.java
@@ -15,9 +15,10 @@ import org.eclipse.jface.viewers.IPostSelectionProvider;
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.ui.ISelectionListener;
 import org.eclipse.ui.IWorkbenchPart;
+import org.eclipse.ui.services.IDisposable;
 import org.simantics.browsing.ui.GraphExplorer;
 
-public class DefaultExplorerSelectionListener implements ISelectionListener {
+public class DefaultExplorerSelectionListener implements ISelectionListener, IDisposable {
 
     private static final boolean DEBUG = false;
 
@@ -35,6 +36,13 @@ public class DefaultExplorerSelectionListener implements ISelectionListener {
         this.filter = filter;
     }
 
+    @Override
+    public void dispose() {
+        owner = null;
+        explorer = null;
+        filter = null;
+    }
+
     @Override
     public void selectionChanged(IWorkbenchPart part, ISelection selection) {
         // Always disregard own selections.
@@ -44,6 +52,7 @@ public class DefaultExplorerSelectionListener implements ISelectionListener {
         if (DEBUG)
             System.out.println("[" + owner + "] workbench selection changed: part=" + part + ", selection=" + selection);
         ISelection s = selection;
+        WorkbenchSelectionFilter filter = this.filter;
         if (filter != null)
             s = filter.filterSelection(part, selection);
 
@@ -52,9 +61,12 @@ public class DefaultExplorerSelectionListener implements ISelectionListener {
                 System.out.println("** [" + owner + "] workbench selection changed: part=" + part + ", selection=" + selection);
                 System.out.println("    SETTING NEW SELECTION");
             }
-            if(!explorer.isDisposed()) {
+            GraphExplorer explorer = this.explorer;
+            if(explorer != null && !explorer.isDisposed()) {
                 IPostSelectionProvider selectionProvider = (IPostSelectionProvider)explorer.getAdapter(IPostSelectionProvider.class);
-                selectionProvider.setSelection(selection);
+                if (selectionProvider != null) {
+                    selectionProvider.setSelection(selection);
+                }
             }
         } else {
             if (DEBUG)

diff --git a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerViewBase.java b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerViewBase.java
index 72307b32f22ec8371841ed96ef4c469e7d85453a..4b1d8ffef94c1e59d4756ddb994bfd3b293df6af 100644 (file)
--- a/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerViewBase.java
+++ b/bundles/org.simantics.browsing.ui.swt/src/org/simantics/browsing/ui/swt/GraphExplorerViewBase.java
@@ -27,6 +27,7 @@ import org.eclipse.ui.IViewSite;
 import org.eclipse.ui.IWorkbenchPart;
 import org.eclipse.ui.PartInitException;
 import org.eclipse.ui.part.ViewPart;
+import org.eclipse.ui.services.IDisposable;
 import org.simantics.browsing.ui.Column;
 import org.simantics.browsing.ui.GraphExplorer;
 import org.simantics.browsing.ui.NodeContext;
@@ -298,6 +299,8 @@ public abstract class GraphExplorerViewBase extends ViewPart {
         // Remember to remove the installed workbench selection listener
         if (workbenchSelectionListener != null) {
             getSite().getWorkbenchWindow().getSelectionService().removePostSelectionListener(workbenchSelectionListener);
+            if (workbenchSelectionListener instanceof IDisposable)
+                ((IDisposable) workbenchSelectionListener).dispose();
             workbenchSelectionListener = null;
 
             getSite().setSelectionProvider(null);
@@ -520,16 +523,16 @@ public abstract class GraphExplorerViewBase extends ViewPart {
         return true;
     }
 
-    @SuppressWarnings("rawtypes")
+    @SuppressWarnings("unchecked")
     @Override
-    public Object getAdapter(Class adapter) {
+    public <T> T getAdapter(Class<T> adapter) {
 
         if (GraphExplorer.class == adapter)
-            return explorer;
+            return (T) explorer;
         else if(ISessionContextProvider.class == adapter)
-            return getSessionContextProvider();
+            return (T) getSessionContextProvider();
         else if(IPropertyPage.class == adapter)
-            return getPropertyPage();
+            return (T) getPropertyPage();
 
         return super.getAdapter(adapter);
 

diff --git a/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorInput2.java b/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorInput2.java
index e1ec2f52106cbbcfc4a0ae1d52157e8bf309702b..d40197722a239f9752c68547de0230d87fcdcbde 100644 (file)
--- a/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorInput2.java
+++ b/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorInput2.java
@@ -56,7 +56,7 @@ import org.simantics.utils.ui.workbench.StringMemento;
 public class ResourceEditorInput2 extends PlatformObject implements IResourceEditorInput2, IPersistableElement {
 
     private final static boolean          DEBUG_EXISTS    = false;
-    private final static boolean          DEBUG_UPDATE    = false;
+    private final static boolean          DEBUG_UPDATE    = true;
 
     private static final String           NO_NAME         = ResourceEditorInput.NO_NAME;
 
@@ -345,9 +345,8 @@ public class ResourceEditorInput2 extends PlatformObject implements IResourceEdi
     /* (non-Javadoc)
      * @see org.eclipse.core.runtime.IAdaptable#getAdapter(java.lang.Class)
      */
-    @SuppressWarnings("rawtypes")
     @Override
-    public Object getAdapter(Class adapter) {
+    public <T> T getAdapter(Class<T> adapter) {
         //System.out.println("[ResourceEditorInput] getAdapter: " + adapter.getName());
         return null;
     }
@@ -421,7 +420,7 @@ public class ResourceEditorInput2 extends PlatformObject implements IResourceEdi
             System.out.println("update(" + this + ")");
 
         try {
-            assertExists(g);
+            //assertExists(g);
 
             name = g.syncRequest(new TitleRequest(editorID, this));
             if (name == null)

diff --git a/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorSupport.java b/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorSupport.java
index 54a697a72422d191f601dd537ae6031ae5d78dbc..4075e1e11efaec0a6b3161db1bf9cf2b3f67bdba 100644 (file)
--- a/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorSupport.java
+++ b/bundles/org.simantics.ui/src/org/simantics/ui/workbench/ResourceEditorSupport.java
@@ -21,19 +21,20 @@ import org.simantics.db.ReadGraph;
 import org.simantics.db.Session;
 import org.simantics.db.common.procedure.adapter.ListenerAdapter;
 import org.simantics.db.common.request.ParametrizedRead;
-import org.simantics.db.common.request.UnaryRead;
+import org.simantics.db.common.request.UniqueRead;
 import org.simantics.db.event.ChangeEvent;
 import org.simantics.db.event.ChangeListener;
 import org.simantics.db.exception.DatabaseException;
 import org.simantics.db.management.ISessionContext;
 import org.simantics.db.management.ISessionContextProvider;
-import org.simantics.db.request.Read;
 import org.simantics.db.service.GraphChangeListenerSupport;
 import org.simantics.ui.SimanticsUI;
 import org.simantics.utils.datastructures.map.Tuple;
 import org.simantics.utils.ui.ExceptionUtils;
 import org.simantics.utils.ui.SWTUtils;
 import org.simantics.utils.ui.workbench.WorkbenchUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * A helper class for easing the attachment of a Simantics database session to
@@ -50,6 +51,9 @@ import org.simantics.utils.ui.workbench.WorkbenchUtils;
  */
 public class ResourceEditorSupport implements IAdaptable, ChangeListener {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(ResourceEditorSupport.class);
+    private static final boolean DEBUG = false;
+
     private IEditorPart                             editorPart;
 
     private ChangeListener                          editorPartChangeListener;
@@ -152,7 +156,7 @@ public class ResourceEditorSupport implements IAdaptable, ChangeListener {
             return;
 
         inputListener = new InputListener();
-        getSession().asyncRequest(validationRequest(editorPart), inputListener);
+        getSession().asyncRequest(new ValidationRequest(), inputListener);
     }
 
     public synchronized void deactivateValidation() {
@@ -161,6 +165,7 @@ public class ResourceEditorSupport implements IAdaptable, ChangeListener {
         if (inputListener == null)
             return;
         inputListener.dispose();
+        inputListener = null;
     }
 
     public ISessionContext getSessionContext() {
@@ -175,19 +180,18 @@ public class ResourceEditorSupport implements IAdaptable, ChangeListener {
         return session;
     }
 
-    @SuppressWarnings("rawtypes")
+    @SuppressWarnings("unchecked")
     @Override
-    public Object getAdapter(Class adapter) {
+    public <T> T getAdapter(Class<T> adapter) {
         if (adapter == ISessionContext.class)
-            return getSessionContext();
+            return (T) getSessionContext();
         if (adapter == Session.class)
-            return getSession();
+            return (T) getSession();
         return null;
     }
 
     @Override
     public void graphChanged(ChangeEvent e) throws DatabaseException {
-        //System.out.println(this + ": graph change: " + e);
         // Only forward the update to the editor if the input is still valid and
         // the editor implements ChangeListener
         if (editorPart instanceof ChangeListener)
@@ -225,44 +229,55 @@ public class ResourceEditorSupport implements IAdaptable, ChangeListener {
     }
 
     /**
-     * @param input
-     * @return a read request that returns <code>true</code> for valid inputs
-     *         and <code>false</code> for non-existent or invalid inputs.
+     * A read request that returns an {@link Evaluation} of the current state of
+     * <code>editorPart</code>.
+     * 
+     * <p>
+     * This request class is not static but has no parameters that could get
+     * stuck in the database client caches. UniqueRead does not need arguments
+     * and without custom hashCode/equals implementations, each instance of this
+     * request is a different one. This is exactly the behaviour we want in this
+     * case.
      */
-    private Read<Evaluation> validationRequest(IEditorPart editorPart) {
-        return new UnaryRead<IEditorPart, Evaluation>(editorPart) {
-            @Override
-            public Evaluation perform(ReadGraph graph) throws DatabaseException {
-                IEditorInput input = parameter.getEditorInput();
-                IResourceEditorInput resourceInput = getResourceInput(parameter);
-
-                //System.out.println(ResourceEditorSupport.this + ": checking input " + input);
-
-                boolean exists = true;
-                boolean valid = true;
-                if (resourceInput != null) {
-                    exists = resourceInput.exists(graph);
-                    if (exists && inputValidator != null) {
-                        valid = graph.syncRequest(inputValidator.get(resourceInput));
-                    }
-                } else {
-                    exists = input.exists();
+    private class ValidationRequest extends UniqueRead<Evaluation> {
+        @Override
+        public Evaluation perform(ReadGraph graph) throws DatabaseException {
+            IEditorPart part = editorPart;
+            if (part == null)
+                return new Evaluation(null, null, InputState.INVALID, "", "");
+
+            IEditorInput input = part.getEditorInput();
+            IResourceEditorInput resourceInput = getResourceInput(part);
+
+            if (DEBUG)
+                LOGGER.trace("ValidationRequest: checking input " + input);
+
+            boolean exists = true;
+            boolean valid = true;
+            if (resourceInput != null) {
+                exists = resourceInput.exists(graph);
+                if (exists && inputValidator != null) {
+                    valid = graph.syncRequest(inputValidator.get(resourceInput));
                 }
+            } else {
+                exists = input.exists();
+            }
 
-                InputState state = InputState.parse(exists, valid);
-                if (state == InputState.VALID) {
-                    // Make sure any cached data in the editor input is up-to-date.
+            InputState state = InputState.parse(exists, valid);
+            if (state == InputState.VALID) {
+                // Make sure any cached data in the editor input is up-to-date.
+                if (resourceInput != null)
                     resourceInput.update(graph);
-                }
-
-                Evaluation eval = new Evaluation(parameter, input, state, input.getName(), input.getToolTipText());
-                //System.out.println(ResourceEditorSupport.this + ": validation evaluation: " + eval);
-                return eval;
             }
-        };
+
+            Evaluation eval = new Evaluation(part, input, state, input.getName(), input.getToolTipText());
+            if (DEBUG)
+                LOGGER.trace("ValidationRequest: evaluation result: " + eval);
+            return eval;
+        }
     }
 
-    private class InputListener extends ListenerAdapter<Evaluation> {
+    private static class InputListener extends ListenerAdapter<Evaluation> {
 
         private boolean disposed = false;
 
@@ -272,11 +287,11 @@ public class ResourceEditorSupport implements IAdaptable, ChangeListener {
 
         @Override
         public void execute(Evaluation evaluation) {
-            //System.out.println("InputListener: " + evaluation);
+            if (DEBUG)
+                LOGGER.trace("InputListener: " + evaluation);
             switch (evaluation.getInputState()) {
                 case VALID:
                     break;
-
                 case INVALID:
                 case NON_EXISTENT:
                     scheduleEditorClose(evaluation.getEditorPart());
@@ -291,18 +306,15 @@ public class ResourceEditorSupport implements IAdaptable, ChangeListener {
 
         @Override
         public boolean isDisposed() {
-            return disposed || ResourceEditorSupport.this.isDisposed();
+            return disposed;
         }
     }
 
-    private void scheduleEditorClose(final IEditorPart editorPart) {
-        SWTUtils.asyncExec(editorPart.getSite().getShell(), new Runnable() {
-            @Override
-            public void run() {
-                // Don't have to check isDisposed since closeEditor
-                // will ignore already closed editor parts.
-                WorkbenchUtils.closeEditor(editorPart, false);
-            }
+    private static void scheduleEditorClose(IEditorPart editorPart) {
+        SWTUtils.asyncExec(editorPart.getSite().getShell(), () -> {
+            // Don't have to check isDisposed since closeEditor
+            // will ignore already closed editor parts.
+            WorkbenchUtils.closeEditor(editorPart, false);
         });
     }