java.lang.IllegalArgumentException: Cannot add a callback twice

[bubbleguuum]

I've upgraded my app to recent Glide master (as of Sept 28th) and since got few of these crash reports (hopefully in a beta version of the app not widely deployed):

java.lang.IllegalArgumentException: Cannot add a callback twice
at com.bumptech.glide.request.target.ViewTarget$SizeDeterminer.getSize(SourceFile:176)
at com.bumptech.glide.request.target.ViewTarget.getSize(SourceFile:68)
at com.bumptech.glide.request.GenericRequest.begin(SourceFile:255)
at com.bumptech.glide.manager.RequestTracker.resumeRequests(SourceFile:70)
at com.bumptech.glide.RequestManager.resumeRequests(SourceFile:138)
at com.bumptech.glide.RequestManager.onStart(SourceFile:148)
at com.bumptech.glide.manager.ActivityFragmentLifecycle.onStart(SourceFile:48)
at com.bumptech.glide.manager.RequestManagerFragment.onStart(SourceFile:57)
at android.app.Fragment.performStart(Fragment.java)
at android.app.FragmentManagerImpl.moveToState(FragmentManager.java)
at android.app.FragmentManagerImpl.moveToState(FragmentManager.java)
at android.app.FragmentManagerImpl.moveToState(FragmentManager.java)
at android.app.FragmentManagerImpl.dispatchStart(FragmentManager.java)
at android.app.Activity.performStart(Activity.java)
at android.app.Activity.performRestart(Activity.java)
...

This is caused by commit f2285a8

This crash happen when the enclosing Activity (and its fragments) are restarted, for example when the user makes the app visible after having put it in the background.

Before this commit, the same code path was likely trigerred but it didn't crash because no IllegalArgumentException was thrown. Is it a consequence of a possibly bug in my own code calling Glide, but that just would have been ignored before this commit (and with possible side effects) ?

The app doesn't do any "recursive thumbnail calls" and nothing too extraordinary, just loading http thumbnails in a ListView using the ViewHolder pattern, in a Fragment.

[sjudd]

Thanks for bringing this up! After thinking about it again, this behavior is probably expected. Here's how it could happen

1. Load image A into View V, where View V doesn't have fixed dimensions (either match_parent, or a layout_weight).
2. Pause load for image A before View V undergoes layout.
3. Resume load for image A before View V undergoes layout.

Such a scenario is generally rare, but if you have a view with visibility GONE that may or may not later become visible, or otherwise have a View that does not undergo layout immediately, it becomes fairly likely.

This is a bug in Glide. For now I'll remove the assertion.

Ideally the Target interface would allow you to remove a callback as well as add one, but since that's not possible and changing the interface would be a breaking change, the ideal fix will have to wait for 4.0.

[bubbleguuum]

Thanks for the explanation. Most of my ImageViews have a fixed size, except notably in one layout where it is match_parent for both dimensions.

From the numbers of crash reports I got, I confirm it is in the "fairly likely" scenario for it to happen, thus not that rare.

Looking again at my Glide code, I noticed that I use Glide.with(Activity) instead of Glide.with(Fragment) [The ListView is managed in a Fragment], where the Activity passed is the activity owning the fragment. I do this because it leads to simplier code in my app where I can just use imageView.getContext() as the Activity passed to with(), instead of having to deal with passing the Fragment in many function calls for finally using Glide.with(Fragment).
It seems to work fine but is there any possible unintended side effect of passing the Activity instead of the Fragment to with() ?

[sjudd]

Got it, thanks for confirming, I'll push a fix shortly.

Glide uses the activity or fragment you pass in listen to the lifecycle of your app and respond accordingly. If you pass in the activity rather than the fragment, Glide will be listening to the activity's lifecycle events rather than the fragment's. If your Fragment's lifecycle isn't tied to your Activity's, some requests may not be paused or cancelled when they could be, which ranges from inefficient to dangerous depending on what you do in response to the requests completing.

Generally speaking passing in the Activity is probably safe, but using the Fragment is definitely safe and usually more efficient.