Repository navigation
Description
In some scenario the cycle detection leads the application to an OutOfMemoryError.
Specifically, this seems to happen if:
- At least one object O involved in a cyclic dependency is a Singleton
- O has not been initialized yet
- Multiple threads require the injection of O at the same time
The following test reproduces the issue (not always but often)
import java.util.concurrent.CountDownLatch;
import org.junit.Test;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Singleton;
public class GuiceCircularDependenciesIT extends AbstractModule {
static interface Store {
void sell();
}
static interface Customer {
void buy();
}
static class StoreImpl implements Store {
private final Customer customer;
@Inject public StoreImpl(Customer customer) {
this.customer = customer;
}
@Override
public void sell() { }
}
static class CustomerImpl implements Customer {
private Store store;
@Inject public CustomerImpl(Store store) {
this.store = store;
}
@Override
public void buy() {
store.sell();
}
}
@Override
protected void configure() {
bind(Store.class).to(StoreImpl.class);
bind(Customer.class).to(CustomerImpl.class).in(Singleton.class);
}
@Test
public void testCircularDependencies() {
for (int j = 0; j < 100; ++j) {
Injector injector = Guice.createInjector(this);
CountDownLatch latch = new CountDownLatch(8);
for (int i = 0; i < 8; ++i) {
Thread thread = new Thread(() -> {
try {
Customer instance = injector.getInstance(Customer.class);
instance.buy();
} catch (Exception e) {
System.out.println(e);
} finally {
latch.countDown();
}
});
thread.setDaemon(true);
thread.start();
}
try {
latch.await();
System.out.println("DONE: " + j);
} catch (InterruptedException e) {
}
}
}
}a sample output, when the issue occurs, is
DONE: 0
DONE: 1
DONE: 2
DONE: 3
DONE: 4
Exception in thread "Thread-47" java.lang.OutOfMemoryError: Java heap space
at java.base/java.util.Arrays.copyOf(Arrays.java:3689)
at java.base/java.util.ArrayList.grow(ArrayList.java:238)
at java.base/java.util.ArrayList.grow(ArrayList.java:243)
at java.base/java.util.ArrayList.add(ArrayList.java:486)
at java.base/java.util.ArrayList.add(ArrayList.java:499)
at com.google.common.collect.AbstractMapBasedMultimap.put(AbstractMapBasedMultimap.java:195)
at com.google.common.collect.AbstractListMultimap.put(AbstractListMultimap.java:115)
at com.google.inject.internal.CycleDetectingLock$CycleDetectingLockFactory$ReentrantCycleDetectingLock.addAllLockIdsAfter(CycleDetectingLock.java:290)
at com.google.inject.internal.CycleDetectingLock$CycleDetectingLockFactory$ReentrantCycleDetectingLock.detectPotentialLocksCycle(CycleDetectingLock.java:257)
at com.google.inject.internal.CycleDetectingLock$CycleDetectingLockFactory$ReentrantCycleDetectingLock.lockOrDetectPotentialLocksCycle(CycleDetectingLock.java:149)
at com.google.inject.internal.SingletonScope$1.get(SingletonScope.java:159)
at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:39)
at com.google.inject.internal.InjectorImpl$1.get(InjectorImpl.java:1094)
at com.google.inject.internal.InjectorImpl.getInstance(InjectorImpl.java:1131)
at foo.GuiceCircularDependenciesIT.lambda$0(GuiceCircularDependenciesIT.java:63)
at foo.GuiceCircularDependenciesIT$$Lambda$30/0x00000008000b9040.run(Unknown Source)
at java.base/java.lang.Thread.run(Thread.java:834)
I'm running version 4.2.3
Please note that, unless I'm doing something wrong, this makes Guice default settings harmful if you are running it in a multi-thread application.
As a matter of fact, the only way to fix the code above is to remove the cyclic dependency, and with the circular proxy feature enabled (and it is by default) it becomes harder to spot it. Nobody (running a multi-thread application) reasonably wants to take the risk of getting an OutOfMemory Error - which is typically fatal for the application - based on race conditions - which can be hard to find while testing.
Most likely, as long as the bug is there, whoever runs a multi-thread application may want to disable the feature completely, for example calling Binder::disableCircularProxies(), to override the default behavior.
Activity
[-]Cycle detection in a multi-threaded environment leads to OutOfMemoryError[/-][+]Cycle detection in a multi-thread environment leads to OutOfMemoryError[/+]cdietrich commentedon Aug 2, 2021
we are also affected by this issue
cdietrich commentedon Jun 9, 2022
any update here?
trancexpress commentedon Jun 10, 2022
Using the reproducer from the description, bisected this to:
5e6c933
This is the first commit I see the OOM with, I imagine something in the changes in
com.google.inject.internal.SingletonScopeis causing the OOM. I can do partial reverts, to not see the OOM:trancexpress commentedon Jun 10, 2022
Hi @sameb ,
I see you are still active and were part of the original PR that caused this OOM: #915
Could you take a look here? Is there maybe some oversight, that causes an endless loop in
ReentrantCycleDetectingLock.detectPotentialLocksCycle()? I'll try to check myself but this will likely take a while.Best regards and thanks,
Simeon Andreev
Prevent endless loop in lock cycle detection google#1510
This change prevents an endless loop in: ReentrantCycleDetectingLock.…
This change prevents an endless loop in: ReentrantCycleDetectingLock.…
This change prevents an endless loop in: ReentrantCycleDetectingLock.…
Prevent endless loop in lock cycle detection. The following scenario …
Prevent endless loop in lock cycle detection. The following scenario …
Prevent endless loop in lock cycle detection. The following scenario …
trancexpress commentedon Apr 18, 2023
Thanks for fixing this!
sameb commentedon Apr 18, 2023
You're welcome. Sorry this was left unfixed for so long. I'm spending this & next week cleaning up Guice's PR and issue backlog, as a tribute to @crazybob.
trancexpress commentedon Apr 18, 2023
I did not know, this is very saddening :(