bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.59k stars 6.12k forks source link

Why does SizeConfigStrategy.decrementBitmapOfSize throws NPE instead of giving a warning and what am i doing wrong? #2735

Open dreampowder opened 6 years ago

dreampowder commented 6 years ago

Glide Version: 4.4

Integration libraries: none

Device/Android Version: Completely random

Issue details / Repro steps / Use case background: Hi there. First of all, i have been searching in issues / documentation /web for finding an answer to this question for days (I've also asked a question in SO but sadly, no one answered)
While Glide is an amazing library, it is also a bit overwhelming library at least for me to understand in such a short time.

In the project that i inherited, the app's Fabric.io dashboard flooded with NPE errors from SizeConfigStrategy.decrementBitmapOfSize. I cannot replicate this error in my test devices, (neighter our other testers) but yet this the top error in our fabric dashboard. When i check the source code i see that SizeConfigStrategy. decrementBitmapOfSize function throws this exception when it couldn't find a match. But why doesn't glide shows a warning about this error, and not throw an exception or why can't we catch the source of this exception?

I have also implemented a Glide Module with;

@com.bumptech.glide.annotation.GlideModule
public class GlideModule extends AppGlideModule {
    @Override
    public void applyOptions(Context context, GlideBuilder builder) {
        super.applyOptions(context, builder);        
        Timber.i("Configuring Glide Module!");
 builder.setDiskCacheExecutor(GlideExecutor.newDiskCacheExecutor(GlideExecutor.UncaughtThrowableStrategy.IGNORE));
        builder.setSourceExecutor(GlideExecutor.newSourceExecutor(GlideExecutor.UncaughtThrowableStrategy.IGNORE));
    }

    @Override
    public void registerComponents(Context context, Glide glide, Registry registry) {
        super.registerComponents(context, glide, registry);
    }
}

and i see that module is running, but Glide doesn't seem to catching this exception.

And this is the static function found in the project for loading images into imageviews:

private static BitmapImageViewTarget loadCircleImageIntoView(RequestManager manager, final Context context, final ImageView imageView, String imageUrl, @DrawableRes int placeholder, boolean fitCenter, boolean centerCrop) {

        String cacheSignature = imageUrl+"-"+fitCenter+"-"+centerCrop;
        RequestBuilder<Bitmap> requestBuilder = Glide
                                                .with(context)
                                                .asBitmap();
        RequestOptions options = new RequestOptions()
                .circleCrop()
                .fitCenter()
                .centerInside()
                .signature(new ObjectKey(cacheSignature))
                .diskCacheStrategy(DiskCacheStrategy.ALL);

        if (fitCenter){
            options = options.fitCenter();
        }
        if (centerCrop){
            options = options.centerCrop();
        }
        requestBuilder = requestBuilder.apply(options);
        return requestBuilder
                .load(imageUrl)
                .into(new BitmapImageViewTarget(imageView){
            @Override
            protected void setResource(Bitmap resource) {
                super.setResource(resource);
                RoundedBitmapDrawable circularBitmapDrawable = RoundedBitmapDrawableFactory.create(context.getResources(), resource);
                circularBitmapDrawable.setCircular(true);
                imageView.setImageDrawable(circularBitmapDrawable);
            }
        });
}

So my questions are: 1- How can we "catch" this error? 2- Why does "GlideExecutor.UncaughtThrowableStrategy.IGNORE" doesn't work in this scenario 3- Is it really necessary to throw an error in this situation? (Don't get me wrong, i am just trting to understand how glide works)

Layout XML: -

Stack trace / LogCat:

