Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting a crash in Glide 3.3 library #138

Closed
GargManish opened this issue Sep 16, 2014 · 18 comments
Closed

Getting a crash in Glide 3.3 library #138

GargManish opened this issue Sep 16, 2014 · 18 comments
Labels

Comments

@GargManish
Copy link

@GargManish GargManish commented Sep 16, 2014

Hi All,

I have seen one random crash in my application which was due to Glide image loading. Please find below the logs.

There is one more issue which I am facing. Sometime Glide is displaying random images. It is displaying those images which had been downloaded in other activity.

14-Sep-2014 08:41:31 PM java.lang.IllegalArgumentException: You cannot start a load for a destroyed activity
    at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:63)
    at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:29)
    at com.bumptech.glide.Glide.with(Glide.java:537)
    at com.miamiheat.common.MHImageDownloadWrapper.loadImage(MHImageDownloadWrapper.java:12)
    at com.miamiheat.ui.module.MHWallpaperModule.setWallpaperViewData(MHWallpaperModule.java:234)
    at com.miamiheat.ui.module.MHWallpaperModule.taskManagerResponseCallback(MHWallpaperModule.java:257)
    at com.miamiheat.service.taskmanager.MHWallpaperTaskManager.asyncResultCallback(MHWallpaperTaskManager.java:133)
    at com.miamiheat.service.framework.MHAsyncServiceTask.onPostExecute(MHAsyncServiceTask.java:191)
    at android.os.AsyncTask.finish(AsyncTask.java:631)
    at android.os.AsyncTask.access$600(AsyncTask.java:177)
    at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:644)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:176)
    at android.app.ActivityThread.main(ActivityThread.java:5419)
    at java.lang.reflect.Method.invokeNative(Native Method)
    at java.lang.reflect.Method.invoke(Method.java:525)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1046)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:862)
    at dalvik.system.NativeStart.main(Native Method)
@TWiStErRob

This comment has been minimized.

Copy link
Collaborator

@TWiStErRob TWiStErRob commented Sep 16, 2014

Please include your Glide.with line and describe the source of the context. Here's are my guesses without the above info:

What I can see from the stacktrace is that you're using an async task to load something (list of wallpapers?) then in onPostExecute you notify your wallpaper module which displays the wallpaper through Glide.

Now this can mean that you have an activity where the user presses a button, you fire an async task and then finish() in which case you just need to pass getApplicationContext instead of this when creating the callback/asynctask. If you're using a non-static inner class make it static and the context passing explicit and don't use OuterActivity.this because this leaks the activity (the exception is clear, your activity is dead, it has no UI, the lifecycle ended).

In general when you start an asynctask it should be cancelled in onPause/onStop or at least check activity.isFinishing/isDestroyed in onPostExecute or should be made really static and use getApplicationContext in which case it obviously shouldn't update the UI.

Hope this helps. If none of the above applies to you and your activity didn't leak then it can be a bug in Glide.

