Bug 378444 - Eclipse Context - no gc for unused TrackableComputationExtv20120627-124442

objects

7 files changed, 93 insertions, 15 deletions

diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ContextObjectSupplier.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ContextObjectSupplier.java
index c54caf337..35e2e7c53 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ContextObjectSupplier.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ContextObjectSupplier.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2011 IBM Corporation and others.
+ * Copyright (c) 2009, 2012 IBM Corporation and others.
  * 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
@@ -10,6 +10,8 @@
  *******************************************************************************/
 package org.eclipse.e4.core.internal.contexts;
 
+import java.lang.ref.Reference;
+import java.lang.ref.WeakReference;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.Stack;
@@ -21,6 +23,7 @@ import org.eclipse.e4.core.di.IInjector;
 import org.eclipse.e4.core.di.suppliers.IObjectDescriptor;
 import org.eclipse.e4.core.di.suppliers.IRequestor;
 import org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier;
+import org.eclipse.e4.core.internal.di.Requestor;
 
 public class ContextObjectSupplier extends PrimaryObjectSupplier {
 
@@ -43,6 +46,12 @@ public class ContextObjectSupplier extends PrimaryObjectSupplier {
 			this.context = context;
 		}
 
+		public Reference<Object> getReference() {
+			if (requestor instanceof Requestor)
+				return ((Requestor) requestor).getReference();
+			return super.getReference();
+		}
+
 		public boolean update(IEclipseContext eventsContext, int eventType, Object[] extraArguments) {
 			if (eventType == ContextChangeEvent.INITIAL) {
 				// needs to be done inside runnable to establish dependencies
@@ -52,7 +61,9 @@ public class ContextObjectSupplier extends PrimaryObjectSupplier {
 					IEclipseContext targetContext = (active[i]) ? context.getActiveLeaf() : context;
 					if (ECLIPSE_CONTEXT_NAME.equals(keys[i])) {
 						result[i] = targetContext;
-						targetContext.getParent(); // creates pseudo-link
+						IEclipseContext parent = targetContext.getParent(); // creates pseudo-link
+						if (parent == null)
+							targetContext.get(ECLIPSE_CONTEXT_NAME); // pseudo-link in case there is no parent
 					} else if (targetContext.containsKey(keys[i]))
 						result[i] = targetContext.get(keys[i]);
 				}
@@ -226,4 +237,11 @@ public class ContextObjectSupplier extends PrimaryObjectSupplier {
 		return objectSupplier;
 	}
 
+	public WeakReference<Object> makeReference(Object object) {
+		if (context instanceof EclipseContext) {
+			return ((EclipseContext) context).trackedWeakReference(object);
+		}
+		return super.makeReference(object);
+	}
+
 }
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
index 55d921f38..253d9c1e5 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
@@ -10,6 +10,8 @@
  *******************************************************************************/
 package org.eclipse.e4.core.internal.contexts;
 
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -75,7 +77,6 @@ public class EclipseContext implements IEclipseContext {
 
 	private Map<String, HashSet<Computation>> listeners = Collections.synchronizedMap(new HashMap<String, HashSet<Computation>>(10, 0.8f));
 	private Map<String, ValueComputation> localValueComputations = Collections.synchronizedMap(new HashMap<String, ValueComputation>());
-	private Set<Computation> activeRATs = new HashSet<Computation>();
 
 	final protected Map<String, Object> localValues = Collections.synchronizedMap(new HashMap<String, Object>());
 
@@ -89,6 +90,13 @@ public class EclipseContext implements IEclipseContext {
 
 	static private ThreadLocal<Stack<Computation>> currentComputation = new ThreadLocal<Stack<Computation>>();
 
+	// I don't think we need to sync referenceQueue access
+	private ReferenceQueue<Object> referenceQueue = new ReferenceQueue<Object>();
+
+	private Map<Reference<?>, TrackableComputationExt> activeComputations = Collections.synchronizedMap(new HashMap<Reference<?>, TrackableComputationExt>());
+
+	private final static Object[] nullArgs = new Object[] {null};
+
 	/**
 	 * A context key (value "activeChildContext") that identifies another {@link IEclipseContext}
 	 * that is a child of the context. The meaning of active is up to the application.
@@ -152,12 +160,12 @@ public class EclipseContext implements IEclipseContext {
 			childContext.dispose();
 		}
 
+		activeComputations.clear();
+
 		ContextChangeEvent event = new ContextChangeEvent(this, ContextChangeEvent.DISPOSE, null, null, null);
 		Set<Scheduled> scheduled = new LinkedHashSet<Scheduled>();
 		Set<Computation> allComputations = getListeners();
 		listeners.clear();
-		allComputations.addAll(activeRATs);
-		activeRATs.clear();
 		for (Computation computation : allComputations) {
 			computation.handleInvalid(event, scheduled);
 		}
@@ -289,13 +297,15 @@ public class EclipseContext implements IEclipseContext {
 		TrackableComputationExt computation = new TrackableComputationExt(runnable, this);
 		ContextChangeEvent event = new ContextChangeEvent(this, ContextChangeEvent.INITIAL, null, null, null);
 		boolean result = computation.update(event);
-		if (result)
-			activeRATs.add(computation);
+		if (result) {
+			Reference<Object> ref = computation.getReference();
+			if (ref != null)
+				activeComputations.put(ref, computation);
+		}
 	}
 
 	public void removeRAT(Computation computation) {
-		activeRATs.remove(computation);
-		// also remove from listeners
+		// remove from listeners
 		Collection<HashSet<Computation>> allListeners = listeners.values();
 		for (HashSet<Computation> group : allListeners) {
 			group.remove(computation);
@@ -321,6 +331,18 @@ public class EclipseContext implements IEclipseContext {
 			invalidate(name, ContextChangeEvent.ADDED, oldValue, value, scheduled);
 			processScheduled(scheduled);
 		}
+
+		// cleanup unused computation listeners
+		Reference<?> ref = referenceQueue.poll();
+		if (ref != null) {
+			ContextChangeEvent event = new ContextChangeEvent(this, ContextChangeEvent.UNINJECTED, nullArgs, null, null);
+			for (; ref != null; ref = referenceQueue.poll()) {
+				TrackableComputationExt obsoleteComputation = activeComputations.remove(ref);
+				if (obsoleteComputation == null)
+					continue;
+				obsoleteComputation.update(event);
+			}
+		}
 	}
 
 	public void modify(String name, Object value) {
@@ -733,4 +755,8 @@ public class EclipseContext implements IEclipseContext {
 		trackAccess(internalName);
 		return internalGet(this, internalName, true);
 	}
+
+	public WeakReference<Object> trackedWeakReference(Object object) {
+		return new WeakReference<Object>(object, referenceQueue);
+	}
 }
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/RunAndTrackExt.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/RunAndTrackExt.java
index 614b24e79..088c7dd59 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/RunAndTrackExt.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/RunAndTrackExt.java
@@ -11,6 +11,7 @@
 
 package org.eclipse.e4.core.internal.contexts;
 
+import java.lang.ref.Reference;
 import org.eclipse.e4.core.contexts.IEclipseContext;
 import org.eclipse.e4.core.contexts.RunAndTrack;
 
@@ -27,5 +28,9 @@ abstract public class RunAndTrackExt extends RunAndTrack {
 		return group;
 	}
 
+	public Reference<Object> getReference() {
+		return null;
+	}
+
 	abstract public boolean update(IEclipseContext eventsContext, int eventType, Object[] extraArguments);
 }
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java
index 61d93d8b5..38278d77e 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/TrackableComputationExt.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2010 IBM Corporation and others.
+ * Copyright (c) 2009, 2012 IBM Corporation and others.
  * 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
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.e4.core.internal.contexts;
 
+import java.lang.ref.Reference;
 import java.util.Set;
 import org.eclipse.e4.core.contexts.IEclipseContext;
 import org.eclipse.e4.core.contexts.RunAndTrack;
@@ -130,4 +131,10 @@ public class TrackableComputationExt extends Computation {
 		return runnable.toString();
 	}
 
+	public Reference<Object> getReference() {
+		if (runnable instanceof RunAndTrackExt)
+			return ((RunAndTrackExt) runnable).getReference();
+		return null;
+	}
+
 }
diff --git a/bundles/org.eclipse.e4.core.di/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.core.di/META-INF/MANIFEST.MF
index 3200f8cb3..ecd64d260 100644
--- a/bundles/org.eclipse.e4.core.di/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.core.di/META-INF/MANIFEST.MF
@@ -23,7 +23,7 @@ Export-Package: org.eclipse.e4.core.di;
    org.eclipse.e4.ui.workbench.swt,
    org.eclipse.ui.workbench",
  org.eclipse.e4.core.di.suppliers;x-friends:="org.eclipse.e4.core.contexts,org.eclipse.e4.core.di.extensions,org.eclipse.e4.ui.di",
- org.eclipse.e4.core.internal.di;x-internal:=true,
+ org.eclipse.e4.core.internal.di;x-friends:="org.eclipse.e4.core.contexts",
  org.eclipse.e4.core.internal.di.osgi;x-internal:=true,
  org.eclipse.e4.core.internal.di.shared;x-friends:="org.eclipse.e4.core.contexts,org.eclipse.e4.core.di.extensions"
 Require-Bundle: org.eclipse.osgi;bundle-version="3.6.0";resolution:=optional
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/di/suppliers/PrimaryObjectSupplier.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/di/suppliers/PrimaryObjectSupplier.java
index 876bb491b..1de4a4038 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/di/suppliers/PrimaryObjectSupplier.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/di/suppliers/PrimaryObjectSupplier.java
@@ -10,6 +10,8 @@
  *******************************************************************************/
 package org.eclipse.e4.core.di.suppliers;
 
+import java.lang.ref.WeakReference;
+
 /**
  * The base class for an "object supplier" - something that knows how to instantiate objects
  * corresponding to the object descriptor.
@@ -56,4 +58,16 @@ abstract public class PrimaryObjectSupplier {
 	 */
 	abstract public void resumeRecording();
 
+	/**
+	 * Creates a new reference to the object.
+	 * <p> 
+	 * Suppliers may override to provide improved memory management, for instance, by 
+	 * to tracking references with reference queues.
+	 * </p>
+	 * @param object the referred object
+	 * @return a new weak reference to the object
+	 */
+	public WeakReference<Object> makeReference(Object object) {
+		return new WeakReference<Object>(object);
+	}
 }
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/Requestor.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/Requestor.java
index e1d215eaa..41552b28a 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/Requestor.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/Requestor.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2011 IBM Corporation and others.
+ * Copyright (c) 2010, 2012 IBM Corporation and others.
  * 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
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.e4.core.internal.di;
 
+import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.AccessibleObject;
 import org.eclipse.e4.core.di.IInjector;
@@ -43,9 +44,12 @@ abstract public class Requestor implements IRequestor {
 		this.injector = injector;
 		this.primarySupplier = primarySupplier;
 		this.tempSupplier = tempSupplier;
-		if (requestingObject != null)
-			objectRef = new WeakReference<Object>(requestingObject);
-		else
+		if (requestingObject != null) {
+			if (primarySupplier != null)
+				objectRef = primarySupplier.makeReference(requestingObject);
+			else
+				objectRef = new WeakReference<Object>(requestingObject);
+		} else
 			objectRef = null;
 		this.track = track;
 		groupUpdates = (reflectionObject == null) ? false : reflectionObject.isAnnotationPresent(GroupUpdates.class);
@@ -81,6 +85,10 @@ abstract public class Requestor implements IRequestor {
 		return object.getClass();
 	}
 
+	public Reference<Object> getReference() {
+		return objectRef;
+	}
+
 	/**
 	 * Determines if the requestor wants to be called whenever one of the dependent object changes.
 	 */