From dabffd4c2ed414d6feb2d110dfa8e572155d2851 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Thu, 9 Jan 2025 12:55:08 -0800 Subject: [PATCH] Write a test for a strange behavior of SingletonScope where proxies used to resolve cycles become non-functional if errors are thrown. PiperOrigin-RevId: 713766660 --- core/test/com/google/inject/AllTests.java | 1 - core/test/com/google/inject/ScopesTest.java | 156 ++++++++++++++++++-- 2 files changed, 147 insertions(+), 10 deletions(-) diff --git a/core/test/com/google/inject/AllTests.java b/core/test/com/google/inject/AllTests.java index b4bddd8181..672cbcf9ab 100644 --- a/core/test/com/google/inject/AllTests.java +++ b/core/test/com/google/inject/AllTests.java @@ -82,7 +82,6 @@ public static Test suite() { suite.addTestSuite(ReflectionTest.class); suite.addTestSuite(RequestInjectionTest.class); suite.addTestSuite(RequireAtInjectOnConstructorsTest.class); - suite.addTestSuite(ScopesTest.class); suite.addTestSuite(SerializationTest.class); suite.addTestSuite(SuperclassTest.class); suite.addTestSuite(TypeConversionTest.class); diff --git a/core/test/com/google/inject/ScopesTest.java b/core/test/com/google/inject/ScopesTest.java index 7fb7fa3f81..24e04fc63a 100644 --- a/core/test/com/google/inject/ScopesTest.java +++ b/core/test/com/google/inject/ScopesTest.java @@ -16,9 +16,18 @@ package com.google.inject; +import static com.google.common.truth.Truth.assertThat; import static com.google.inject.Asserts.assertContains; import static com.google.inject.name.Names.named; import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static junit.framework.Assert.assertSame; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import com.google.common.base.Joiner; import com.google.common.collect.ArrayListMultimap; @@ -50,10 +59,18 @@ import java.util.concurrent.Future; import java.util.concurrent.FutureTask; import java.util.concurrent.TimeUnit; -import junit.framework.TestCase; +import java.util.concurrent.atomic.AtomicBoolean; -/** @author crazybob@google.com (Bob Lee) */ -public class ScopesTest extends TestCase { +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * @author crazybob@google.com (Bob Lee) + */ +@RunWith(JUnit4.class) +public class ScopesTest { static final long DEADLOCK_TIMEOUT_SECONDS = 1; @@ -73,8 +90,8 @@ protected void configure() { } }; - @Override - protected void setUp() throws Exception { + @Before + public void setUp() throws Exception { AnnotatedSingleton.nextInstanceId = 0; BoundAsSingleton.nextInstanceId = 0; EagerSingleton.nextInstanceId = 0; @@ -87,6 +104,7 @@ protected void setUp() throws Exception { ProvidedByAnnotatedSingleton.nextInstanceId = 0; } + @Test public void testSingletons() { Injector injector = Guice.createInjector(singletonsModule); @@ -123,6 +141,7 @@ public void testSingletons() { injector.getInstance(ProvidedByAnnotatedSingleton.class)); } + @Test public void testJustInTimeAnnotatedSingleton() { Injector injector = Guice.createInjector(); @@ -135,6 +154,7 @@ public void testJustInTimeAnnotatedSingleton() { injector.getInstance(ProvidedByAnnotatedSingleton.class)); } + @Test public void testSingletonIsPerInjector() { assertNotSame( Guice.createInjector().getInstance(AnnotatedSingleton.class), @@ -144,6 +164,7 @@ public void testSingletonIsPerInjector() { Guice.createInjector().getInstance(ProvidedByAnnotatedSingleton.class)); } + @Test public void testOverriddingAnnotation() { Injector injector = Guice.createInjector( @@ -164,6 +185,7 @@ protected void configure() { injector.getInstance(ProvidedByAnnotatedSingleton.class)); } + @Test public void testScopingAnnotationsOnAbstractTypeViaBind() { try { Guice.createInjector( @@ -199,6 +221,7 @@ interface ComponentAnnotationTest {} static class ComponentAnnotationTestImpl implements ComponentAnnotationTest {} + @Test public void testScopingAnnotationsOnAbstractTypeIsValidForComponent() { Guice.createInjector( new AbstractModule() { @@ -209,6 +232,7 @@ protected void configure() { }); } + @Test public void testScopingAnnotationsOnAbstractTypeViaImplementedBy() { try { Guice.createInjector().getInstance(D.class); @@ -229,6 +253,7 @@ interface D {} static class DImpl implements D {} + @Test public void testScopingAnnotationsOnAbstractTypeViaProvidedBy() { try { Guice.createInjector().getInstance(E.class); @@ -254,6 +279,7 @@ public E get() { } } + @Test public void testScopeUsedButNotBound() { try { Guice.createInjector( @@ -279,6 +305,7 @@ static class B {} @CustomScoped static class C {} + @Test public void testSingletonsInProductionStage() { Guice.createInjector(Stage.PRODUCTION, singletonsModule); @@ -290,6 +317,7 @@ public void testSingletonsInProductionStage() { assertEquals(0, NotASingleton.nextInstanceId); } + @Test public void testSingletonsInDevelopmentStage() { Guice.createInjector(Stage.DEVELOPMENT, singletonsModule); @@ -301,14 +329,17 @@ public void testSingletonsInDevelopmentStage() { assertEquals(0, NotASingleton.nextInstanceId); } + @Test public void testSingletonScopeIsNotSerializable() throws IOException { Asserts.assertNotSerializable(Scopes.SINGLETON); } + @Test public void testNoScopeIsNotSerializable() throws IOException { Asserts.assertNotSerializable(Scopes.NO_SCOPE); } + @Test public void testUnscopedProviderWorksOutsideOfRequestedScope() { final RememberProviderScope scope = new RememberProviderScope(); @@ -344,6 +375,7 @@ protected void configure() { } } + @Test public void testScopeAnnotationWithoutRuntimeRetention() { try { Guice.createInjector(new OuterRuntimeModule()); @@ -371,6 +403,7 @@ protected void configure() { } } + @Test public void testBindScopeToAnnotationWithoutScopeAnnotation() { try { Guice.createInjector(new OuterDeprecatedModule()); @@ -406,6 +439,7 @@ protected void configure() { } } + @Test public void testBindScopeTooManyTimes() { try { Guice.createInjector(new OuterScopeModule()); @@ -421,6 +455,7 @@ public void testBindScopeTooManyTimes() { } } + @Test public void testBindDuplicateScope() { Injector injector = Guice.createInjector( @@ -437,6 +472,7 @@ protected void configure() { injector.getInstance(AnnotatedCustomScoped.class)); } + @Test public void testDuplicateScopeAnnotations() { Injector injector = Guice.createInjector( @@ -458,6 +494,7 @@ protected void configure() { } } + @Test public void testNullScopedAsASingleton() { Injector injector = Guice.createInjector( @@ -487,6 +524,7 @@ public Provider scope(Key key, Provider unscoped) { } } + @Test public void testSingletonAnnotationOnParameterizedType() { Injector injector = Guice.createInjector(); assertSame( @@ -605,6 +643,7 @@ public ProvidedByAnnotatedSingleton get() { } } + @Test public void testScopeThatGetsAnUnrelatedObject() { Injector injector = Guice.createInjector( @@ -637,6 +676,7 @@ public T get() { } } + @Test public void testIsSingletonPositive() { final Key a = Key.get(String.class, named("A")); final Key b = Key.get(String.class, named("B")); @@ -677,7 +717,7 @@ String provideG() { } }; - @SuppressWarnings("unchecked") // we know the module contains only bindings + // we know the module contains only bindings List moduleBindings = Elements.getElements(singletonBindings); ImmutableMap, Binding> map = indexBindings(moduleBindings); assertFalse(Scopes.isSingleton(map.get(a))); // linked bindings are not followed by modules @@ -702,6 +742,7 @@ String provideG() { assertTrue(Scopes.isSingleton(injector.getBinding(i))); } + @Test public void testIsSingletonNegative() { final Key a = Key.get(String.class, named("A")); final Key b = Key.get(String.class, named("B")); @@ -737,7 +778,7 @@ String provideE() { } }; - @SuppressWarnings("unchecked") // we know the module contains only bindings + // we know the module contains only bindings List moduleBindings = Elements.getElements(singletonBindings); ImmutableMap, Binding> map = indexBindings(moduleBindings); assertFalse(Scopes.isSingleton(map.get(a))); @@ -756,6 +797,7 @@ String provideE() { assertFalse(Scopes.isSingleton(injector.getBinding(f))); } + @Test public void testIsScopedPositive() { final Key a = Key.get(String.class, named("A")); final Key b = Key.get(String.class, named("B")); @@ -793,7 +835,7 @@ String provideE() { } }; - @SuppressWarnings("unchecked") // we know the module contains only bindings + // we know the module contains only bindings List moduleBindings = Elements.getElements(customBindings); ImmutableMap, Binding> map = indexBindings(moduleBindings); assertFalse(isCustomScoped(map.get(a))); // linked bindings are not followed by modules @@ -814,6 +856,7 @@ String provideE() { assertTrue(isCustomScoped(injector.getBinding(g))); } + @Test public void testIsScopedNegative() { final Key a = Key.get(String.class, named("A")); final Key b = Key.get(String.class, named("B")); @@ -852,7 +895,7 @@ String provideE() { } }; - @SuppressWarnings("unchecked") // we know the module contains only bindings + // we know the module contains only bindings List moduleBindings = Elements.getElements(customBindings); ImmutableMap, Binding> map = indexBindings(moduleBindings); assertFalse(isCustomScoped(map.get(a))); @@ -908,6 +951,7 @@ static class ThrowingSingleton { } } + @Test public void testSingletonConstructorThrows() { Injector injector = Guice.createInjector(); @@ -923,6 +967,94 @@ public void testSingletonConstructorThrows() { assertEquals(2, ThrowingSingleton.nextInstanceId); } + public interface BlockThenThrow { + void doSomething(); + } + + // Tests for a strange behavior of Guice where if a singleton fails to construct and cyclic + // proxies have been handed out, then we will never satisify those proxies. + @Test + public void testCyclicProxyForSingletonThatFailsToConstruct() throws Exception { + // use the barrier to get both threads to enter the providers at the same time + final AtomicBoolean returnReal = new AtomicBoolean(false); + Injector injector = + Guice.createInjector( + new AbstractModule() { + // The first time through they wait on the barrier, this allows us to create a nice + // small lock cycle + final AtomicBoolean firstA = new AtomicBoolean(true); + final AtomicBoolean firstB = new AtomicBoolean(true); + final CyclicBarrier enterBarrier = new CyclicBarrier(2); + + @Named("A") + @Singleton + @Provides + BlockThenThrow provideA(@Named("B") Provider b) throws Exception { + if (returnReal.get()) { + return () -> {}; + } + if (firstA.getAndSet(false)) { + enterBarrier.await(); + // This should be a cyclic proxy + return b.get(); + } + throw new RuntimeException(); + } + + @Named("B") + @Singleton + @Provides + BlockThenThrow provideB(@Named("A") Provider a) throws Exception { + if (returnReal.get()) { + return () -> {}; + } + if (firstB.getAndSet(false)) { + enterBarrier.await(); + return a.get(); + } + + throw new RuntimeException(); + } + }); + Key aKey = Key.get(BlockThenThrow.class, named("A")); + Key bKey = Key.get(BlockThenThrow.class, named("B")); + Future aThreadResult = + Executors.newSingleThreadExecutor((r) -> new Thread(r, "A")) + .submit(() -> injector.getInstance(aKey)); + Future bThreadResult = + Executors.newSingleThreadExecutor((r) -> new Thread(r, "B")) + .submit(() -> injector.getInstance(bKey)); + + // one of the two will be a proxy, the other will be an error, which is which is racy. + BlockThenThrow proxy = null; + try { + proxy = aThreadResult.get(); + } catch (ExecutionException e) { + // expected + } + try { + proxy = bThreadResult.get(); + } catch (ExecutionException e) { + // expected + } + BlockThenThrow finalProxy = proxy; + assertThat(finalProxy).isNotNull(); + // And they clearly aren't ready yet + IllegalStateException e = assertThrows(IllegalStateException.class, finalProxy::doSomething); + assertThat(e) + .hasMessageThat() + .contains("Please wait until after injection has completed to use this object."); + // Allow injection to succeed + returnReal.set(true); + injector.getInstance(aKey); + injector.getInstance(bKey); + // still throws about not being ready, it never will be + e = assertThrows(IllegalStateException.class, finalProxy::doSomething); + assertThat(e) + .hasMessageThat() + .contains("Please wait until after injection has completed to use this object."); + } + /** * Should only be created by {@link SBarrierProvider}. * @@ -970,6 +1102,7 @@ public S get() { * other creating a deadlock and await timeout. */ + @Test public void testInjectorsDontDeadlockOnSingletons() throws Exception { final Provider provider = new SBarrierProvider(2); final Injector injector = @@ -1041,6 +1174,7 @@ static class HImpl implements H { * exactly one circular proxy object is created. */ + @Test public void testSiblingInjectorGettingCircularSingletonsOneCircularProxy() throws Exception { final Provider provider = new SBarrierProvider(2); final Injector injector = @@ -1172,6 +1306,7 @@ static class K2 { * cycle. */ + @Test public void testUnresolvableSingletonCircularDependencyErrorMessage() throws Exception { final Provider provider = new SBarrierProvider(3); final Injector injector = @@ -1293,6 +1428,7 @@ protected void configure() { // Test for https://github.com/google/guice/issues/1032 + @Test public void testScopeAppliedByUserInsteadOfScoping() throws Exception { Injector injector = java.util.concurrent.Executors.newSingleThreadExecutor() @@ -1341,6 +1477,7 @@ public Void visitScope(Scope scope) { } } + @Test public void testForInstanceOfNoScopingReturnsUnscoped() { Injector injector = Guice.createInjector( @@ -1368,6 +1505,7 @@ public Boolean visitNoScoping() { })); } + @Test public void testScopedLinkedBindingDoesNotPropagateEagerSingleton() { final Key a = Key.get(String.class, named("A")); final Key b = Key.get(String.class, named("B"));