Note: you can wrap stacktrace/code inside ``` on both sides to make it formatted.

@GargManish

This comment has been minimized.

Copy link
Author

@GargManish GargManish commented Sep 16, 2014

Glide.with(context).load(imageUrl).animate(R.anim.fade_in).into(imageView);

I am passing activity context to it.

I am starting async task from the onCreate and stopping in the OnDestroy, I have also cleared my listview in the onDestroy. It fetches data and send it to the activity. Activity is populating data in the listView. Wallpapermodule is one of the list item and is inherited from the LinearLayout.

@TWiStErRob

This comment has been minimized.

Copy link
Collaborator

@TWiStErRob TWiStErRob commented Sep 16, 2014

After you call task.cancel(true) from onDestroy do you check for isCancelled() in the async task's onPostExecute? (just a hunch)

@GargManish

This comment has been minimized.

Copy link
Author

@GargManish GargManish commented Sep 16, 2014

Yes I am doing that. However rendering elements in the listview is not getting done by the asyncTask but by the adapter.
Async Task had already finished before rendering.

Just want to update one thing, I was using Glide.with(Context) and was passing Activity context in it. Is it right way? Now I have started to use Glide.with(Activity).

@sjudd

This comment has been minimized.

Copy link
Member

@sjudd sjudd commented Sep 16, 2014

As long as the context was the Activity, there isn't any difference between the two. However, If the context was the application context, there is a difference as @TWiStErRob mentioned above. In general If you're loading images in an Activity, you should pass in the activity, if you're loading images in a Fragment you should pass in the Fragment, and if you're loading images for some reason other than displaying or using them in a particular view, you should pass in the application context.

All that being said, I think there's probably a bug in your cancellation code, I'd guess @TWiStErRob is on the right track here. The framework method we're using to throw this exception has pretty clear documentation that it only returns true after the onDestroy call: http://developer.android.com/reference/android/app/Activity.html#isDestroyed().

Two things to try.
When your async task finishes and you call notifyDataSetChanged() on your list, check to see if isDestroyed() returns true. If it does, you didn't manage to cancel your task for some reason.

Similarly you can try passing your activity down to wherever you call Glide.with(...).load(...) and check isDestroyed there too. If isDestroyed() is false in your async task but true right before you start your load, you may have some other race.

Let me know what you find and thanks for following up on this.

@sjudd

This comment has been minimized.

Copy link
Member

@sjudd sjudd commented Sep 29, 2014

Please re-open if you've tried the above and are still having issues.

@sjudd sjudd closed this Sep 29, 2014
@TWiStErRob TWiStErRob added the question label Oct 17, 2015
@Cliffus

This comment has been minimized.

Copy link

@Cliffus Cliffus commented Oct 22, 2015

is there a reason why Glide doesn't always use the Application Context? The reason why people use libraries such as Glide is that they don't have to worry about the little details which can cause your application to crash.

Thanks for your feedback!

@TWiStErRob

This comment has been minimized.

Copy link
Collaborator

@TWiStErRob TWiStErRob commented Oct 22, 2015

Please open a new issue, this is rather old, the current version is 3.6.1.

@Cliffus

This comment has been minimized.

Copy link

@Cliffus Cliffus commented Oct 22, 2015

correct, but I'm having the exact same issue (which is apparently not an issue, but a feature ;-), should I really open a new ticket? I'm only suggesting an improvement.

@TWiStErRob

This comment has been minimized.

Copy link
Collaborator

@TWiStErRob TWiStErRob commented Oct 22, 2015

It won't be an improvement because it would be against the resource sensitive nature of Android/Glide. Glide uses the object given in Glide.with to attach a fragment to, which obviously cannot be done for an app context. This fragment cancels/pauses any load to prevent unnecessary background work if the result would not be used anyway. See for example http://stackoverflow.com/a/32887693/253468.

That little detail you mentioned can mean battery drain or memory leak in your app, which, thanks to Glide, is now surfaced. Why do you think that starting a load after the activity (where the image would be potentially displayed) has been destroyed is good idea?

@Cliffus

This comment has been minimized.

Copy link

@Cliffus Cliffus commented Oct 22, 2015

ok, that totally makes sense, but do you also get that crashing the app isn't a good solution too? I currently worked around this issue, but it isn't pretty.

What's the 'official' way of handling these situations, because our app sometimes crashes because of the IllegalArgumentException, even when we think we handled all the edge cases.

@Cliffus

This comment has been minimized.

Copy link

@Cliffus Cliffus commented Oct 22, 2015

(the workaround I mentioned: check if the Activity is finishing, if so, stop loading the image)

@TWiStErRob

This comment has been minimized.

Copy link
Collaborator

@TWiStErRob TWiStErRob commented Oct 22, 2015

Is isFinishing true when the activity is fully destroyed? (I.e.isDestroyed is true)
Can you share some code? It would be nice to fully see the case when this happens.

@Cliffus

This comment has been minimized.

Copy link

@Cliffus Cliffus commented Oct 22, 2015

well, we are currently in our last sprint to release the application. When I've collected some more crash data, I'll try to share it with you.

@cristiano939

This comment has been minimized.

Copy link

@cristiano939 cristiano939 commented Dec 4, 2015

I'm having the exact same issue, right now,
but the app isn't destroying the activity, all of a sudden it happens.
It could be caused by a recycleview or something like that?

@TWiStErRob

This comment has been minimized.

Copy link
Collaborator

@TWiStErRob TWiStErRob commented Dec 4, 2015

Please open a new issue with more information on your case, just write #138 to reference this issue. For example what are you doing as user just before that "all of a sudden" and what are you doing as a dev (code) just before that "all of a sudden". And of course the usual issue template

@shivangbtech

This comment has been minimized.

Copy link

@shivangbtech shivangbtech commented Oct 18, 2016

Getting Same Problem.

javierarboleda added a commit to VisualTiles/VisualTilesTogether that referenced this issue Nov 27, 2016
- Fixed crash due to error "You cannot start a load for a destroyed activity"
   See this link for more info on issue: bumptech/glide#138

   - fix was in TileListRecyclerViewAdapter "Glide.with(mContext)" changed to "Glide.with(mContext.getApplicationContext())"
@Mr-Wu

This comment has been minimized.

Copy link

@Mr-Wu Mr-Wu commented Apr 17, 2017

good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.