Fatal Exception: java.lang.NullPointerException: Tried to decrement empty size, size: 108732, removed: [108732](ARGB_8888), this: SizeConfigStrategy{groupedMap=GroupedLinkedMap( {[4665600](ARGB_8888):3}, {[46656](ARGB_8888):4}, {[577600](ARGB_8888):0}, {[47524](ARGB_8888):1}, {[390640](ARGB_8888):1}, {[108732](ARGB_8888):0} ), sortedSizes=(null[{}], ARGB_8888[{46656=1, 47524=1, 390640=1, 4665600=1}])}
       at com.bumptech.glide.load.engine.bitmap_recycle.SizeConfigStrategy.decrementBitmapOfSize(SizeConfigStrategy.java:111)
       at com.bumptech.glide.load.engine.bitmap_recycle.SizeConfigStrategy.removeLast(SizeConfigStrategy.java:99)
       at com.bumptech.glide.load.engine.bitmap_recycle.LruBitmapPool.trimToSize(LruBitmapPool.java:223)
       at com.bumptech.glide.load.engine.bitmap_recycle.LruBitmapPool.clearMemory(LruBitmapPool.java:205)
       at com.bumptech.glide.load.engine.bitmap_recycle.LruBitmapPool.trimMemory(LruBitmapPool.java:215)
       at com.bumptech.glide.Glide.trimMemory(Glide.java:556)
       at com.bumptech.glide.Glide.onTrimMemory(Glide.java:752)
       at android.app.Application.onTrimMemory(Application.java:136)
       at android.app.ActivityThread.handleTrimMemory(ActivityThread.java:5229)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1765)
       at android.os.Handler.dispatchMessage(Handler.java:105)
       at android.os.Looper.loop(Looper.java:156)
       at android.app.ActivityThread.main(ActivityThread.java:6523)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:941)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:831)
sjudd commented 6 years ago

This looks like a bug of some sort, let me know if you find a way to reproduce it. I haven't seen this before.

To answer your questions, you can't catch the exception. It's thrown on the main thread so the uncaught throwable strategy is unused. Yes an exception is appropriate here.

sjudd commented 6 years ago

Also do you see this on all versions of Android? Or one in particular?

dreampowder commented 6 years ago

when i look at the device/os distribution:

screen shot 2017-12-20 at 15 52 45

sjudd commented 6 years ago

What are those specific OS versions?

dreampowder commented 6 years ago

@sjudd there are andorid major versions like 7.0, 7.1.1, etc..

LNeway commented 6 years ago

I have the same problem....the image is the OS versions statistic.

baiduhi_2018-2-2_15-14-48

Thanks...

dreampowder commented 6 years ago

I have checked my code and i think the main source of this problem is that somewhere in my code, my bitmap has already been recycled and i am trying to recycle it again from glide. When i check my carsh logs in fabric (which are 1600 of them in fabric sadly), source of the problem is:

private void decrementBitmapOfSize(Integer size, Bitmap removed) {
    Bitmap.Config config = removed.getConfig();
    NavigableMap<Integer, Integer> sizes = getSizesForConfig(config);
    Integer current = sizes.get(size);
    if (current == null) {
      throw new NullPointerException("Tried to decrement empty size"
          + ", size: " + size
          + ", removed: " + logBitmap(removed)
          + ", this: " + this);
    }

    if (current == 1) {
      sizes.remove(size);
    } else {
      sizes.put(size, current - 1);
    }
  }

In the first line glide calls for

Bitmap.Config config = removed.getConfig();

which return null in my code. Because Glide tries to decrement size "108732", which is available in "sizes" map

When we investigate the second line, which is:

NavigableMap<Integer, Integer> sizes = getSizesForConfig(config);

that calls for this:

private NavigableMap<Integer, Integer> getSizesForConfig(Bitmap.Config config) {
    NavigableMap<Integer, Integer> sizes = sortedSizes.get(config);
    if (sizes == null) {
      sizes = new TreeMap<>();
      sortedSizes.put(config, sizes);
    }
    return sizes;
  }

When we see the log, we see that the sizes map is like this:

