diff --git a/library/src/main/java/com/bumptech/glide/Glide.java b/library/src/main/java/com/bumptech/glide/Glide.java index 4fc4b71ef4..bc7751556c 100644 --- a/library/src/main/java/com/bumptech/glide/Glide.java +++ b/library/src/main/java/com/bumptech/glide/Glide.java @@ -137,18 +137,21 @@ public static Glide get(@NonNull Context context) { } @GuardedBy("Glide.class") - private static void checkAndInitializeGlide( + @VisibleForTesting + static void checkAndInitializeGlide( @NonNull Context context, @Nullable GeneratedAppGlideModule generatedAppGlideModule) { // In the thread running initGlide(), one or more classes may call Glide.get(context). // Without this check, those calls could trigger infinite recursion. if (isInitializing) { throw new IllegalStateException( - "You cannot call Glide.get() in registerComponents()," - + " use the provided Glide instance instead"); + "Glide has been called recursively, this is probably an internal library error!"); } isInitializing = true; - initializeGlide(context, generatedAppGlideModule); - isInitializing = false; + try { + initializeGlide(context, generatedAppGlideModule); + } finally { + isInitializing = false; + } } /** diff --git a/library/src/main/java/com/bumptech/glide/RegistryFactory.java b/library/src/main/java/com/bumptech/glide/RegistryFactory.java index a8b7462db5..3ec3e961ed 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/library/src/main/java/com/bumptech/glide/RegistryFactory.java @@ -90,22 +90,24 @@ static GlideSupplier lazilyCreateAndInitializeRegistry( final List manifestModules, @Nullable final AppGlideModule annotationGeneratedModule) { return new GlideSupplier() { - private boolean isInitializingOrInitialized; + // Rely on callers using memoization if they want to avoid duplicate work, but + // rely on ourselves to verify that no recursive initialization occurs. + private boolean isInitializing; @Override public Registry get() { - if (isInitializingOrInitialized) { + if (isInitializing) { throw new IllegalStateException( "Recursive Registry initialization! In your" + " AppGlideModule and LibraryGlideModules, Make sure you're using the provided " + "Registry rather calling glide.getRegistry()!"); } - isInitializingOrInitialized = true; - Trace.beginSection("Glide registry"); + isInitializing = true; try { return createAndInitRegistry(glide, manifestModules, annotationGeneratedModule); } finally { + isInitializing = false; Trace.endSection(); } } diff --git a/library/test/src/test/java/com/bumptech/glide/InitializeGlideTest.java b/library/test/src/test/java/com/bumptech/glide/InitializeGlideTest.java new file mode 100644 index 0000000000..e4e4b5907f --- /dev/null +++ b/library/test/src/test/java/com/bumptech/glide/InitializeGlideTest.java @@ -0,0 +1,70 @@ +package com.bumptech.glide; + +import static org.junit.Assert.assertThrows; + +import android.content.Context; +import androidx.annotation.NonNull; +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.bumptech.glide.tests.TearDownGlide; +import java.util.Set; +import org.junit.Rule; +import org.junit.Test; +import org.junit.function.ThrowingRunnable; +import org.junit.runner.RunWith; + +// This test is about edge cases that might otherwise make debugging more challenging. +@RunWith(AndroidJUnit4.class) +public class InitializeGlideTest { + @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + private final Context context = ApplicationProvider.getApplicationContext(); + private static final class TestException extends RuntimeException { + private static final long serialVersionUID = 2515021766931124927L; + } + + @Test + public void initialize_whenInternalMethodThrows_throwsException() { + assertThrows( + TestException.class, + new ThrowingRunnable() { + @Override + public void run() { + synchronized (Glide.class) { + Glide.checkAndInitializeGlide( + context, + new GeneratedAppGlideModule() { + @NonNull + @Override + Set> getExcludedModuleClasses() { + throw new TestException(); + } + }); + } + } + }); + } + + @Test + public void initialize_whenInternalMethodThrows_andCalledTwice_throwsException() { + GeneratedAppGlideModule throwingGeneratedAppGlideModule = + new GeneratedAppGlideModule() { + @NonNull + @Override + Set> getExcludedModuleClasses() { + throw new TestException(); + } + }; + ThrowingRunnable initializeGlide = new ThrowingRunnable() { + @Override + public void run() throws Throwable { + synchronized (Glide.class) { + Glide.checkAndInitializeGlide(context, throwingGeneratedAppGlideModule); + } + } + }; + + assertThrows(TestException.class, initializeGlide); + // Make sure the second exception isn't hidden by some Glide initialization related exception. + assertThrows(TestException.class, initializeGlide); + } +} diff --git a/library/test/src/test/java/com/bumptech/glide/RegistryFactoryTest.java b/library/test/src/test/java/com/bumptech/glide/RegistryFactoryTest.java new file mode 100644 index 0000000000..6591418e34 --- /dev/null +++ b/library/test/src/test/java/com/bumptech/glide/RegistryFactoryTest.java @@ -0,0 +1,63 @@ +package com.bumptech.glide; + +import static org.junit.Assert.assertThrows; + +import android.content.Context; +import androidx.annotation.NonNull; +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.bumptech.glide.module.AppGlideModule; +import com.bumptech.glide.tests.TearDownGlide; +import com.bumptech.glide.util.GlideSuppliers.GlideSupplier; +import com.google.common.collect.ImmutableList; +import org.junit.Rule; +import org.junit.Test; +import org.junit.function.ThrowingRunnable; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class RegistryFactoryTest { + @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + private final Context context = ApplicationProvider.getApplicationContext(); + + private static final class TestException extends RuntimeException { + private static final long serialVersionUID = 2334956185897161236L; + } + + @Test + public void create_whenCalledTwiceWithThrowingModule_throwsOriginalException() { + AppGlideModule throwingAppGlideModule = + new AppGlideModule() { + @Override + public void registerComponents(@NonNull Context context, @NonNull Glide glide, + @NonNull Registry registry) { + throw new TestException(); + } + }; + + Glide glide = Glide.get(context); + GlideSupplier registrySupplier = + RegistryFactory.lazilyCreateAndInitializeRegistry( + glide, + /* manifestModules= */ ImmutableList.of(), + throwingAppGlideModule); + + assertThrows( + TestException.class, + new ThrowingRunnable() { + @Override + public void run() { + registrySupplier.get(); + } + }); + + assertThrows( + TestException.class, + new ThrowingRunnable() { + @Override + public void run() { + registrySupplier.get(); + } + }); + } +}