Revision: 1593
Author: guice.mirror...@gmail.com
Date: Sun Oct 16 15:36:01 2011
Log:
Fix a very obscure bug where more than one InitializableReference of the
same identity could result in an NPE during injector creation if the one
that got ejected from the Map was requested for injection from another
InitializableReference.
Revision created by MOE tool push_codebase.
MOE_MIGRATION=3477
http://code.google.com/p/google-guice/source/detail?r=1593
Modified:
/trunk/core/src/com/google/inject/internal/Initializer.java
/trunk/core/test/com/google/inject/RequestInjectionTest.java
=======================================
--- /trunk/core/src/com/google/inject/internal/Initializer.java Thu Jul 7
17:34:16 2011
+++ /trunk/core/src/com/google/inject/internal/Initializer.java Sun Oct 16
15:36:01 2011
@@ -18,6 +18,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.inject.Key;
@@ -37,11 +38,16 @@
* @author jessewil...@google.com (Jesse Wilson)
*/
final class Initializer {
+
/** the only thread that we'll use to inject members. */
private final Thread creatingThread = Thread.currentThread();
/** zero means everything is injected. */
private final CountDownLatch ready = new CountDownLatch(1);
+
+ /** Maps from instances that need injection to the MembersInjector that
will inject them. */
+ private final Map<Object, MembersInjectorImpl<?>>
pendingMembersInjectors =
+ Maps.newIdentityHashMap();
/** Maps instances that need injection to a source that registered them
*/
private final Map<Object, InjectableReference<?>> pendingInjection =
Maps.newIdentityHashMap();
@@ -64,7 +70,8 @@
return Initializables.of(instance);
}
- InjectableReference<T> initializable = new
InjectableReference<T>(injector, instance, key, source);
+ InjectableReference<T> initializable =
+ new InjectableReference<T>(injector, instance, key, source);
pendingInjection.put(instance, initializable);
return initializable;
}
@@ -76,7 +83,7 @@
void validateOustandingInjections(Errors errors) {
for (InjectableReference<?> reference : pendingInjection.values()) {
try {
- reference.validate(errors);
+ pendingMembersInjectors.put(reference.instance,
reference.validate(errors));
} catch (ErrorsException e) {
errors.merge(e.getErrors());
}
@@ -111,7 +118,6 @@
private final T instance;
private final Object source;
private final Key<T> key;
- private MembersInjectorImpl<T> membersInjector;
public InjectableReference(InjectorImpl injector, T instance, Key<T>
key, Object source) {
this.injector = injector;
@@ -120,10 +126,10 @@
this.source = checkNotNull(source, "source");
}
- public void validate(Errors errors) throws ErrorsException {
+ public MembersInjectorImpl<T> validate(Errors errors) throws
ErrorsException {
@SuppressWarnings("unchecked") // the type of 'T' is a TypeLiteral<T>
TypeLiteral<T> type = TypeLiteral.get((Class<T>)
instance.getClass());
- membersInjector = injector.membersInjectorStore.get(type,
errors.withSource(source));
+ return injector.membersInjectorStore.get(type,
errors.withSource(source));
}
/**
@@ -148,6 +154,13 @@
// toInject needs injection, do it right away. we only do this once,
even if it fails
if (pendingInjection.remove(instance) != null) {
+ // safe because we only insert a members injector for the
appropriate instance
+ @SuppressWarnings("unchecked")
+ MembersInjectorImpl<T> membersInjector =
+
(MembersInjectorImpl<T>)pendingMembersInjectors.remove(instance);
+ Preconditions.checkState(membersInjector != null,
+ "No membersInjector available for instance: " + instance + ",
from key: " + key);
+
// if in Stage.TOOL, we only want to inject & notify toolable
injection points.
// (otherwise we'll inject all of them)
membersInjector.injectAndNotify(instance,
errors.withSource(source), key, source,
=======================================
--- /trunk/core/test/com/google/inject/RequestInjectionTest.java Thu Jul 7
17:34:16 2011
+++ /trunk/core/test/com/google/inject/RequestInjectionTest.java Sun Oct 16
15:36:01 2011
@@ -19,6 +19,10 @@
import static com.google.inject.Asserts.assertContains;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
+import com.google.inject.matcher.Matchers;
+import com.google.inject.spi.TypeEncounter;
+import com.google.inject.spi.TypeListener;
+
import junit.framework.TestCase;
import java.lang.annotation.Retention;
@@ -34,6 +38,7 @@
@Retention(RUNTIME)
@BindingAnnotation @interface ForMethod {}
+ @Override
protected void setUp() throws Exception {
super.setUp();
HasInjections.staticField = 0;
@@ -44,6 +49,7 @@
final HasInjections hi = new HasInjections();
Guice.createInjector(new AbstractModule() {
+ @Override
protected void configure() {
bindConstant().annotatedWith(ForMethod.class).to("test");
bindConstant().annotatedWith(ForField.class).to(5);
@@ -59,6 +65,7 @@
public void testInjectStatics() throws CreationException {
Guice.createInjector(new AbstractModule() {
+ @Override
protected void configure() {
bindConstant().annotatedWith(ForMethod.class).to("test");
bindConstant().annotatedWith(ForField.class).to(5);
@@ -74,6 +81,7 @@
final HasInjections hi = new HasInjections();
Guice.createInjector(new AbstractModule() {
+ @Override
protected void configure() {
bindConstant().annotatedWith(ForMethod.class).to("test");
bindConstant().annotatedWith(ForField.class).to(5);
@@ -91,6 +99,7 @@
public void testValidationErrorOnInjectedMembers() {
try {
Guice.createInjector(new AbstractModule() {
+ @Override
protected void configure() {
requestInjection(new NeedsRunnable());
}
@@ -105,6 +114,7 @@
public void testInjectionErrorOnInjectedMembers() {
try {
Guice.createInjector(new AbstractModule() {
+ @Override
protected void configure() {
bind(Runnable.class).toProvider(new Provider<Runnable>() {
public Runnable get() {
@@ -125,6 +135,7 @@
public void testUserExceptionWhileInjectingInstance() {
try {
Guice.createInjector(new AbstractModule() {
+ @Override
protected void configure() {
requestInjection(new BlowsUpOnInject());
}
@@ -140,6 +151,7 @@
public void testUserExceptionWhileInjectingStatically() {
try {
Guice.createInjector(new AbstractModule() {
+ @Override
protected void configure() {
requestStaticInjection(BlowsUpOnInject.class);
}
@@ -182,4 +194,41 @@
throw new UnsupportedOperationException("Snap");
}
}
-}
+
+ /*
+ * Tests that initializables of the same instance don't clobber
+ * membersInjectors in InitializableReference, so that they ultimately
+ * can be requested in any order.
+ */
+ public void testEarlyInjectableReferencesWithSameIdentity() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ // Add a listener to trigger all toInstance bindings to get an
Initializable.
+ bindListener(Matchers.any(), new TypeListener() {
+ @Override
+ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I>
encounter) {
+ }
+ });
+
+ // Bind two different Keys to the IDENTITICAL object
+ // ORDER MATTERS! We want the String binding to push out the
Object one
+ String fail = new String("better not fail!");
+ bind(Object.class).toInstance(fail);
+ bind(String.class).toInstance(fail);
+
+ // Then try to inject those objects in a requestInjection,
+ // letting us get into InjectableReference.get before it has
+ // finished running through all its injections.
+ // Each of these technically has its own InjectableReference
internally.
+ // ORDER MATTERS!.. because Object is injected first, that
InjectableReference
+ // attempts to process its members injector, but it wasn't
initialized,
+ // because String pushed it out of the queue!
+ requestInjection(new Object() {
+ @SuppressWarnings("unused") @Inject Object obj;
+ @SuppressWarnings("unused") @Inject String str;
+ });
+ }
+ });
+ }
+}
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To post to this group, send email to google-guice-dev@googlegroups.com.
To unsubscribe from this group, send email to
google-guice-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/google-guice-dev?hl=en.
