Skip to content

Commit

Permalink
Fix a subtle bug in Guice eager singleton evaluation. Add a test.
Browse files Browse the repository at this point in the history
A linked binding should only be treated as an eager singleton if the target binding is eager singleton *and* the source binding is unscoped. If the source binding is scoped, it may be a scope with context constraints (such as @RequestScoped) that shouldn't be evaluated during injector creation.

Bug repro:
bind(Key1).toInstance(<something>)
bind(Key2).to(Key1).in(RequestScoped.class);

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=249417146
  • Loading branch information
highegg authored and ronshapiro committed May 27, 2019
1 parent 6a2b571 commit f8cc171
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,11 @@ private boolean isEagerSingleton(InjectorImpl injector, BindingImpl<?> binding,

// handle a corner case where a child injector links to a binding in a parent injector, and
// that binding is singleton. We won't catch this otherwise because we only iterate the child's
// bindings.
// bindings. This only applies if the linked binding is not itself scoped.
if (binding instanceof LinkedBindingImpl) {
Key<?> linkedBinding = ((LinkedBindingImpl<?>) binding).getLinkedKey();
return isEagerSingleton(injector, injector.getBinding(linkedBinding), stage);
return binding.getScoping().isNoScope()
&& isEagerSingleton(injector, injector.getBinding(linkedBinding), stage);
}

return false;
Expand Down
36 changes: 36 additions & 0 deletions core/test/com/google/inject/ScopesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1340,4 +1340,40 @@ public Boolean visitNoScoping() {
}
}));
}

public void testScopedLinkedBindingDoesNotPropagateEagerSingleton() {
final Key<String> a = Key.get(String.class, named("A"));
final Key<String> b = Key.get(String.class, named("B"));

final Scope notInScopeScope =
new Scope() {
@Override
public <T> Provider<T> scope(Key<T> key, Provider<T> unscoped) {
return new Provider<T>() {
@Override
public T get() {
throw new IllegalStateException("Not in scope");
}
};
}
};

Module module =
new AbstractModule() {
@Override
protected void configure() {
bind(a).toInstance("a");
bind(b).to(a).in(CustomScoped.class);
bindScope(CustomScoped.class, notInScopeScope);
}
};

Injector injector = Guice.createInjector(module);
Provider<String> bProvider = injector.getProvider(b);
try {
bProvider.get();
fail("expected failure");
} catch (ProvisionException expected) {
}
}
}

0 comments on commit f8cc171

Please sign in to comment.