diff --git a/library/src/main/java/com/bumptech/glide/Registry.java b/library/src/main/java/com/bumptech/glide/Registry.java index 97ef93a87a84a8af70bf835edb2f9a3f04e7baa8..2f51117e169657f27b180f2470b8f037038e7bff 100644 --- a/library/src/main/java/com/bumptech/glide/Registry.java +++ b/library/src/main/java/com/bumptech/glide/Registry.java @@ -50,10 +50,10 @@ public class Registry { private final ModelToResourceClassCache modelToResourceClassCache = new ModelToResourceClassCache(); private final LoadPathCache loadPathCache = new LoadPathCache(); - private final Pool> exceptionListPool = FactoryPools.threadSafeList(); + private final Pool> throwableListPool = FactoryPools.threadSafeList(); public Registry() { - this.modelLoaderRegistry = new ModelLoaderRegistry(exceptionListPool); + this.modelLoaderRegistry = new ModelLoaderRegistry(throwableListPool); this.encoderRegistry = new EncoderRegistry(); this.decoderRegistry = new ResourceDecoderRegistry(); this.resourceEncoderRegistry = new ResourceEncoderRegistry(); @@ -454,8 +454,9 @@ public class Registry { if (decodePaths.isEmpty()) { result = null; } else { - result = new LoadPath<>(dataClass, resourceClass, transcodeClass, decodePaths, - exceptionListPool); + result = + new LoadPath<>( + dataClass, resourceClass, transcodeClass, decodePaths, throwableListPool); } loadPathCache.put(dataClass, resourceClass, transcodeClass, result); } @@ -480,7 +481,7 @@ public class Registry { ResourceTranscoder transcoder = transcoderRegistry.get(registeredResourceClass, registeredTranscodeClass); decodePaths.add(new DecodePath<>(dataClass, registeredResourceClass, - registeredTranscodeClass, decoders, transcoder, exceptionListPool)); + registeredTranscodeClass, decoders, transcoder, throwableListPool)); } } return decodePaths; diff --git a/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java b/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java index 9bc35e27bc1e4a031bc9e2105a72fab8d10e58e2..4e7d1c38f845a9d963ebdfa3740f045baad55561 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java @@ -41,7 +41,7 @@ class DecodeJob implements DataFetcherGenerator.FetcherReadyCallback, private static final String TAG = "DecodeJob"; @Synthetic final DecodeHelper decodeHelper = new DecodeHelper<>(); - private final List exceptions = new ArrayList<>(); + private final List throwables = new ArrayList<>(); private final StateVerifier stateVerifier = StateVerifier.newInstance(); private final DiskCacheProvider diskCacheProvider; private final Pools.Pool> pool; @@ -187,7 +187,7 @@ class DecodeJob implements DataFetcherGenerator.FetcherReadyCallback, currentFetcher = null; startFetchTime = 0L; isCancelled = false; - exceptions.clear(); + throwables.clear(); pool.release(this); } @@ -227,19 +227,25 @@ class DecodeJob implements DataFetcherGenerator.FetcherReadyCallback, return; } runWrapped(); - } catch (RuntimeException e) { + } catch (Throwable t) { + // Catch Throwable and not Exception to handle OOMs. Throwables are swallowed by our + // usage of .submit() in GlideExecutor so we're not silently hiding crashes by doing this. We + // are however ensuring that our callbacks are always notified when a load fails. Without this + // notification, uncaught throwables never notify the corresponding callbacks, which can cause + // loads to silently hang forever, a case that's especially bad for users using Futures on + // background threads. if (Log.isLoggable(TAG, Log.DEBUG)) { Log.d(TAG, "DecodeJob threw unexpectedly" + ", isCancelled: " + isCancelled - + ", stage: " + stage, e); + + ", stage: " + stage, t); } // When we're encoding we've already notified our callback and it isn't safe to do so again. if (stage != Stage.ENCODE) { - exceptions.add(e); + throwables.add(t); notifyFailed(); } if (!isCancelled) { - throw e; + throw t; } } finally { // Keeping track of the fetcher here and calling cleanup is excessively paranoid, we call @@ -309,7 +315,7 @@ class DecodeJob implements DataFetcherGenerator.FetcherReadyCallback, private void notifyFailed() { setNotifiedOrThrow(); - GlideException e = new GlideException("Failed to load resource", new ArrayList<>(exceptions)); + GlideException e = new GlideException("Failed to load resource", new ArrayList<>(throwables)); callback.onLoadFailed(e); onLoadFailed(); } @@ -379,7 +385,7 @@ class DecodeJob implements DataFetcherGenerator.FetcherReadyCallback, fetcher.cleanup(); GlideException exception = new GlideException("Fetching data failed", e); exception.setLoggingDetails(attemptedKey, dataSource, fetcher.getDataClass()); - exceptions.add(exception); + throwables.add(exception); if (Thread.currentThread() != currentThread) { runReason = RunReason.SWITCH_TO_SOURCE_SERVICE; callback.reschedule(this); @@ -400,7 +406,7 @@ class DecodeJob implements DataFetcherGenerator.FetcherReadyCallback, resource = decodeFromData(currentFetcher, currentData, currentDataSource); } catch (GlideException e) { e.setLoggingDetails(currentAttemptingKey, currentDataSource); - exceptions.add(e); + throwables.add(e); } if (resource != null) { notifyEncodeAndRelease(resource, currentDataSource); diff --git a/library/src/main/java/com/bumptech/glide/load/engine/DecodePath.java b/library/src/main/java/com/bumptech/glide/load/engine/DecodePath.java index e74320bf0b10393337c5c75398f6a325dfd3ef51..a65eccdcdeada138ca5e91fbcf55bcfa7289a7d2 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/DecodePath.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/DecodePath.java @@ -23,13 +23,13 @@ public class DecodePath { private final Class dataClass; private final List> decoders; private final ResourceTranscoder transcoder; - private final Pool> listPool; + private final Pool> listPool; private final String failureMessage; public DecodePath(Class dataClass, Class resourceClass, Class transcodeClass, List> decoders, - ResourceTranscoder transcoder, Pool> listPool) { + ResourceTranscoder transcoder, Pool> listPool) { this.dataClass = dataClass; this.decoders = decoders; this.transcoder = transcoder; @@ -47,7 +47,7 @@ public class DecodePath { private Resource decodeResource(DataRewinder rewinder, int width, int height, Options options) throws GlideException { - List exceptions = listPool.acquire(); + List exceptions = listPool.acquire(); try { return decodeResourceWithList(rewinder, width, height, options, exceptions); } finally { @@ -56,7 +56,7 @@ public class DecodePath { } private Resource decodeResourceWithList(DataRewinder rewinder, int width, - int height, Options options, List exceptions) throws GlideException { + int height, Options options, List exceptions) throws GlideException { Resource result = null; for (int i = 0, size = decoders.size(); i < size; i++) { ResourceDecoder decoder = decoders.get(i); @@ -68,7 +68,7 @@ public class DecodePath { } // Some decoders throw unexpectedly. If they do, we shouldn't fail the entire load path, but // instead log and continue. See #2406 for an example. - } catch (IOException | RuntimeException e) { + } catch (IOException | RuntimeException | OutOfMemoryError e) { if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "Failed to decode data for " + decoder, e); } diff --git a/library/src/main/java/com/bumptech/glide/load/engine/GlideException.java b/library/src/main/java/com/bumptech/glide/load/engine/GlideException.java index f932aac27d2c5214fa0dbd69d7f35490fb63d76e..059e5f0e2e9f05c6592964cf33b593ddd75b8d4a 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/GlideException.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/GlideException.java @@ -18,20 +18,20 @@ import java.util.List; public final class GlideException extends Exception { private static final StackTraceElement[] EMPTY_ELEMENTS = new StackTraceElement[0]; - private final List causes; + private final List causes; private Key key; private DataSource dataSource; private Class dataClass; public GlideException(String message) { - this(message, Collections.emptyList()); + this(message, Collections.emptyList()); } - public GlideException(String detailMessage, Exception cause) { + public GlideException(String detailMessage, Throwable cause) { this(detailMessage, Collections.singletonList(cause)); } - public GlideException(String detailMessage, List causes) { + public GlideException(String detailMessage, List causes) { super(detailMessage); setStackTrace(EMPTY_ELEMENTS); this.causes = causes; @@ -62,7 +62,7 @@ public final class GlideException extends Exception { * * @see #getRootCauses() */ - public List getCauses() { + public List getCauses() { return causes; } @@ -74,8 +74,8 @@ public final class GlideException extends Exception { * a given model using multiple different pathways, there may be multiple related or unrelated * reasons for a load to fail. */ - public List getRootCauses() { - List rootCauses = new ArrayList<>(); + public List getRootCauses() { + List rootCauses = new ArrayList<>(); addRootCauses(this, rootCauses); return rootCauses; } @@ -88,20 +88,20 @@ public final class GlideException extends Exception { * complete stack traces. */ public void logRootCauses(String tag) { - List causes = getRootCauses(); + List causes = getRootCauses(); for (int i = 0, size = causes.size(); i < size; i++) { Log.i(tag, "Root cause (" + (i + 1) + " of " + size + ")", causes.get(i)); } } - private void addRootCauses(Exception exception, List rootCauses) { - if (exception instanceof GlideException) { - GlideException glideException = (GlideException) exception; - for (Exception e : glideException.getCauses()) { - addRootCauses(e, rootCauses); + private void addRootCauses(Throwable throwable, List rootCauses) { + if (throwable instanceof GlideException) { + GlideException glideException = (GlideException) throwable; + for (Throwable t : glideException.getCauses()) { + addRootCauses(t, rootCauses); } } else { - rootCauses.add(exception); + rootCauses.add(throwable); } } @@ -136,18 +136,18 @@ public final class GlideException extends Exception { // Appendable throws, PrintWriter, PrintStream, and IndentedAppendable do not, so this should // never happen. @SuppressWarnings("PMD.PreserveStackTrace") - private static void appendExceptionMessage(Exception e, Appendable appendable) { + private static void appendExceptionMessage(Throwable t, Appendable appendable) { try { - appendable.append(e.getClass().toString()).append(": ").append(e.getMessage()).append('\n'); + appendable.append(t.getClass().toString()).append(": ").append(t.getMessage()).append('\n'); } catch (IOException e1) { - throw new RuntimeException(e); + throw new RuntimeException(t); } } // Appendable throws, PrintWriter, PrintStream, and IndentedAppendable do not, so this should // never happen. @SuppressWarnings("PMD.PreserveStackTrace") - private static void appendCauses(List causes, Appendable appendable) { + private static void appendCauses(List causes, Appendable appendable) { try { appendCausesWrapped(causes, appendable); } catch (IOException e) { @@ -156,7 +156,7 @@ public final class GlideException extends Exception { } @SuppressWarnings("ThrowableResultOfMethodCallIgnored") - private static void appendCausesWrapped(List causes, Appendable appendable) + private static void appendCausesWrapped(List causes, Appendable appendable) throws IOException { int size = causes.size(); for (int i = 0; i < size; i++) { @@ -166,7 +166,7 @@ public final class GlideException extends Exception { .append(String.valueOf(size)) .append("): "); - Exception cause = causes.get(i); + Throwable cause = causes.get(i); if (cause instanceof GlideException) { GlideException glideCause = (GlideException) cause; glideCause.printStackTrace(appendable); diff --git a/library/src/main/java/com/bumptech/glide/load/engine/LoadPath.java b/library/src/main/java/com/bumptech/glide/load/engine/LoadPath.java index d182da6ea27fc7dde5156ea74ed521945c570ac9..b747da7aa4dfa021d2dbe2705df92378ec6149a7 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/LoadPath.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/LoadPath.java @@ -21,13 +21,13 @@ import java.util.List; */ public class LoadPath { private final Class dataClass; - private final Pool> listPool; + private final Pool> listPool; private final List> decodePaths; private final String failureMessage; public LoadPath(Class dataClass, Class resourceClass, Class transcodeClass, - List> decodePaths, Pool> listPool) { + List> decodePaths, Pool> listPool) { this.dataClass = dataClass; this.listPool = listPool; this.decodePaths = Preconditions.checkNotEmpty(decodePaths); @@ -37,17 +37,17 @@ public class LoadPath { public Resource load(DataRewinder rewinder, Options options, int width, int height, DecodePath.DecodeCallback decodeCallback) throws GlideException { - List exceptions = listPool.acquire(); + List throwables = listPool.acquire(); try { - return loadWithExceptionList(rewinder, options, width, height, decodeCallback, exceptions); + return loadWithExceptionList(rewinder, options, width, height, decodeCallback, throwables); } finally { - listPool.release(exceptions); + listPool.release(throwables); } } private Resource loadWithExceptionList(DataRewinder rewinder, Options options, int width, int height, DecodePath.DecodeCallback decodeCallback, - List exceptions) throws GlideException { + List exceptions) throws GlideException { int size = decodePaths.size(); Resource result = null; for (int i = 0; i < size; i++) { diff --git a/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java b/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java index 77923f2c4c6b7b7e600a6e2da5ea1d6137d756a6..87cb135ef2118b9a991efb2eb44fb295ac4df8d2 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java +++ b/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java @@ -17,8 +17,8 @@ public class ModelLoaderRegistry { private final MultiModelLoaderFactory multiModelLoaderFactory; private final ModelLoaderCache cache = new ModelLoaderCache(); - public ModelLoaderRegistry(Pool> exceptionListPool) { - this(new MultiModelLoaderFactory(exceptionListPool)); + public ModelLoaderRegistry(Pool> throwableListPool) { + this(new MultiModelLoaderFactory(throwableListPool)); } // Visible for testing. diff --git a/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java b/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java index d18d12e46d43014324bb5366156cf116e38c13a4..bae38759fefd027b95f617313ac50a44cea6e970 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java +++ b/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoader.java @@ -27,10 +27,10 @@ import java.util.List; class MultiModelLoader implements ModelLoader { private final List> modelLoaders; - private final Pool> exceptionListPool; + private final Pool> exceptionListPool; MultiModelLoader(List> modelLoaders, - Pool> exceptionListPool) { + Pool> exceptionListPool) { this.modelLoaders = modelLoaders; this.exceptionListPool = exceptionListPool; } @@ -74,15 +74,15 @@ class MultiModelLoader implements ModelLoader { static class MultiFetcher implements DataFetcher, DataCallback { private final List> fetchers; - private final Pool> exceptionListPool; + private final Pool> throwableListPool; private int currentIndex; private Priority priority; private DataCallback callback; @Nullable - private List exceptions; + private List exceptions; - MultiFetcher(List> fetchers, Pool> exceptionListPool) { - this.exceptionListPool = exceptionListPool; + MultiFetcher(List> fetchers, Pool> throwableListPool) { + this.throwableListPool = throwableListPool; Preconditions.checkNotEmpty(fetchers); this.fetchers = fetchers; currentIndex = 0; @@ -92,14 +92,14 @@ class MultiModelLoader implements ModelLoader { public void loadData(Priority priority, DataCallback callback) { this.priority = priority; this.callback = callback; - exceptions = exceptionListPool.acquire(); + exceptions = throwableListPool.acquire(); fetchers.get(currentIndex).loadData(priority, this); } @Override public void cleanup() { if (exceptions != null) { - exceptionListPool.release(exceptions); + throwableListPool.release(exceptions); } exceptions = null; for (DataFetcher fetcher : fetchers) { diff --git a/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoaderFactory.java b/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoaderFactory.java index 4c63136a50f45c4920383d57b5acfa29aa666753..56e586f586e9f7966f4487ed7172b6952c03bbeb 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoaderFactory.java +++ b/library/src/main/java/com/bumptech/glide/load/model/MultiModelLoaderFactory.java @@ -22,16 +22,16 @@ public class MultiModelLoaderFactory { private final List> entries = new ArrayList<>(); private final Factory factory; private final Set> alreadyUsedEntries = new HashSet<>(); - private final Pool> exceptionListPool; + private final Pool> throwableListPool; - public MultiModelLoaderFactory(Pool> exceptionListPool) { - this(exceptionListPool, DEFAULT_FACTORY); + public MultiModelLoaderFactory(Pool> throwableListPool) { + this(throwableListPool, DEFAULT_FACTORY); } // Visible for testing. - MultiModelLoaderFactory(Pool> exceptionListPool, + MultiModelLoaderFactory(Pool> throwableListPool, Factory factory) { - this.exceptionListPool = exceptionListPool; + this.throwableListPool = throwableListPool; this.factory = factory; } @@ -128,7 +128,7 @@ public class MultiModelLoaderFactory { } } if (loaders.size() > 1) { - return factory.build(loaders, exceptionListPool); + return factory.build(loaders, throwableListPool); } else if (loaders.size() == 1) { return loaders.get(0); } else { @@ -185,8 +185,8 @@ public class MultiModelLoaderFactory { static class Factory { public MultiModelLoader build( - List> modelLoaders, Pool> exceptionListPool) { - return new MultiModelLoader<>(modelLoaders, exceptionListPool); + List> modelLoaders, Pool> throwableListPool) { + return new MultiModelLoader<>(modelLoaders, throwableListPool); } } diff --git a/library/src/test/java/com/bumptech/glide/load/model/MultiModelLoaderFactoryTest.java b/library/src/test/java/com/bumptech/glide/load/model/MultiModelLoaderFactoryTest.java index b6acd775c12109ca45fb249cc3cb7d50ef121372..8e9c6c46ce3963a7da03ff217d6bbceda7a60662 100644 --- a/library/src/test/java/com/bumptech/glide/load/model/MultiModelLoaderFactoryTest.java +++ b/library/src/test/java/com/bumptech/glide/load/model/MultiModelLoaderFactoryTest.java @@ -35,16 +35,16 @@ public class MultiModelLoaderFactoryTest { @Rule public ExpectedException exception = ExpectedException.none(); - private Pool> exceptionListPool; + private Pool> throwableListPool; private MultiModelLoaderFactory multiFactory; @Before public void setUp() { MockitoAnnotations.initMocks(this); - exceptionListPool = FactoryPools.threadSafeList(); + throwableListPool = FactoryPools.threadSafeList(); - multiFactory = new MultiModelLoaderFactory(exceptionListPool, - multiModelLoaderFactory); + multiFactory = + new MultiModelLoaderFactory(throwableListPool, multiModelLoaderFactory); when(firstFactory.build(eq(multiFactory))).thenReturn(firstModelLoader); when(secondFactory.build(eq(multiFactory))).thenReturn(secondModelLoader); } @@ -268,7 +268,7 @@ public class MultiModelLoaderFactoryTest { Class dataClass) { ArgumentCaptor>> captor = Util.cast(ArgumentCaptor.forClass(List.class)); multiFactory.build(modelClass, dataClass); - verify(multiModelLoaderFactory).build(captor.capture(), eq(exceptionListPool)); + verify(multiModelLoaderFactory).build(captor.capture(), eq(throwableListPool)); List> captured = captor.getValue(); List> result = new ArrayList<>(captured.size());