Bug 334498 - [DI] ContextInjectionFactory.make on nested class within @PostConstruct method of outer class leads to endless recursion
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 1e9ad34..4e51ffa 100644
--- a/bundles/org.eclipse.e4.core.di/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.core.di/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-SymbolicName: org.eclipse.e4.core.di
-Bundle-Version: 0.10.0.qualifier
+Bundle-Version: 1.0.0.qualifier
Bundle-Name: %pluginName
Bundle-Vendor: %providerName
Bundle-Localization: plugin
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/InjectorImpl.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/InjectorImpl.java
index 11339f3..e21bff2 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/InjectorImpl.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/InjectorImpl.java
@@ -74,6 +74,8 @@
private Map<Class<?>, Method[]> methodsCache = new WeakHashMap<Class<?>, Method[]>();
private Map<Class<?>, Map<Method, Boolean>> isOverriddenCache = new WeakHashMap<Class<?>, Map<Method, Boolean>>();
+ private Set<Class<?>> classesBeingCreated = new HashSet<Class<?>>(5);
+
public void inject(Object object, PrimaryObjectSupplier objectSupplier) {
inject(object, objectSupplier, null);
}
@@ -268,55 +270,63 @@
}
private Object internalMake(Class<?> clazz, PrimaryObjectSupplier objectSupplier, PrimaryObjectSupplier tempSupplier) {
- boolean isSingleton = clazz.isAnnotationPresent(Singleton.class);
- if (isSingleton) {
- synchronized (singletonCache) {
- if (singletonCache.containsKey(clazz))
- return singletonCache.get(clazz);
- }
- }
+ if (classesBeingCreated.contains(clazz))
+ throw new InjectionException("Recursive reference trying to create class " + clazz.getName()); //$NON-NLS-1$
+ try {
+ classesBeingCreated.add(clazz);
- Constructor<?>[] constructors = clazz.getDeclaredConstructors();
- // Sort the constructors by descending number of constructor arguments
- ArrayList<Constructor<?>> sortedConstructors = new ArrayList<Constructor<?>>(constructors.length);
- for (Constructor<?> constructor : constructors)
- sortedConstructors.add(constructor);
- Collections.sort(sortedConstructors, new Comparator<Constructor<?>>() {
- public int compare(Constructor<?> c1, Constructor<?> c2) {
- int l1 = c1.getParameterTypes().length;
- int l2 = c2.getParameterTypes().length;
- return l2 - l1;
- }
- });
-
- for (Constructor<?> constructor : sortedConstructors) {
- // skip private and protected constructors; allow public and package visibility
- int modifiers = constructor.getModifiers();
- if (((modifiers & Modifier.PRIVATE) != 0) || ((modifiers & Modifier.PROTECTED) != 0))
- continue;
-
- // unless this is the default constructor, it has to be tagged
- if (!constructor.isAnnotationPresent(Inject.class) && constructor.getParameterTypes().length != 0)
- continue;
-
- ConstructorRequestor requestor = new ConstructorRequestor(constructor, this, objectSupplier, tempSupplier);
- Object[] actualArgs = resolveArgs(requestor, objectSupplier, tempSupplier, false, true, false);
- if (unresolved(actualArgs) != -1)
- continue;
- requestor.setResolvedArgs(actualArgs);
-
- Object newInstance = requestor.execute();
- if (newInstance != null) {
- inject(newInstance, objectSupplier, tempSupplier);
- if (isSingleton) {
- synchronized (singletonCache) { // TBD this is not quite right, synch the method
- singletonCache.put(clazz, newInstance);
- }
+ boolean isSingleton = clazz.isAnnotationPresent(Singleton.class);
+ if (isSingleton) {
+ synchronized (singletonCache) {
+ if (singletonCache.containsKey(clazz))
+ return singletonCache.get(clazz);
}
- return newInstance;
}
+
+ Constructor<?>[] constructors = clazz.getDeclaredConstructors();
+ // Sort the constructors by descending number of constructor arguments
+ ArrayList<Constructor<?>> sortedConstructors = new ArrayList<Constructor<?>>(constructors.length);
+ for (Constructor<?> constructor : constructors)
+ sortedConstructors.add(constructor);
+ Collections.sort(sortedConstructors, new Comparator<Constructor<?>>() {
+ public int compare(Constructor<?> c1, Constructor<?> c2) {
+ int l1 = c1.getParameterTypes().length;
+ int l2 = c2.getParameterTypes().length;
+ return l2 - l1;
+ }
+ });
+
+ for (Constructor<?> constructor : sortedConstructors) {
+ // skip private and protected constructors; allow public and package visibility
+ int modifiers = constructor.getModifiers();
+ if (((modifiers & Modifier.PRIVATE) != 0) || ((modifiers & Modifier.PROTECTED) != 0))
+ continue;
+
+ // unless this is the default constructor, it has to be tagged
+ if (!constructor.isAnnotationPresent(Inject.class) && constructor.getParameterTypes().length != 0)
+ continue;
+
+ ConstructorRequestor requestor = new ConstructorRequestor(constructor, this, objectSupplier, tempSupplier);
+ Object[] actualArgs = resolveArgs(requestor, objectSupplier, tempSupplier, false, true, false);
+ if (unresolved(actualArgs) != -1)
+ continue;
+ requestor.setResolvedArgs(actualArgs);
+
+ Object newInstance = requestor.execute();
+ if (newInstance != null) {
+ inject(newInstance, objectSupplier, tempSupplier);
+ if (isSingleton) {
+ synchronized (singletonCache) { // TBD this is not quite right, synch the method
+ singletonCache.put(clazz, newInstance);
+ }
+ }
+ return newInstance;
+ }
+ }
+ throw new InjectionException("Could not find satisfiable constructor in " + clazz.getName()); //$NON-NLS-1$
+ } finally {
+ classesBeingCreated.remove(clazz);
}
- throw new InjectionException("Could not find satisfiable constructor in " + clazz.getName()); //$NON-NLS-1$
}
public void resolveArguments(IRequestor requestor, boolean initial) {
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/RecursiveObjectCreationTest.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/RecursiveObjectCreationTest.java
new file mode 100644
index 0000000..b7950b3
--- /dev/null
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/RecursiveObjectCreationTest.java
@@ -0,0 +1,88 @@
+/*******************************************************************************
+ * 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 javax.annotation.PostConstruct;
+import javax.inject.Inject;
+
+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.InjectionException;
+
+public class RecursiveObjectCreationTest extends TestCase {
+
+ static public class CheckSelfInject {
+
+ public CheckSelfInject other;
+
+ @Inject
+ public CheckSelfInject(CheckSelfInject other) {
+ this.other = other;
+ }
+ }
+
+ /**
+ * Checks a simple case of constructor needing the same class
+ */
+ public void testSelfInject() {
+ IEclipseContext context = EclipseContextFactory.create();
+ boolean exceptionReceived = false;
+ try {
+ CheckSelfInject testInstance = ContextInjectionFactory.make(CheckSelfInject.class, context);
+ assertNotNull(testInstance); // unreachable
+ } catch (InjectionException e) {
+ exceptionReceived = true;
+ }
+ assertTrue(exceptionReceived);
+ }
+
+ ///////////////////////////////////////////////////////////////////////
+
+ static public class TestOuterClass {
+
+ public class TestInnerClassInject {
+ @Inject
+ public TestInnerClassInject() {
+ // placeholder
+ }
+ }
+
+ public TestInnerClassInject innerInject;
+
+ @Inject
+ public TestOuterClass() {
+ // placeholder
+ }
+
+ @PostConstruct
+ public void init(IEclipseContext context) {
+ innerInject = ContextInjectionFactory.make(TestInnerClassInject.class, context);
+ }
+ }
+
+ /**
+ * Checks inner class using outer class which is still being created
+ */
+ public void testNested() {
+ IEclipseContext context = EclipseContextFactory.create();
+ boolean exceptionReceived = false;
+ try {
+ TestOuterClass outer = ContextInjectionFactory.make(TestOuterClass.class, context);
+ assertNotNull(outer); // unreachable
+ } catch (InjectionException e) {
+ exceptionReceived = true;
+ }
+ assertTrue(exceptionReceived);
+ }
+}
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 407a2b6..a99e9ea 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
@@ -47,6 +47,7 @@
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.RecursiveObjectCreationTest;
import org.eclipse.e4.core.internal.tests.di.extensions.InjectionEventTest;
import org.eclipse.e4.core.internal.tests.di.extensions.InjectionMixedSuppliersTest;
import org.eclipse.e4.core.internal.tests.di.extensions.InjectionPreferencesTest;
@@ -100,5 +101,6 @@
addTestSuite(DependenciesLeakTest.class);
addTestSuite(ActivationInjectionTest.class);
addTestSuite(GenericsInjectionTest.class);
+ addTestSuite(RecursiveObjectCreationTest.class);
}
}
