Bug 336948 - Potential leak of ContextInjectionListener
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ClassRequestor.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ClassRequestor.java
index 60bbf2b..f8b32a8 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ClassRequestor.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ClassRequestor.java
@@ -38,7 +38,7 @@
}
public Object execute() throws InjectionException {
- // do nothing
+ clearResolvedArgs();
return null;
}
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java
index 1000ee4..d36abb2 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java
@@ -56,6 +56,7 @@
constructor.setAccessible(false);
if (pausedRecording)
primarySupplier.resumeRecoding();
+ clearResolvedArgs();
}
return result;
}
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/FieldRequestor.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/FieldRequestor.java
index 6110270..d6a7a2b 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/FieldRequestor.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/FieldRequestor.java
@@ -26,7 +26,10 @@
}
public Object execute() throws InjectionException {
+ if (actualArgs == null)
+ return null; // optional field
setField(field, actualArgs[0]);
+ clearResolvedArgs();
return null;
}
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java
index f3fea0d..89346d7 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java
@@ -34,6 +34,10 @@
}
public Object execute() throws InjectionException {
+ if (actualArgs == null) {
+ if (method.getParameterTypes().length > 0)
+ return null; // optional method call
+ }
Object userObject = getRequestingObject();
if (userObject == null)
return null;
@@ -62,6 +66,7 @@
method.setAccessible(false);
if (pausedRecording)
primarySupplier.resumeRecoding();
+ clearResolvedArgs();
}
return result;
}
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 1ec1e6c..f5f8bce 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
@@ -128,4 +128,18 @@
objectDescriptors = calcDependentObjects();
return objectDescriptors;
}
+
+ /**
+ * Don't hold on to the resolved results as it will prevent
+ * them from being garbage collected.
+ */
+ protected void clearResolvedArgs() {
+ if (actualArgs == null)
+ return;
+ for (int i = 0; i < actualArgs.length; i++) {
+ actualArgs[i] = null;
+ }
+ actualArgs = null;
+ return;
+ }
}
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/InjectionResultLeakTest.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/InjectionResultLeakTest.java
new file mode 100644
index 0000000..7933b52
--- /dev/null
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/InjectionResultLeakTest.java
@@ -0,0 +1,75 @@
+/*******************************************************************************
+ * Copyright (c) 2011 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * IBM Corporation - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.e4.core.internal.tests.di;
+
+import java.lang.ref.WeakReference;
+import javax.inject.Inject;
+import javax.inject.Named;
+import junit.framework.TestCase;
+import org.eclipse.e4.core.contexts.ContextInjectionFactory;
+import org.eclipse.e4.core.contexts.EclipseContextFactory;
+import org.eclipse.e4.core.contexts.IEclipseContext;
+import org.eclipse.e4.core.di.annotations.Optional;
+
+/**
+ * Test that we don't hold on to the values calculated during injection.
+ * This test relies on VM performing garbage collection in System.gc().
+ * The actual VM processing of GC requests is up to the implementation
+ * so this test might not work on all VMs or might become invalid on
+ * future VMs.
+ */
+public class InjectionResultLeakTest extends TestCase {
+
+ static class PartConsumer {
+ Object part;
+
+ @Inject
+ void setPart(@Optional @Named("testGC") Object part) {
+ this.part = part;
+ }
+ }
+
+ public void testLeaks() {
+ IEclipseContext context = EclipseContextFactory.create();
+ WeakReference<?> ref;
+
+ { // scope the "part" object to help GC
+ Object part = new Object();
+ ref = new WeakReference<Object>(part);
+ assertEquals(part, ref.get());
+
+ context.set("testGC", part);
+
+ PartConsumer consumer = ContextInjectionFactory.make(
+ PartConsumer.class, context);
+ assertEquals(part, consumer.part);
+
+ part = null; // another "let's help GC" statement
+ context.remove("testGC");
+
+ assertNull(consumer.part);
+ }
+
+ // gc a few times
+ System.runFinalization();
+ System.gc();
+ System.runFinalization();
+ System.gc();
+ System.runFinalization();
+ System.gc();
+ System.runFinalization();
+ System.gc();
+
+ // partA should have been gc'd
+ assertNull("The object should have been garbage collected", ref.get());
+ }
+
+}
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java
index 3d6b29e..4b6dbca 100644
--- a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java
@@ -44,6 +44,7 @@
import org.eclipse.e4.core.internal.tests.di.InjectArraysTest;
import org.eclipse.e4.core.internal.tests.di.InjectBaseTypeTest;
import org.eclipse.e4.core.internal.tests.di.InjectionOrderTest;
+import org.eclipse.e4.core.internal.tests.di.InjectionResultLeakTest;
import org.eclipse.e4.core.internal.tests.di.InvokeTest;
import org.eclipse.e4.core.internal.tests.di.extensions.InjectionEventTest;
import org.eclipse.e4.core.internal.tests.di.extensions.InjectionMixedSuppliersTest;
@@ -63,6 +64,7 @@
addTestSuite(InjectionOrderTest.class);
addTestSuite(InvokeTest.class);
addTestSuite(InjectBaseTypeTest.class);
+ addTestSuite(InjectionResultLeakTest.class);
addTest(AtInjectTest.suite());
// Contexts
