Status: New
Owner: ----

New issue 729 by antony.s...@gmail.com: Deadlock using FactoryModuleBuilder from different threads
http://code.google.com/p/google-guice/issues/detail?id=729

As per the discussion here:

https://groups.google.com/forum/?fromgroups=#!topic/google-guice/cOaZVIXo5cI

We're experiencing a deadlock in our multi-threaded code, and I think I've tracked it down to our use of FactoryModuleBuilder to create one of our objects during an HTTP response. We use the pattern of:
install(new FactoryModuleBuilder()
                .implement(User.class, User.class)
                .build(UserFactory.class));

And our dead lock looks like (we have more than a dozen threads all blocked in the same way): "Thread-2616" prio=10 tid=0x00007f41647e1800 nid=0xea9 waiting for monitor entry [0x00007f417fefd000]
   java.lang.Thread.State: BLOCKED (on object monitor)
at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:102) - waiting to lock <0x0000000608d609a0> (a com.google.inject.internal.InheritingState) at com.google.inject.internal.InjectorImpl.createChildInjector(InjectorImpl.java:217) at com.google.inject.internal.InjectorImpl.createChildInjector(InjectorImpl.java:224) at com.google.inject.assistedinject.FactoryProvider2.getBindingFromNewInjector(FactoryProvider2.java:602) at com.google.inject.assistedinject.FactoryProvider2.invoke(FactoryProvider2.java:625)
        at $Proxy16.create(Unknown Source)
        at ourCode

So I am concluding that the Factory Module Builder pattern isn't thread safe per-se, and will just go back to creating our object manually, then using injector#injectMembers() afterwards.

Am I missing something or does this sound about right? Is this correct? Or am I missing something? If it is, it would be great to get a warning added somewhere..

The relavent Guice code is FactoryProvider2:
 /**
* When a factory method is invoked, we create a child injector that binds all parameters, then
   * use that to get an instance of the return type.
   */
public Object invoke(Object proxy, final Method method, final Object[] args) throws Throwable {

and then the synchro block in InternalInjectorCreator:

  public Injector build() {
    if (shellBuilder == null) {
      throw new AssertionError("Already built, builders are not reusable.");
    }

// Synchronize while we're building up the bindings and other injector state. This ensures that // the JIT bindings in the parent injector don't change while we're being built
    synchronized (shellBuilder.lock()) {
shells = shellBuilder.build(initializer, bindingData, stopwatch, errors);
      stopwatch.resetAndLog("Injector construction");

      initializeStatically();
    }

    injectDynamically();

    if (shellBuilder.getStage() == Stage.TOOL) {
      // wrap the primaryInjector in a ToolStageInjector
      // to prevent non-tool-friendy methods from being called.
      return new ToolStageInjector(primaryInjector());
    } else {
      return primaryInjector();
    }
  }

The entire relevant stack trace is attached (all blocked threads in Guice stack).

There is one running thread in the Guice stack, but we're definitely seeing either a total dead lock, or that it's taking longer than 10seconds to inject the classes.

Attachments:
        tdump.log  45.2 KB

--
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.

Reply via email to