Glide: Canvas: trying to use a recycled bitmap

[jt-gilkeson]

I'm trying to load 3 random images at a time and then (custom) crossfade them in synchronously. Once the crossfade starts I wait 6 seconds and grab the next 3 random images (basically an auto-rotating gallery). 15 images total coming from the server, eventually cached as we cycle through.

Here's my code: https://gist.github.com/jt-gilkeson/c0ca583df16958172fb0

I'm getting the following error in Crashlytics from the field (I don't see it in-house, but it happens regularly in the wild). Not sure if something is wrong internally to glide or whether I'm doing something wrong in the way I'm using it.

Fatal Exception: java.lang.RuntimeException: Canvas: trying to use a recycled bitmap android.graphics.Bitmap@43693f28
       at android.graphics.Canvas.throwIfCannotDraw(Canvas.java:1084)
       at android.view.GLES20Canvas.drawBitmap(GLES20Canvas.java:844)
       at com.bumptech.glide.load.resource.bitmap.GlideBitmapDrawable.draw(GlideBitmapDrawable.java:101)
       at android.widget.ImageView.onDraw(ImageView.java:1058)
       at android.view.View.draw(View.java:15303)
       at android.view.View.getDisplayList(View.java:14197)
       ...
       at android.view.View.draw(View.java:15017)
       at android.view.ViewGroup.drawChild(ViewGroup.java:3298)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:3135)
       at android.view.View.draw(View.java:15306)
       at android.widget.FrameLayout.draw(FrameLayout.java:472)
       at com.android.internal.policy.impl.PhoneWindow$DecorView.draw(PhoneWindow.java:2568)
       at android.view.View.getDisplayList(View.java:14197)
       at android.view.View.getDisplayList(View.java:14239)
       at android.view.HardwareRenderer$GlRenderer.buildDisplayList(HardwareRenderer.java:1570)
       at android.view.HardwareRenderer$GlRenderer.draw(HardwareRenderer.java:1449)
       at android.view.ViewRootImpl.draw(ViewRootImpl.java:2705)
       at android.view.ViewRootImpl.performDraw(ViewRootImpl.java:2571)
       at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2143)
       at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1236)
       at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:6471)
       at android.view.Choreographer$CallbackRecord.run(Choreographer.java:803)
       at android.view.Choreographer.doCallbacks(Choreographer.java:603)
       at android.view.Choreographer.doFrame(Choreographer.java:573)
       at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:789)
       at android.os.Handler.handleCallback(Handler.java:733)
       at android.os.Handler.dispatchMessage(Handler.java:95)
       at android.os.Looper.loop(Looper.java:157)
       at android.app.ActivityThread.main(ActivityThread.java:5356)
       at java.lang.reflect.Method.invokeNative(Method.java)
       at java.lang.reflect.Method.invoke(Method.java:515)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1265)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1081)
       at dalvik.system.NativeStart.main(NativeStart.java)

[TWiStErRob]