sizes = [108732](ARGB_8888), this: SizeConfigStrategy{groupedMap=GroupedLinkedMap( {[4665600](ARGB_8888):3}, {[46656](ARGB_8888):4}, {[577600](ARGB_8888):0}, {[47524](ARGB_8888):1}, {[390640](ARGB_8888):1}, {[108732](ARGB_8888):0} )

and sortedSizesMap:

sortedSizes = (null[{}], ARGB_8888[{46656=1, 47524=1, 390640=1, 4665600=1}])

eveerything is available in sizes map except the size 108732. When we also see that we have a null[{}] entry in the sortedsizes map, which i think it has been added when getSizesForConfig call made with a null value.

So should we say that we should add a null check and precautions for this crash on "getSizesForConfig" when we call this function with a null value?

P.S: i have checked my code and couldn't find any place that calls for bitmap.recycle(), and also i've read that bitmap.getConfig() returns null when the image is a gif, which my app doesn't use it for sure.

sjudd commented 6 years ago

Thanks for the investigation!

I don't think the null config is the issue. sortedSizes contains a null key which is used to hold Bitmaps with null configs. If the Bitmap actually had a null config, this would be WAI.

It's more likely that the config becomes null after a call to recycle() while the image is still in the BitmapPool. It's not totally clear from the framework code, but it's listed as undefined: https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/graphics/java/android/graphics/Bitmap.java#1466.

If so, you're probably putting a Bitmap to the pool twice or retaining and recycling one that you shouldn't. There's some generic advice on this here: http://bumptech.github.io/glide/doc/resourcereuse.html#common-errors

Are you using any custom Transformations? Are you ever putting directly to the Bitmap pool? Or doing something like imageView.getImageDrawable() after loading an image into the ImageView with Glide?

dreampowder commented 6 years ago

@sjudd thanks for the quick reply. I've investigated further in the code and i've seen these calls in some of my activities, do you think that these might be the cause?


//Declared at the beggining of the Activity
private BitmapImageViewTarget mAvatarTarget;
private BitmapImageViewTarget mCompanyAvatarTarget;

@Override
    protected void onDestroy() {
        super.onDestroy();
        if (mAvatarTarget != null) {
            Glide.with(getApplicationContext()).clear(mAvatarTarget);
        }
        if (mCompanyAvatarTarget != null) {
            Glide.with(getApplicationContext()).clear(mCompanyAvatarTarget);
        }
    }

As for filters the app uses those filters for those targets:

mAvatarTarget transformations:

    private static BitmapImageViewTarget loadCircleImageIntoView(RequestManager manager, final Context context, final ImageView imageView, String imageUrl, @DrawableRes int placeholder, boolean fitCenter, boolean centerCrop) {

        String cacheSignature = imageUrl+"-"+fitCenter+"-"+centerCrop;
//        Timber.i("Square transforming following: %s", cacheSignature);

        RequestBuilder<Bitmap> requestBuilder = Glide
                                                .with(context)
                                                .asBitmap();
        RequestOptions options = new RequestOptions()
                .circleCrop()
                .fitCenter()
                .centerInside()
//                .transform(new CropSquareTransformation())
                .signature(new ObjectKey(cacheSignature))
                .diskCacheStrategy(DiskCacheStrategy.ALL);

        if (fitCenter){
            options = options.fitCenter();
        }
        if (centerCrop){
            options = options.centerCrop();
        }
        requestBuilder = requestBuilder.apply(options);
        return requestBuilder
                .load(imageUrl)
                .into(new BitmapImageViewTarget(imageView){
            @Override
            protected void setResource(Bitmap resource) {
                super.setResource(resource);
                RoundedBitmapDrawable circularBitmapDrawable = RoundedBitmapDrawableFactory.create(context.getResources(), resource);
                circularBitmapDrawable.setCircular(true);
                imageView.setImageDrawable(circularBitmapDrawable);
            }
        });
}

and mCompanyAvatarTarget:

    private static BitmapImageViewTarget loadRoundImageIntoView(RequestManager manager, final Context context, final ImageView imageView, String imageUrl, @DrawableRes int placeholder) {
//        Timber.i("Square transforming following2: %s", imageUrl);
        String signature = imageUrl+" - round-image";
        RequestOptions requestOptions = new RequestOptions()
//                .transform(new SquaringTransformation())
                .signature(new ObjectKey(signature))
                .diskCacheStrategy(DiskCacheStrategy.ALL)
                .placeholder(placeholder);

        return manager
                .asBitmap()

                .apply(requestOptions)
                .load(imageUrl)
                .into(new BitmapImageViewTarget(imageView){
                    @Override
                    protected void setResource(Bitmap resource) {
                        super.setResource(resource);
                        RoundedBitmapDrawable circularBitmapDrawable = RoundedBitmapDrawableFactory.create(context.getResources(), resource);
                        circularBitmapDrawable.setCornerRadius(5);
                        imageView.setImageDrawable(circularBitmapDrawable);
                    }
                });
}

Thanks again!