IllegalStateException in LateBoundConstructor only when calling Provider.get() for types setup in a child Injector.

[GoogleCodeExporter]

It appears that some initialization never occurs when types are configured
in the nested injector instead of the parent injected.

This only occurs for types setup in the nested injector - if you move the
bind to the parent injector, the repo test executes perfectly.

Unfortunately this makes nested injectors pretty pointless for me. :s

Stack trace:
java.util.concurrent.ExecutionException: java.lang.IllegalStateException:
Construct before bind, null
    at java.util.concurrent.FutureTask$Sync.innerGet(Unknown Source)
    at java.util.concurrent.FutureTask.get(Unknown Source)
    at NestedInjectorScopedBugTest.main(NestedInjectorScopedBugTest.java:35)
Caused by: java.lang.IllegalStateException: Construct before bind, null
    at
com.google.inject.internal.base.Preconditions.checkState(Preconditions.java:137)
    at
com.google.inject.InjectorImpl$LateBoundConstructor.get(InjectorImpl.java:457)
    at com.google.inject.InjectorImpl$7$1.call(InjectorImpl.java:783)
    at com.google.inject.InjectorImpl.callInContext(InjectorImpl.java:829)
    at com.google.inject.InjectorImpl$7.get(InjectorImpl.java:779)
    at ExampleBrokenClass.run(ExampleBrokenClass.java:32)
    at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
    at java.util.concurrent.FutureTask$Sync.innerRun(Unknown Source)
    at java.util.concurrent.FutureTask.run(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
    at java.lang.Thread.run(Unknown Source)

Original issue reported on code.google.com by bigwall...@gmail.com on 26 Jan 2009 at 10:55

Attachments:

• GuiceBugRepo.zip

[GoogleCodeExporter]

I forgot to mention - this was with snapshot-20081123

Original comment by bigwall...@gmail.com on 26 Jan 2009 at 10:58

[GoogleCodeExporter]

I've created a patch to add a JUnit test to InjectorTest that duplicates the 
problem
bettet (including the positive and negative cases). I've been trying to debug 
what is
going on that leads to this, but so far have had no luck (the control flow with 
all
this is pretty complicated)

Original comment by bigwall...@gmail.com on 7 Feb 2009 at 10:18

Attachments:

• InjectorTest.java.patch

[GoogleCodeExporter]

As a side note (for the record here), the problem occurs when a type needs a 
provider
of its own type, as well as an instance of another (unrelated) type to be 
constructed.

If the (unrelated) type is bound in the child injector, attempting to use the
Provider for its own type (from inside a created instance) will explode with the
exception noted (the test duplicates this). 

If the (unrelated) type is bound in the parent injector it works without 
problems.

Original comment by bigwall...@gmail.com on 7 Feb 2009 at 10:24

[GoogleCodeExporter]

Ho ho ho. I figured out what the bug is, and its a doozy. 

private <T> BindingImpl<T> getJustInTimeBinding(Key<T> key, Errors errors) 
(approx
line 166) calls getRecursiveJustInTimeBinding(), which attempts to bind on the 
parent
before binding in the child.  

It calls:

<T> void initializeBinding(BindingImpl<T> binding, Errors errors) in 
InjectorImpl
(approx line 346) initializes each binding, removing the binding from the 
current
objects jitBindings if the binding cannot be initialized.

on the parent, and if a type has been bound at the child, this will always fail 
due
to the blacklisting, and then it will be attempted (and created on the child).
HOWEVER, in the case of types that require Providers to their own types, I 
believe
the removal of the reference in the parents jitBinding cache is not enough, as 
there
is a reference within another key to the originally created (but could not be
initialized) object. 

When the child then creates the binding (which it does successfully), it 
attempts to
find the binding for the Provider for its own type and hits the parent again 
first,
and does successfully find it there, and uses it within the childs Provider 
object. 

This is why you can get a instance, but the instance you retrieved has been 
given a
bogus (uninitialized) provider.

When I remove the code that asks the parent for a instance before the child, 
the test
passes successfully. The triggering event in this case is the blacklisting, but 
this
seems very problematic in general and I'm not sure it is worth the effort. 
Also, type
blacklisting seems like a 'dis-feature'? I'll send a separate email about that. 

Original comment by bigwall...@gmail.com on 8 Feb 2009 at 12:08

[GoogleCodeExporter]

As a side note, I'm making up a patch with the fixes for this. 

Original comment by bigwall...@gmail.com on 8 Feb 2009 at 12:16

[GoogleCodeExporter]

Wow, it sure is a doozy. We need to keep the blacklisting so that parent and 
child injectors can't have 
conflicting bindings for the same type. But we should be able to fix this 
regardless. Doing so may be tricky.

Original comment by limpbizkit on 8 Feb 2009 at 1:09

[GoogleCodeExporter]

As a side note, blacklisting wise, what would be considered conflicting 
bindings? 
i.e. if a parent says A.class is bound to some Provider<A>, and a child wants to
change that it will fail without the current blacklisting.

If a child binds A.class to some Provider<A>, I don't see why the parent needs 
to
know or care? (i.e. the results of the parent should not be impacted by the 
bindings
on any children, right?)

Original comment by bigwall...@gmail.com on 8 Feb 2009 at 1:20

[GoogleCodeExporter]

Original comment by limpbizkit on 26 Apr 2009 at 9:16

• Added labels: Type-Defect, Priority-High

[GoogleCodeExporter]

It's struck again! See this thread:
  http://groups.google.com/group/google-guice/browse_thread/thread/6ff3e85822ad4dc3

Original comment by limpbizkit on 12 Jun 2009 at 2:34

[GoogleCodeExporter]

A supplement test case to the new one from r1013.

--

static class DImpl implements D {}
public void testCircularWithChild() {
    Injector injector = Guice.createInjector().
            createChildInjector(new AbstractModule() {
        @Override
        protected void configure() {
            bind(D.class).to(DImpl.class);
        }
    });

    injector.getInstance(A.class);
  }

--

This should pass, but fails (even if DImpl is explicitly bound in the child). 
Explicitly binding A or B makes it pass, presumably because B is getting stuck 
in the
parent's JIT due to the same bug.

.. Just another test, but this time asserting that things succeed properly (as
opposed to fail properly).

Original comment by sberlin on 12 Jun 2009 at 2:14

[GoogleCodeExporter]

Patch attached that fixes this issue.  I expanded the existing test in
ImplicitBindingTest#testCircularJitBindingsLeaveNoResidue to make sure the 
cleanup
works properly in a variety of conditions.

It doesn't expose any new public APIs, and should have no overhead in the normal
successful construction case of a parent injector... but it may have some 
overhead
for child injectors.  That said, if there is any overhead, it will happen where 
the
code just would have blown up and failed before.

The logic of the patch is a little complex, so if someone could give it a look 
before
I commit, that would be great.

Original comment by sberlin on 24 May 2010 at 1:21

Attachments:

• cleanup-failed-bindings.txt

[GoogleCodeExporter]

FYI, the regression test attached to comment #2 passes after the patch in 
comment #11
is applied.  (Before #11 is applied, the test fails.)  I will commit the test 
from #2
with this change also.

Original comment by sberlin on 24 May 2010 at 1:52

[GoogleCodeExporter]

fixed in r1167.  i cleaned up the tests and asserted more things, so i'm pretty
confident the code is doing just what it should be doing.  please take a minute 
to
review the commit, though.

Original comment by sberlin on 25 May 2010 at 12:27

• Changed state: Fixed