Skip to content

Commit

Permalink
Automated g4 rollback of changelist 209077692.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

All requests still end up failing.

*** Original change description ***

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....

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=210027336
  • Loading branch information
sjudd committed Aug 30, 2018
1 parent 37127f0 commit d18e5e8
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 34 deletions.
33 changes: 2 additions & 31 deletions library/src/main/java/com/bumptech/glide/Registry.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,10 @@
import com.bumptech.glide.provider.ResourceDecoderRegistry;
import com.bumptech.glide.provider.ResourceEncoderRegistry;
import com.bumptech.glide.util.pool.FactoryPools;
import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* Manages component registration to extend or replace Glide's default loading, decoding, and
Expand Down Expand Up @@ -533,43 +530,17 @@ public <Model, TResource, Transcode> List<Class<?>> getRegisteredResourceClasses
if (result == null) {
result = new ArrayList<>();
List<Class<?>> dataClasses = modelLoaderRegistry.getDataClasses(modelClass);

Map<Class<?>, List<? extends Class<?>>> dataClassToResourceClasses = new HashMap<>();
Map<Class<?>, List<Class<Transcode>>> resourceClassToTranscodeClasses = new HashMap<>();

for (Class<?> dataClass : dataClasses) {
List<? extends Class<?>> registeredResourceClasses =
decoderRegistry.getResourceClasses(dataClass, resourceClass);

dataClassToResourceClasses.put(dataClass, registeredResourceClasses);

for (Class<?> registeredResourceClass : registeredResourceClasses) {

List<Class<Transcode>> registeredTranscodeClasses =
transcoderRegistry.getTranscodeClasses(registeredResourceClass, transcodeClass);

resourceClassToTranscodeClasses.put(registeredResourceClass, registeredTranscodeClasses);

List<Class<Transcode>> registeredTranscodeClasses = transcoderRegistry
.getTranscodeClasses(registeredResourceClass, transcodeClass);
if (!registeredTranscodeClasses.isEmpty() && !result.contains(registeredResourceClass)) {
result.add(registeredResourceClass);
}
}
}

// Throw the exception before populating the cache in the hopes that a subsequent attempt will
// succeed and only one request will randomly fail. This is really debugging logic that should
// go away when we find the actual cause for b/73882030.
if (result.isEmpty() && !File.class.equals(transcodeClass)) {
if (dataClasses.isEmpty()) {
throw new IllegalStateException("Failed to find any data classes for: " + modelClass);
}

throw new IllegalStateException(
"Failed to find any resource or transcode classes for model: " + modelClass
+ ", data to resource classes: " + dataClassToResourceClasses
+ ", resource to transcode classes: " + resourceClassToTranscodeClasses);
}

modelToResourceClassCache.put(modelClass, resourceClass,
Collections.unmodifiableList(result));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ public boolean startNext() {
// TODO(b/73882030): This case gets triggered when it shouldn't. With this assertion it causes
// all loads to fail. Without this assertion it causes loads to miss the disk cache
// unnecessarily
throw new IllegalStateException(
"Failed to find any load path from " + helper.getModelClass() + " to "
+ helper.getTranscodeClass());
// throw new IllegalStateException(
// "Failed to find any load path from " + helper.getModelClass() + " to "
// + helper.getTranscodeClass());
}
while (modelLoaders == null || !hasNextModelLoader()) {
resourceClassIndex++;
Expand Down

0 comments on commit d18e5e8

Please sign in to comment.