Skip to content

Commit a9e1ae2

Browse files
committed
Add back assertion on empty resource classes in ResourceCacheGenerator
Currently we're failing to load some images that are present in cache due to a race condition. I haven't had any luck looking through the code to see why this happens. I suspect a race of some sort. I'm hoping that adding the exception back in with additional logging will help me move forward. I'm also hoping that this change avoids the worst of the behavior in the bug by failing only one request, rather than all requests. I may actually have to revert back to failing all requests if I can detect via ecatcher that this exception happens but no one reports it. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=209077692
1 parent 33d698d commit a9e1ae2

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

library/src/main/java/com/bumptech/glide/Registry.java

+31-2
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@
2525
import com.bumptech.glide.provider.ResourceDecoderRegistry;
2626
import com.bumptech.glide.provider.ResourceEncoderRegistry;
2727
import com.bumptech.glide.util.pool.FactoryPools;
28+
import java.io.File;
2829
import java.util.ArrayList;
2930
import java.util.Arrays;
3031
import java.util.Collections;
32+
import java.util.HashMap;
3133
import java.util.List;
34+
import java.util.Map;
3235

3336
/**
3437
* Manages component registration to extend or replace Glide's default loading, decoding, and
@@ -530,17 +533,43 @@ public <Model, TResource, Transcode> List<Class<?>> getRegisteredResourceClasses
530533
if (result == null) {
531534
result = new ArrayList<>();
532535
List<Class<?>> dataClasses = modelLoaderRegistry.getDataClasses(modelClass);
536+
537+
Map<Class<?>, List<? extends Class<?>>> dataClassToResourceClasses = new HashMap<>();
538+
Map<Class<?>, List<Class<Transcode>>> resourceClassToTranscodeClasses = new HashMap<>();
539+
533540
for (Class<?> dataClass : dataClasses) {
534541
List<? extends Class<?>> registeredResourceClasses =
535542
decoderRegistry.getResourceClasses(dataClass, resourceClass);
543+
544+
dataClassToResourceClasses.put(dataClass, registeredResourceClasses);
545+
536546
for (Class<?> registeredResourceClass : registeredResourceClasses) {
537-
List<Class<Transcode>> registeredTranscodeClasses = transcoderRegistry
538-
.getTranscodeClasses(registeredResourceClass, transcodeClass);
547+
548+
List<Class<Transcode>> registeredTranscodeClasses =
549+
transcoderRegistry.getTranscodeClasses(registeredResourceClass, transcodeClass);
550+
551+
resourceClassToTranscodeClasses.put(registeredResourceClass, registeredTranscodeClasses);
552+
539553
if (!registeredTranscodeClasses.isEmpty() && !result.contains(registeredResourceClass)) {
540554
result.add(registeredResourceClass);
541555
}
542556
}
543557
}
558+
559+
// Throw the exception before populating the cache in the hopes that a subsequent attempt will
560+
// succeed and only one request will randomly fail. This is really debugging logic that should
561+
// go away when we find the actual cause for b/73882030.
562+
if (result.isEmpty() && !File.class.equals(transcodeClass)) {
563+
if (dataClasses.isEmpty()) {
564+
throw new IllegalStateException("Failed to find any data classes for: " + modelClass);
565+
}
566+
567+
throw new IllegalStateException(
568+
"Failed to find any resource or transcode classes for model: " + modelClass
569+
+ ", data to resource classes: " + dataClassToResourceClasses
570+
+ ", resource to transcode classes: " + resourceClassToTranscodeClasses);
571+
}
572+
544573
modelToResourceClassCache.put(modelClass, resourceClass,
545574
Collections.unmodifiableList(result));
546575
}

library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ public boolean startNext() {
5353
// TODO(b/73882030): This case gets triggered when it shouldn't. With this assertion it causes
5454
// all loads to fail. Without this assertion it causes loads to miss the disk cache
5555
// unnecessarily
56-
// throw new IllegalStateException(
57-
// "Failed to find any load path from " + helper.getModelClass() + " to "
58-
// + helper.getTranscodeClass());
56+
throw new IllegalStateException(
57+
"Failed to find any load path from " + helper.getModelClass() + " to "
58+
+ helper.getTranscodeClass());
5959
}
6060
while (modelLoaders == null || !hasNextModelLoader()) {
6161
resourceClassIndex++;

0 commit comments

Comments
 (0)