Bug 378444 - Eclipse Context - no gc for unused TrackableComputationExt
objects
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 c54caf3..35e2e7c 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.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 @@
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 @@
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 @@
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 55d921f..253d9c1 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 614b24e..088c7dd 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 @@
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 61d93d8..38278d7 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 @@
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 3200f8c..ecd64d2 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 @@
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 876bb49..1de4a40 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 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 e1d215e..41552b2 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 @@
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 @@
return object.getClass();
}
+ public Reference<Object> getReference() {
+ return objectRef;
+ }
+
/**
* Determines if the requestor wants to be called whenever one of the dependent object changes.
*/
