Deadlock on "waitForValue" #785
Comments
|
From eiden@no-cache.net on December 01, 2013 11:24:07 Here is a test that demonstrates the deadlock class DeadlockingModule extends AbstractModule { static class Deadlock<T> { |
|
From SmokejumperIT on December 05, 2013 11:13:52 Still fails in 4.0-beta. |
|
From sberlin on December 05, 2013 11:27:56 Before I dig in... in that test-case, what would you expect the behavior to be? |
|
From SmokejumperIT on December 05, 2013 12:30:50 I'd prefer #1, would accept #2, and would be happy with anything that didn't just hang my server indefinitely. :D |
|
From sberlin on December 05, 2013 13:54:38 In the latest HEAD version, I get this failure: com.google.common.util.concurrent.UncheckedExecutionException: com.google.common.util.concurrent.UncheckedExecutionException: java.lang.IllegalStateException: Recursive load |
|
From sberlin on December 05, 2013 13:54:38 ...sp;at com.google.inject.internal.MembersInjectorStore$1.create(MembersInjectorStore.java:1) Could you try building from HEAD and see if you also get that? If you do, I'm inclined to close this as WontFix. |
|
From SmokejumperIT on December 09, 2013 07:05:52 This change so that we're not hanging is a huge improvement: thank you! And the error message and stack traces are meaningful, which is also very useful. But can we get a slightly better error message, so that we know what the injection is deadlocking on? If that works, then I think you could close it as fixed. |
|
From sberlin on December 09, 2013 07:11:39 The best place for that change would be in Guava, I think... changing the "recursive load" exception to "Recursive load of key: <key>". I'll bring it up with them and see what we can do. |
|
From SmokejumperIT on December 23, 2013 06:42:36 Did you raise this as a ticket over on Guava? If not, I will. |
|
From sameb@google.com on December 23, 2013 07:20:49 Yup, I fixed it internally, which looks like it got pushed out as https://code.google.com/p/guava-libraries/source/detail?r=2501cceb9f8059153cacda024843b8969e2851d8 and should be included in Guava v16 release. Oncwe we fix up the Guice deps to depend directly on guava instead embedding it, you should see the change. |
|
From SmokejumperIT on January 09, 2014 13:50:45 I guess this wasn't as done as we hoped: our staging environment is still getting lock-ups within Guava intermittently, even though I'm using a HEAD version of this code. (I attached the SNAPSHOT jar that we are using.) I suspect it's the same issue, although it's now occurring at a different location. (If it's not the same issue, I'll fork this and open a new one.) In the attached stack dump, we're getting a thread (exec-18) which is parking to wait on com.google.inject.internal.guava.util.concurrent.$AbstractFuture$Sync (memory location 0x00000000ea36bcd0) -- condition 0x00007f6254ef0000 -- but I don't see anywhere that's locked. It is just sitting there forever, however, so it looks like some kind of permit bookkeeping has been messed up. That thread is also holding onto a lock of com.google.inject.internal.InheritingState (memory location 0x00000000ea036818), which is resulting in other threads also locking up. The use case is a web server, where a single static injector is being used to construct the resources (read: "MVC controllers"), one per request, and so we're getting a lot of thread contention. Would a temporary work-around be to synchronize on the injector, so that it is only used once at a time? Attachment: gist |
|
From sberlin on January 09, 2014 14:11:04 I don't know if Guava released a version with the changes, and Guice definitely hasn't updated the version of Guava it uses yet. So using head Guice won't have changed anything. |
|
From sberlin on January 09, 2014 14:12:56 Ah, wait, sorry I was referring to if the recursive load exception happened, but it looks like you're saying you get deadlock instead of the exception still. I'm not sure why that'd happen. Does the testcase you posted earlier still result in this deadlock for you? If not, can you try to come up with a new test-case that reproduces what you're seeing? |
|
From SmokejumperIT on January 10, 2014 04:52:32 I'll see what I can pull together for you. |
|
From SmokejumperIT on January 10, 2014 05:36:20 This'll do: import java.util.concurrent.ExecutorService import com.google.inject.Guice class GuiceHanging { static class Injected { static class Parent { static class Child1 { static class Child2 { static void main(String[] args) { |
|
From SmokejumperIT on January 10, 2014 05:39:15 Output of running that script is attached. Note that if you remove the "child1InChild2" field, it runs just fine. Binary attachments: hanging thread output.txt |
|
From SmokejumperIT on January 10, 2014 05:42:52 Synchronizing on the injector does seem to solve the problem. I'd really rather the injector be thread-safe, but it's a passable work-around for the time being. |
|
From SmokejumperIT on January 10, 2014 05:57:28 Just FYI: upgrading to Guava 16.0-rc1 does not fix the problem, although it does make it occur slightly less often. |
|
From bob@vawter.org on April 29, 2014 09:27:27 Here's a "Recursive load of" stack trace for 4.0-beta4 that occurs intermittently. Is synchronizing on use of the Injector still the recommended workaround? How about an explicit binding of the type in question (which is all throughout our app)? Thanks. Attachment: gist |
|
Are there any more updates on this issue? It seems that I'm running into either this issue or one that's very similar. I'm in the process of trying to migrate from Spring to Guice and I've hit this "Recursive load of" error. One difference with what I'm doing though is that I'm in the process of writing some test cases and I'm hitting this error in only a single thread of code so there's no contention between threads and synchronizing on the injector wouldn't help me. Also, I get one of two outcomes from this (and the fact that it's not consistent has me a little freaked out). Sometimes I will get the "Recursive load of" error. But other times the whole thread will just hang on the call "LocalCache.waitForValue". I get one of those two results 100% of the time however. We definitely have loops in our dependency structure but I thought those would be resolved by using a Provider for all the dependencies. Unfortunately I can't narrow the problem down or provided a small example because this is happening in a very complex object graph. This makes me very nervous about proceeding with using Guice. I don't even know where to proceed from here. Anyone have any suggestions? I've also tried upgrading to Guice 4.0 and Guava 19.0 and those didn't help. |
|
If you're able to create a small self-contained test case demonstrating the On Tue, Mar 8, 2016 at 10:28 AM TazmanianD notifications@github.com wrote:
|
|
Okay, I think I was finally able to create a small repro case. I took all of the classes involved in my problem and made empty copies of them (keeping just their constructors) in a new project. Then I just started deleting stuff until the problem disappeared and went back and forth until I got a minimal set. Then I just to remove extra rungs in the dependency ladder. The code below fails for me with the message "java.lang.IllegalStateException: Recursive load of: com.guicetest.Class01.()". I will note that there is an error here in that Interface01 is not bound to anything (and I discovered several errors in my code as I was going through this process). Once I fixed those errors, the problem went away. But since the "Recursive load" error doesn't seem like something the library should permit and it definitely should not hang, it might be good to still investigate and see if there is actually a bug or if the code can respond to bad usage more gracefully. If I change anything in the example below, then I get the message "No implementation for com.guicetest.Interface01 was bound" which is what I would expect to get. Even simply changing the order of arguments on Class01() causes the "Recursive load" problem to go away. Removing the unused arguments on providesClass02 causes the problem to go away. Changing the interface to a class makes the problem go away. So I'm hoping that the error in my setup was ultimately the cause and not that there's still a lurking problem that just happens to require a precise configuration of elements. And hopefully you get the same result if you run this test. package com.guicetest;
import javax.inject.Inject;
import javax.inject.Provider;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Provides;
/* Dependencies:
Class01
Class04
Class03
Class02
@Provides Class01
Class03
Class01
Class04
Class05
Class01
Class05
Interface01
*/
public class MyTest {
public static void main(String[] args) {
Guice.createInjector(new AbstractModule() {
@Override
protected void configure() {
}
@Provides
public Class02 providesClass02(Provider<Class01> arg) {
return null;
}
});
}
}
class Class01 {
@Inject
public Class01(Provider<Class04> arg1, Provider<Class03> arg2) {
}
}
class Class02 {
}
class Class03 {
@Inject
public Class03(Provider<Class01> arg) {
}
}
class Class04 {
@Inject
public Class04(Provider<Class05> arg1, Provider<Class01> arg2) {
}
}
class Class05 {
@Inject
public Class05(Provider<Interface01> arg) {
}
}
interface Interface01 {
}
|
|
This is great, thanks. I'll dig in locally and see what happens. |
|
Hi, there is any news about this? I'm having the same problem. I'm updating a huge application from guice 2.0 to 4.1 and this problem is happening for me. thanks in advance for the feedback. |
|
having the same problem using Guava 20.0 and Guice 4.2.2. |
This test causes Guice to violate the requirements of Guava's LoadingCache by triggering a recursive load. The recursive load crashes either with an UncheckedExecutionException (bad) or causes a deadlock (terrible). google#785
This test causes Guice to violate the requirements of Guava's LoadingCache by triggering a recursive load. The recursive load crashes either with an UncheckedExecutionException (bad) or causes a deadlock (terrible). google#785
This test causes Guice to violate the requirements of Guava's LoadingCache by triggering a recursive load. The recursive load crashes either with an UncheckedExecutionException (bad) or causes a deadlock (terrible). google#785
This test causes Guice to violate the requirements of Guava's LoadingCache by triggering a recursive load. The recursive load crashes either with an UncheckedExecutionException (bad) or causes a deadlock (terrible). google#785
|
We still have this problem with Guice 5.1.0 / Guava 31.1-jre : it makes near impossible to find which binding is missing in a project with a substantial amount of classes. |
…nd/or crash inside Guava's LoadingCache. The problem had to do with the fact that Guice eagerly stores uninitialized JIT bindings in an attempt to allow circular dependencies, and Guice also attempts to cleanup failed JIT bindings (to remove these uninitialized JIT bindings). The JIT binding cache was a guard against a recursive call back into the constructor's cache, but we were removing that guard. We now only remove the guard if we aren't in the process of loading that JIT binding. The failed JIT binding is naturally cleaned up later on, as the existing & new tests attest to. Fixes many issues: - Fixes #1633 - Fixes #785 - Fixes #1389 - Fixes #1394 Many thanks to @swankjesse for the test-case added in #1389 that was helpful in diagnosing the problem, and to @PaulFridrick for the diagnoses in #1633. PiperOrigin-RevId: 525135213
From SmokejumperIT on November 29, 2013 14:58:27
This is using Guice 3.0. When we have multiple injections of the same class happening at the same time, I'm encountering a deadlock here:
java.lang.Thread.State: WAITING (on object monitor)
at java.lang.Object.wait(Native Method)
at java.lang.Object.wait(Object.java:503)
at com.google.inject.internal.util.$MapMaker$StrategyImpl.waitForValue(MapMaker.java:529)
- locked <0x00000000ea6f4248> (a com.google.inject.internal.util.$MapMaker$LinkedStrongEntry)
at com.google.inject.internal.util.$MapMaker$StrategyImpl$FutureValueReference.waitForValue(MapMaker.java:619)
at com.google.inject.internal.util.$MapMaker$StrategyImpl.waitForValue(MapMaker.java:533)
at com.google.inject.internal.util.$MapMaker$StrategyImpl.waitForValue(MapMaker.java:419)
at com.google.inject.internal.util.$CustomConcurrentHashMap$ComputingImpl.get(CustomConcurrentHashMap.java:2061)
at com.google.inject.internal.FailableCache.get(FailableCache.java:50)
at com.google.inject.internal.MembersInjectorStore.get(MembersInjectorStore.java:65)
at com.google.inject.internal.InjectorImpl.getMembersInjector(InjectorImpl.java:950)
at com.google.inject.internal.InjectorImpl.getMembersInjector(InjectorImpl.java:957)
at com.google.inject.internal.InjectorImpl.injectMembers(InjectorImpl.java:943)
If there's any more information that I can provide to you, let me know.
I'm going to try updating to 4.0-beta to see if that solves the problem.
I did a cursory look of the open issues, and didn't see the bug. If it's a duplicate, please just let me know what the duplicated issue is.
Thanks!
(BTW, love Guice. Makes DI not suck, which is always a nice touch.)
Original issue: http://code.google.com/p/google-guice/issues/detail?id=785
The text was updated successfully, but these errors were encountered: