Closed
Description
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)
Activity
TWiStErRob commentedon 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 passgetApplicationContext
instead ofthis
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 useOuterActivity.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 checkactivity.isFinishing/isDestroyed
inonPostExecute
or should be made really static and usegetApplicationContext
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 commentedon Sep 16, 2014
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 commentedon Sep 16, 2014
After you call
task.cancel(true)
fromonDestroy
do you check forisCancelled()
in the async task'sonPostExecute
? (just a hunch)GargManish commentedon 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 commentedon 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()
returnstrue
. 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. IfisDestroyed()
isfalse
in your async task buttrue
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 commentedon Sep 29, 2014
Please re-open if you've tried the above and are still having issues.
Cliffus commentedon 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 commentedon Oct 22, 2015
Please open a new issue, this is rather old, the current version is 3.6.1.
Cliffus commentedon 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 commentedon 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 commentedon 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 commentedon Oct 22, 2015
(the workaround I mentioned: check if the Activity is finishing, if so, stop loading the image)
TWiStErRob commentedon 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.
13 remaining items