NPE: 4.3.0 and GIFs lifecycle

[moxie0]

  Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'int android.graphics.Bitmap.getWidth()' on a null object reference
      at com.bumptech.glide.load.resource.gif.GifFrameLoader.getWidth(GifFrameLoader.java:127)
      at com.bumptech.glide.load.resource.gif.GifDrawable.getIntrinsicWidth(GifDrawable.java:215)
      at android.widget.ImageView.invalidateDrawable(ImageView.java:232)
      at android.graphics.drawable.Drawable.invalidateSelf(Drawable.java:385)
      at android.graphics.drawable.Drawable.setVisible(Drawable.java:764)
      at com.bumptech.glide.load.resource.gif.GifDrawable.setVisible(GifDrawable.java:210)
      at android.widget.ImageView.onDetachedFromWindow(ImageView.java:1485)
      at android.view.View.dispatchDetachedFromWindow(View.java:14555)
      at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3063)
      at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3063)
      at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3063)
      at android.view.ViewGroup.removeViewInternal(ViewGroup.java:4603)
      at android.view.ViewGroup.removeViewInternal(ViewGroup.java:4576)
      at android.view.ViewGroup.removeView(ViewGroup.java:4507)
      at android.support.v4.view.ViewPager.removeView(ViewPager.java:1499)
      at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1520)
      at android.support.v4.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManager.java:1740)
      at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1809)
      at android.support.v4.app.FragmentManagerImpl.dispatchStateChange(FragmentManager.java:3217)
      at android.support.v4.app.FragmentManagerImpl.dispatchDestroy(FragmentManager.java:3208)
      at android.support.v4.app.FragmentController.dispatchDestroy(FragmentController.java:262)
      at android.support.v4.app.FragmentActivity.onDestroy(FragmentActivity.java:350)
      at android.support.v7.app.AppCompatActivity.onDestroy(AppCompatActivity.java:209)
      at org.thoughtcrime.securesms.PassphraseRequiredActionBarActivity.onDestroy(PassphraseRequiredActionBarActivity.java:83)
      at android.app.Activity.performDestroy(Activity.java:6407)
      at android.app.Instrumentation.callActivityOnDestroy(Instrumentation.java:1142)
      at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:3818)

I'm using Glide 4.3.0 to load GIFs into a RecyclerView. Some users are reporting that if they scroll quickly and then close the activity, they get a crash.

Glide Version: 4.3.0

[sjudd]

Let me know if you figure out repro steps.

[sjudd]

Also can you provide your Glide load line?

[moxie0]

This is the load line:

      glideRequests.load(new GiphyPaddedUrl(image.getGifUrl(), image.getGifSize()))
                   .thumbnail(thumbnailRequest)
                   .placeholder(new ColorDrawable(Util.getRandomElement(MaterialColor.values()).toConversationColor(context)))
                   .diskCacheStrategy(DiskCacheStrategy.ALL)
                   .listener(holder)
                   .into(holder.thumbnail);

The actual line if you're interested:
https://github.com/WhisperSystems/Signal-Android/blob/3ed82c1726b3e0f9a1ededc0c7e4ae837f6ed4f5/src/org/thoughtcrime/securesms/giph/ui/GiphyAdapter.java#L162

glideRequests is passed in to the Adapter's constructor from the Fragment, and is essentially Glide.with(Fragment.this). That's the other change that I made throughout the app along with updating to 4.3.0. I was previously using the equivalent of Glide.with(ApplicationContext.this) in some places, but I saw something that made me think (perhaps erroneously?) I should switch to what I'm doing now.

I can reproduce this in my app pretty reliably, it just requires flinging the RecyclerView and then hitting back. Let me know if you don't have enough information or don't want to mess with the whole Signal app, and I'll try breaking it out into a sample app.

[tinder-ksaurabh]

Able to reproduce this crash on Glide version4.1.1 consistently.

[sjudd]

I tried to reproduce this in Signal and am not able to. A video might help clarify the repro steps, or if it's easy to reproduce in a sample app that would be great too. Glide's giphy sample app is pretty close to what you're doing, so it may make a reasonable starting point.

Does the crash occur when you back out of selecting a GIF? Or back out of the screen where you'd send a GIF after selecting one? The steps I've tried are:

1. Open Signal
2. Tap the pencil icon
3. Enter a phone number
4. Tap the overflow menu
5. Tap "add attachment"
6. Tap GIF
7. Scroll up and down in GIFs and hit back
   Or:
8. Tap Stickers, scroll up and down and hit back
   Or:
9. Tap stickers
10. Tap on a particular sticker to select it
11. Wait to be taken back to the conversation view with the selected GIF playing
12. Tap back to exit the conversation.

None of those three sets of steps seems to produce a crash, at least not on an x86 API 26 emulator.

[sjudd]

@tinder-ksaurabh If you're able to reproduce this, can you share a sample app that does so? Or provide more details about your load lines and what you're doing to reproduce this?

[sjudd]

I wasn't able to reproduce this on an emulator, but I am on a real device. Probably due to the timing required to get this to occur.

[sjudd]

I believe I have a fix for this, still working on making sure it's tested correctly.

A workaround for this for now is to remove any use of .thumbnail(). There's a bug in the logic that coordinates the full and thumb requests that can cause Glide to fail to remove the GifDrawable (or any Drawable) from a view in onStop.