Revision: 1543
Author: sberlin
Date: Sat Apr 30 08:38:05 2011
Log: fix issue 626 -- properly handle circular proxies between
providers (either failing or proxying), prevents StackOverflowErrors.
http://code.google.com/p/google-guice/source/detail?r=1543
Added:
/trunk/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
/trunk/core/src/com/google/inject/internal/ProviderInternalFactory.java
Modified:
/trunk/core/src/com/google/inject/internal/BindingProcessor.java
/trunk/core/src/com/google/inject/internal/BoundProviderFactory.java
/trunk/core/src/com/google/inject/internal/DelegatingInvocationHandler.java
/trunk/core/src/com/google/inject/internal/InjectorImpl.java
/trunk/core/src/com/google/inject/internal/InternalFactoryToProviderAdapter.java
/trunk/core/src/com/google/inject/internal/Scoping.java
/trunk/core/test/com/google/inject/CircularDependencyTest.java
=======================================
--- /dev/null
+++
/trunk/core/src/com/google/inject/internal/InternalFactoryToInitializableAdapter.java
Sat Apr 30 08:38:05 2011
@@ -0,0 +1,52 @@
+/**
+ * Copyright (C) 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.inject.internal;
+
+import com.google.inject.Provider;
+import static com.google.inject.internal.util.Preconditions.checkNotNull;
+import com.google.inject.spi.Dependency;
+import com.google.inject.spi.ProviderInstanceBinding;
+
+/**
+ * Adapts {@link ProviderInstanceBinding} providers, ensuring circular
proxies
+ * fail (or proxy) properly.
+ *
+ * @author [email protected] (Sam Berlin)
+*/
+final class InternalFactoryToInitializableAdapter<T> extends
ProviderInternalFactory<T> {
+
+ private final Initializable<Provider<? extends T>> initializable;
+
+ public InternalFactoryToInitializableAdapter(
+ Initializable<Provider<? extends T>> initializable, Object source,
boolean allowProxy) {
+ super(source, allowProxy);
+ this.initializable = checkNotNull(initializable, "provider");
+ }
+
+ public T get(Errors errors, InternalContext context, Dependency<?>
dependency, boolean linked)
+ throws ErrorsException {
+ try {
+ return circularGet(initializable.get(errors), errors, context,
dependency, linked);
+ } catch(RuntimeException userException) {
+ throw
errors.withSource(source).errorInProvider(userException).toException();
+ }
+ }
+
+ @Override public String toString() {
+ return initializable.toString();
+ }
+}
=======================================
--- /dev/null
+++ /trunk/core/src/com/google/inject/internal/ProviderInternalFactory.java
Sat Apr 30 08:38:05 2011
@@ -0,0 +1,72 @@
+/**
+ * Copyright (C) 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+package com.google.inject.internal;
+
+import static com.google.inject.internal.util.Preconditions.checkNotNull;
+
+import javax.inject.Provider;
+
+import com.google.inject.spi.Dependency;
+
+/**
+ * Base class for InternalFactories that are used by Providers, to handle
+ * circular dependencies.
+ *
+ * @author [email protected] (Sam Berlin)
+ */
+abstract class ProviderInternalFactory<T> implements InternalFactory<T> {
+
+ protected final Object source;
+ private final boolean allowProxy;
+
+ ProviderInternalFactory(Object source, boolean allowProxy) {
+ this.source = checkNotNull(source, "source");
+ this.allowProxy = allowProxy;
+ }
+
+ protected T circularGet(Provider<? extends T> provider, Errors errors,
+ InternalContext context, Dependency<?> dependency, boolean linked)
+ throws ErrorsException {
+ Class<?> expectedType =
dependency.getKey().getTypeLiteral().getRawType();
+ // we do not use 'this' as the key because for JIT provider bindings,
+ // 'this' is recreated on failures... the key suffices.
+ ConstructionContext<T> constructionContext =
context.getConstructionContext(dependency.getKey());
+
+ // We have a circular reference between constructors. Return a proxy.
+ if (constructionContext.isConstructing()) {
+ if (!allowProxy) {
+ throw errors.circularProxiesDisabled(expectedType).toException();
+ } else {
+ // TODO: if we can't proxy this object, can we proxy the other
object?
+ @SuppressWarnings("unchecked")
+ T proxyType = (T) constructionContext.createProxy(errors,
expectedType);
+ return proxyType;
+ }
+ }
+ // First time through...
+ constructionContext.startConstruction();
+ try {
+ T t = errors.checkForNull(provider.get(), source, dependency);
+ constructionContext.setProxyDelegates(t);
+ return t;
+ } finally {
+ constructionContext.finishConstruction();
+ }
+ }
+
+}
=======================================
--- /trunk/core/src/com/google/inject/internal/BindingProcessor.java Thu
Mar 10 19:02:39 2011
+++ /trunk/core/src/com/google/inject/internal/BindingProcessor.java Sat
Apr 30 08:38:05 2011
@@ -100,7 +100,8 @@
Set<InjectionPoint> injectionPoints = binding.getInjectionPoints();
Initializable<Provider<? extends T>> initializable = initializer
.<Provider<? extends T>>requestInjection(injector, provider,
source, injectionPoints);
- InternalFactory<T> factory = new
InternalFactoryToProviderAdapter<T>(initializable, source);
+ InternalFactory<T> factory = new
InternalFactoryToInitializableAdapter<T>(
+ initializable,
source, !injector.options.disableCircularProxies);
InternalFactory<? extends T> scopedFactory
= Scoping.scope(key, injector, factory, source, scoping);
putBinding(new ProviderInstanceBindingImpl<T>(injector, key,
source, scopedFactory, scoping,
@@ -111,8 +112,8 @@
public Boolean visit(ProviderKeyBinding<? extends T> binding) {
prepareBinding();
Key<? extends javax.inject.Provider<? extends T>> providerKey =
binding.getProviderKey();
- BoundProviderFactory<T> boundProviderFactory
- = new BoundProviderFactory<T>(injector, providerKey, source);
+ BoundProviderFactory<T> boundProviderFactory = new
BoundProviderFactory<T>(
+ injector, providerKey,
source, !injector.options.disableCircularProxies);
bindingData.addCreationListener(boundProviderFactory);
InternalFactory<? extends T> scopedFactory = Scoping.scope(
key, injector, (InternalFactory<? extends T>)
boundProviderFactory, source, scoping);
=======================================
--- /trunk/core/src/com/google/inject/internal/BoundProviderFactory.java
Thu Mar 10 19:02:39 2011
+++ /trunk/core/src/com/google/inject/internal/BoundProviderFactory.java
Sat Apr 30 08:38:05 2011
@@ -23,20 +23,20 @@
/**
* Delegates to a custom factory which is also bound in the injector.
*/
-final class BoundProviderFactory<T> implements InternalFactory<T>,
CreationListener {
+final class BoundProviderFactory<T> extends ProviderInternalFactory<T>
implements CreationListener {
private final InjectorImpl injector;
final Key<? extends javax.inject.Provider<? extends T>> providerKey;
- final Object source;
private InternalFactory<? extends javax.inject.Provider<? extends T>>
providerFactory;
BoundProviderFactory(
InjectorImpl injector,
Key<? extends javax.inject.Provider<? extends T>> providerKey,
- Object source) {
+ Object source,
+ boolean allowProxy) {
+ super(source, allowProxy);
this.injector = injector;
this.providerKey = providerKey;
- this.source = source;
}
public void notify(Errors errors) {
@@ -52,10 +52,10 @@
errors = errors.withSource(providerKey);
javax.inject.Provider<? extends T> provider =
providerFactory.get(errors, context, dependency, true);
try {
- return errors.checkForNull(provider.get(), source, dependency);
+ return circularGet(provider, errors, context, dependency, linked);
} catch(RuntimeException userException) {
throw errors.errorInProvider(userException).toException();
- }
+ }
}
@Override public String toString() {
=======================================
---
/trunk/core/src/com/google/inject/internal/DelegatingInvocationHandler.java
Sat Jul 3 08:51:31 2010
+++
/trunk/core/src/com/google/inject/internal/DelegatingInvocationHandler.java
Sat Apr 30 08:38:05 2011
@@ -34,6 +34,9 @@
}
try {
+ // TODO: method.setAccessible(true); ?
+ // this would fix visibility errors when we proxy a
+ // non-public interface.
return method.invoke(delegate, args);
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
=======================================
--- /trunk/core/src/com/google/inject/internal/InjectorImpl.java Wed Mar 2
05:53:17 2011
+++ /trunk/core/src/com/google/inject/internal/InjectorImpl.java Sat Apr 30
08:38:05 2011
@@ -677,14 +677,16 @@
final BindingImpl<? extends Provider<?>> providerBinding
= getBindingOrThrow(providerKey, errors,
JitLimitation.NEW_OR_EXISTING_JIT);
- InternalFactory<T> internalFactory = new InternalFactory<T>() {
+ InternalFactory<T> internalFactory =
+ new
ProviderInternalFactory<T>(providerKey, !options.disableCircularProxies) {
public T get(Errors errors, InternalContext context, Dependency
dependency, boolean linked)
throws ErrorsException {
errors = errors.withSource(providerKey);
- Provider<?> provider = providerBinding.getInternalFactory().get(
+ Provider provider = providerBinding.getInternalFactory().get(
errors, context, dependency, true);
try {
- Object o = provider.get();
+ @SuppressWarnings("unchecked") // type is not checked within
circularGet
+ Object o = circularGet(provider, errors, context, dependency,
linked);
if (o != null && !rawType.isInstance(o)) {
throw errors.subtypeNotProvided(providerType,
rawType).toException();
}
=======================================
---
/trunk/core/src/com/google/inject/internal/InternalFactoryToProviderAdapter.java
Sat Jul 3 08:51:31 2010
+++
/trunk/core/src/com/google/inject/internal/InternalFactoryToProviderAdapter.java
Sat Apr 30 08:38:05 2011
@@ -25,25 +25,24 @@
*/
final class InternalFactoryToProviderAdapter<T> implements
InternalFactory<T> {
- private final Initializable<Provider<? extends T>> initializable;
+ private final Provider<? extends T> provider;
private final Object source;
- public InternalFactoryToProviderAdapter(
- Initializable<Provider<? extends T>> initializable, Object source) {
- this.initializable = checkNotNull(initializable, "provider");
+ public InternalFactoryToProviderAdapter(Provider<? extends T> provider,
Object source) {
+ this.provider = checkNotNull(provider, "provider");
this.source = checkNotNull(source, "source");
}
public T get(Errors errors, InternalContext context, Dependency<?>
dependency, boolean linked)
throws ErrorsException {
try {
- return errors.checkForNull(initializable.get(errors).get(), source,
dependency);
+ return errors.checkForNull(provider.get(), source, dependency);
} catch (RuntimeException userException) {
throw
errors.withSource(source).errorInProvider(userException).toException();
}
}
@Override public String toString() {
- return initializable.toString();
+ return provider.toString();
}
}
=======================================
--- /trunk/core/src/com/google/inject/internal/Scoping.java Sat Jul 3
08:51:31 2010
+++ /trunk/core/src/com/google/inject/internal/Scoping.java Sat Apr 30
08:38:05 2011
@@ -239,8 +239,7 @@
Provider<T> scoped
= scope.scope(key, new
ProviderToInternalFactoryAdapter<T>(injector, creator));
- return new InternalFactoryToProviderAdapter<T>(
- Initializables.<Provider<? extends T>>of(scoped), source);
+ return new InternalFactoryToProviderAdapter<T>(scoped, source);
}
/**
=======================================
--- /trunk/core/test/com/google/inject/CircularDependencyTest.java Sun Dec
12 18:44:36 2010
+++ /trunk/core/test/com/google/inject/CircularDependencyTest.java Sat Apr
30 08:38:05 2011
@@ -21,13 +21,14 @@
/**
* @author [email protected] (Bob Lee)
+ * @author [email protected] (Sam Berlin)
*/
public class CircularDependencyTest extends TestCase {
-
- @Override protected void setUp() throws Exception {
- super.setUp();
- Chicken.nextInstanceId = 0;
- Egg.nextInstanceId = 0;
+
+ @Override
+ protected void setUp() throws Exception {
+ AImpl.nextId = 0;
+ BImpl.nextId = 0;
}
public void testCircularlyDependentConstructors()
@@ -38,39 +39,118 @@
bind(B.class).to(BImpl.class);
}
});
-
+ assertCircularDependencies(injector);
+ }
+
+ public void testCircularlyDependentConstructorsWithProviderInstances()
+ throws CreationException {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ protected void configure() {}
+
+ @Provides @Singleton A a(B b) { return new AImpl(b); }
+ @Provides B b(A a) { return new BImpl(a); }
+ });
+ assertCircularDependencies(injector);
+ }
+
+ public void testCircularlyDependentConstructorsWithProviderKeys()
+ throws CreationException {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ bind(A.class).toProvider(AP.class).in(Singleton.class);
+ bind(B.class).toProvider(BP.class);
+ }
+ });
+ assertCircularDependencies(injector);
+ }
+
+ public void testCircularlyDependentConstructorsWithProvidedBy()
+ throws CreationException {
+ Injector injector = Guice.createInjector();
+ assertCircularDependencies(injector);
+ }
+
+ private void assertCircularDependencies(Injector injector) {
A a = injector.getInstance(A.class);
assertNotNull(a.getB().getA());
+ assertEquals(0, a.id());
+ assertEquals(a.id(), a.getB().getA().id());
+ assertEquals(0, a.getB().id());
+ assertEquals(1, AImpl.nextId);
+ assertEquals(1, BImpl.nextId);
+ assertSame(a, injector.getInstance(A.class));
}
- interface A {
+ @ProvidedBy(AutoAP.class)
+ public interface A {
B getB();
+ int id();
}
@Singleton
static class AImpl implements A {
+ static int nextId;
+ int id = nextId++;
+
final B b;
@Inject public AImpl(B b) {
this.b = b;
}
+ public int id() {
+ return id;
+ }
public B getB() {
return b;
}
}
-
- interface B {
+
+ static class AP implements Provider<A> {
+ @Inject Provider<B> bp;
+ public A get() {
+ return new AImpl(bp.get());
+ }
+ }
+
+ @Singleton
+ static class AutoAP implements Provider<A> {
+ @Inject Provider<B> bp;
+ A a;
+ public A get() {
+ if (a == null) {
+ a = new AImpl(bp.get());
+ }
+ return a;
+ }
+ }
+
+ @ProvidedBy(BP.class)
+ public interface B {
A getA();
+ int id();
}
static class BImpl implements B {
+ static int nextId;
+ int id = nextId++;
+
final A a;
@Inject public BImpl(A a) {
this.a = a;
}
+ public int id() {
+ return id;
+ }
public A getA() {
return a;
}
}
+
+ static class BP implements Provider<B> {
+ @Inject Provider<A> ap;
+ public B get() {
+ return new BImpl(ap.get());
+ }
+ }
public void testUnresolvableCircularDependency() {
try {
@@ -82,14 +162,144 @@
"but it is not an interface.");
}
}
+
+ public void testUnresolvableCircularDependenciesWithProviderInstances() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override protected void configure() {}
+ @Provides C c(D d) { return null; }
+ @Provides D d(C c) { return null; }
+ }).getInstance(C.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + C.class.getName() + " to support a circular
dependency, ",
+ "but it is not an interface.");
+ }
+ }
+
+ public void testUnresolvableCircularDependenciesWithProviderKeys() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override protected void configure() {
+ bind(C2.class).toProvider(C2P.class);
+ bind(D2.class).toProvider(D2P.class);
+ }
+ }).getInstance(C2.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + C2.class.getName() + " to support a circular
dependency, ",
+ "but it is not an interface.");
+ }
+ }
+
+ public void testUnresolvableCircularDependenciesWithProvidedBy() {
+ try {
+ Guice.createInjector().getInstance(C2.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + C2.class.getName() + " to support a circular
dependency, ",
+ "but it is not an interface.");
+ }
+ }
static class C {
@Inject C(D d) {}
}
-
static class D {
@Inject D(C c) {}
}
+
+ static class C2P implements Provider<C2> {
+ @Inject Provider<D2> dp;
+ public C2 get() {
+ dp.get();
+ return null;
+ }
+ }
+ static class D2P implements Provider<D2> {
+ @Inject Provider<C2> cp;
+ public D2 get() {
+ cp.get();
+ return null;
+ }
+ }
+ @ProvidedBy(C2P.class)
+ static class C2 {
+ @Inject C2(D2 d) {}
+ }
+ @ProvidedBy(D2P.class)
+ static class D2 {
+ @Inject D2(C2 c) {}
+ }
+
+ public void testDisabledCircularDependency() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ binder().disableCircularProxies();
+ }
+ }).getInstance(C.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + C.class.getName() + " to support a circular
dependency, ",
+ "but circular proxies are disabled.");
+ }
+ }
+
+ public void testDisabledCircularDependenciesWithProviderInstances() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override protected void configure() {
+ binder().disableCircularProxies();
+ }
+ @Provides C c(D d) { return null; }
+ @Provides D d(C c) { return null; }
+ }).getInstance(C.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + C.class.getName() + " to support a circular
dependency, ",
+ "but circular proxies are disabled.");
+ }
+ }
+
+ public void testDisabledCircularDependenciesWithProviderKeys() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override protected void configure() {
+ binder().disableCircularProxies();
+ bind(C2.class).toProvider(C2P.class);
+ bind(D2.class).toProvider(D2P.class);
+ }
+ }).getInstance(C2.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + C2.class.getName() + " to support a circular
dependency, ",
+ "but circular proxies are disabled.");
+ }
+ }
+
+ public void testDisabledCircularDependenciesWithProvidedBy() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ binder().disableCircularProxies();
+ }
+ }).getInstance(C2.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "Tried proxying " + C2.class.getName() + " to support a circular
dependency, ",
+ "but circular proxies are disabled.");
+ }
+ }
/**
* As reported by issue 349, we give a lousy trace when a class is
circularly
@@ -136,43 +346,12 @@
public A getA() {
return this;
}
- }
-
- static class Chicken {
- static int nextInstanceId;
- final int instanceId = nextInstanceId++;
- @Inject Egg source;
- }
-
- static class Egg {
- static int nextInstanceId;
- final int instanceId = nextInstanceId++;
- @Inject Chicken source;
- }
-
- public void testCircularlyDependentSingletonsWithProviders() {
- Injector injector = Guice.createInjector(new AbstractModule() {
- protected void configure() {
- bind(Chicken.class).in(Singleton.class);
- }
-
- @Provides @Singleton Egg provideEgg(Chicken chicken) {
- Egg egg = new Egg();
- egg.source = chicken;
- return egg;
- }
- });
-
- try {
- injector.getInstance(Egg.class);
- fail();
- } catch (ProvisionException e) {
- assertContains(e.getMessage(),
- "Provider was reentrant while creating a singleton",
- " at " + CircularDependencyTest.class.getName(), "provideEgg(",
- " while locating " + Egg.class.getName());
+
+ public int id() {
+ return 0;
}
}
+
public void testCircularDependencyProxyDelegateNeverInitialized() {
Injector injector = Guice.createInjector(new AbstractModule() {
@@ -226,5 +405,4 @@
return "G";
}
}
-
-}
+}
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/google-guice-dev?hl=en.