You simply don't adhere to the Target interface contract and keep the drawable in use (mOldImageView) even after you've allowed Glide to recycle that bitmap (.into(new GlideDrawableImageViewTarget). Holding on to the drawable after the load has been cleared on that view is not safe. You're lying to Glide that you're loading to mNewImageView, but then setting and keeping the resource into mOldImageView.

Take a look at the discussion and tricks (last is best) for inspiration in https://github.com/bumptech/glide/issues/527#issuecomment-148840717

I think you should be able to apply the load(new).thumbnail(load(prev)) pattern 3 times where all 3 hits the memory cache and crossfade in sync. You can write a custom target which syncs up the glideAnimation.animate(resource, this) (what you overrode) with 2 other targets. That is last target will release the lock and trigger all 3 animates at the same time. This is just 3 image views and ownership of drawable is preserved.

[jt-gilkeson]

Thank a bunch for the quick response and explanation. The thumbnail approach and memory cache hit definitely sounds like a better approach.

As a (sub-optimal) quick fix until I have time to do a full rewrite, would cloning the drawable in onResourceReady before passing it down the line prevent this issue or are you saying that onResourceReady is too late to get the bitmap safely?

 @Override
 public void onResourceReady(GlideDrawable drawable, GlideAnimation anim)
 {
         // clone drawable here
     crossFadeWhenReady(postcard, clonedDrawable);
 }

[TWiStErRob]

onResourceReady would be too late to clone the drawable that was previously set. Of course if you clone the argument to the method it's fine, because you're just being handed that one and it won't be invalidated in the background because you'll be keeping the UI thread busy while cloning. I suggest an .asBitmap() to help you with the cloning so you get a Bitmap in onResourceReady, you don't need GIF support here anyway.

I hope you get the time to make it work, it's hardcore Glide usage, feel free to come back here and ask questions or open new issues when you get around to it. It's a great showcase for Glide's flexibility (simple things simple, advanced things possible).

[jt-gilkeson]

OK I just want to make sure I'm using this safely and that I'm not doing extra copying (i.e. I need to do the bitmap.copy to before setting the mNewImageView and then passing the bitmap to the synchronized crossfade) if you don't mind reviewing:

Glide.with(SplashboardFragment.this)
     .load(photo)
     .asBitmap()
     .override(mDownloadSize, mDownloadSize)
//   .centerCrop()  removed since setting the imageView will take care of this
     .into(new SimpleTarget<Bitmap>()
     {
         @Override
         public void onResourceReady(Bitmap bitmap, GlideAnimation<? super Bitmap> glideAnimation)
         {
             Drawable drawable = new BitmapDrawable(getResources(), bitmap.copy(bitmap.getConfig(), true));
             postcard.mNewImageView.setImageDrawable(drawable);
             crossFadeWhenReady(postcard, drawable);
         }
     });

PS: When I get time to rewrite I'll do an open source project that will take in a list of imageViews and a list of images and synchronize the crossfading using the thumbnail and custom target technique.

[TWiStErRob]

I guess you're right to go SimpleTarget though it's possible to still use BitmapImageViewTarget which would load a perfectly sized image and apply the transformation; and that being done and cached, the renderer has nothing to do except to show the pixels 1:1.

In this case (using SimpleTarget), since you copy the bitmap, you can immediately call Glide.clear(this) to release resources, otherwise those accumulated simple targets will only be cleared when fragment destroys... and givent it's an infinite loop, I guess it's better to do that earlier. It should be safe to call clear while delivering, internal targets (like downloadOnly/preload) do something like this too.

[TWiStErRob]

I'm gonna close this now, since your original problem was the resource leak, feel free to continue commenting though.

[jt-gilkeson]

OK, so you are saying to do the following if I want to let Glide take care of cleaning up its resources and do the rendering while still making the delayed crossfade safe, correct?:

Glide.with(SplashboardFragment.this)
        .load(photo)
        .asBitmap()
        .centerCrop()
        .override(mDownloadSize, mDownloadSize)
        .into(new BitmapImageViewTarget(postcard.mNewImageView)
        {
            @Override
            public void onResourceReady(Bitmap bitmap, GlideAnimation<? super Bitmap> glideAnimation)
            {
                Drawable drawable = new BitmapDrawable(getResources(), bitmap.copy(bitmap.getConfig(), true));
                crossFadeWhenReady(postcard, drawable);
            }
        });

[TWiStErRob]

I meant

.into(new SimpleTarget<Bitmap>() {
    @Override public void onResourceReady(Bitmap bitmap, GlideAnimation<? super Bitmap> glideAnimation) {
        Drawable drawable = new BitmapDrawable(getResources(), bitmap.copy(bitmap.getConfig(), true));
        Glide.clear(this); // added to release original bitmap
        postcard.mNewImageView.setImageDrawable(drawable);
        crossFadeWhenReady(postcard, drawable);
    }
});

or the better

Glide
    .with(SplashboardFragment.this)
    .load(photo)
    .asBitmap()
    .dontAnimate() // be a good sport and tell Glide it doesn't have to worry about anims
    //.centerCrop() // automatic from ImageView.scaleType xml, it's better to have it only in one place
    //.override(mDownloadSize, mDownloadSize) // let Glide figure out the size (e.g. accounts for different densities)
    .into(new BitmapImageViewTarget(postcard.mNewImageView) {
        @Override public void onResourceReady(Bitmap bitmap, GlideAnimation<? super Bitmap> glideAnimation) {
            bitmap = bitmap.copy(bitmap.getConfig(), true); // safe copy
            Glide.clear(this); // get control back over mNewImageView and release original
            super.onResourceReady(bitmap, null); // finish loading the copy as usual
            crossFadeWhenReady(postcard, getView().getDrawable()); // notify custom animation
        }
    })
;

(I wrote these on my phone, so obviously not tested)

[jt-gilkeson]

Thanks again for the clarification. The second way didn't seem to work right - for cached images the crossfade was not animating correctly, so I went back to SimpleTarget with the clear(this) added.

We use the .override to ensure images are cached full size so that if the user rotates the screen there is a cache hit instead of a reload.

[TWiStErRob]

Ah that makes sense, hitting the memory cache on rotation is nice, just make sure you account for densities. You can also use diskCacheStrategy(ALL), but that will still read the disk on rotation however orders magnitude faster than network.

[Duna]

Glide should check internally if there are references on the bitmap and not reuse them.
Please fix this issue ASAP

[hiteshsahu]

I am trying to check if bitmap is null or recycled like this

``` Glide.with(getApplicationContext())
.load(ContentUris.withAppendedId(AppContants.sArtworkUri, CenterRepository.getInstance()
. getAudioCollection().getSongAt(AppConfig.SONG_NUMBER).getAlbumId()))
.asBitmap()
.override(AppConfig.WIDGET_HEIGHT, AppConfig.WIDGET_HEIGHT)
.into(new SimpleTarget() {
@Override
public void onResourceReady(Bitmap resource, GlideAnimation glideAnimation ) {

                //check if recycled or null
                if (resource != null && !resource.isRecycled()) {
                    metadataEditor.putBitmap(RemoteControlClient.MetadataEditor.BITMAP_KEY_ARTWORK,
                            resource);
                }
            }
        });

``````

[TWiStErRob]

@hiteshsahu resource received in Target will never be null or recycled at this point. This issue is about when a Bitmap is drawn by an ImageView later after it has received it in mint condition in onResourceReady.

[mushtaq99]

@TWiStErRob @jt-gilkeson I am Facing the same Issue the code snipet is

try {
Glide.with(PosterActivity.this.getApplicationContext()).asBitmap().load(posterCo.getBack_image()).into(new SimpleTarget(720, 960) {
@Override
public void onResourceReady(@NonNull Bitmap resource, @Nullable Transition transition) {
PosterActivity.this.bitmapRatio(posterCo.getRatio(), "Background", resource, "created");
PosterActivity.this.dialogIs.dismiss();

                    }

//                    public void onResourceReady(Bitmap resource, GlideAnimation glideAnimation) {
//                        PosterActivity.this.bitmapRatio(posterCo.getRatio(), "Background", resource, "created");
//                        PosterActivity.this.dialogIs.dismiss();
//                    }
                });
            } catch (Exception e) {
                e.printStackTrace();
            }