square / picasso

A powerful image downloading and caching library for Android
https://square.github.io/picasso/
Apache License 2.0
18.72k stars 3.97k forks source link

Handler still used after canceling #445

Open arvola opened 10 years ago

arvola commented 10 years ago

I noticed that at times there are exceptions in the logs when navigating activities, even though the image downloads are being canceled prior.

I'm not very familiar with the inner workings of Picasso, but it would seem that BitmapHunter.hunt() does not check if it has been canceled while it's running, and so ends up trying to contact a dead handler at the end:

Handler (com.squareup.picasso.Stats$StatsHandler) {41b13d40} sending message to a Handler on a dead thread
    java.lang.RuntimeException: Handler (com.squareup.picasso.Stats$StatsHandler) {41b13d40} sending message to a Handler on a dead thread
            at android.os.MessageQueue.enqueueMessage(MessageQueue.java:196)
            at android.os.Handler.sendMessageAtTime(Handler.java:473)
            at android.os.Handler.sendMessageDelayed(Handler.java:446)
            at android.os.Handler.sendMessage(Handler.java:383)
            at com.squareup.picasso.Stats.processBitmap(Stats.java:104)
            at com.squareup.picasso.Stats.dispatchBitmapTransformed(Stats.java:59)
            at com.squareup.picasso.BitmapHunter.hunt(BitmapHunter.java:136)
            at com.squareup.picasso.BitmapHunter.run(BitmapHunter.java:83)
            at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:442)
            at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:305)
            at java.util.concurrent.FutureTask.run(FutureTask.java:137)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1076)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:569)
            at java.lang.Thread.run(Thread.java:856)

Since the exception is caught, it doesn't currently affect the app in any significant way, but I'd wager it'd still be better to avoid the exception in the first place.

Thoughts? I can make the necessary changes.

dnkoutso commented 10 years ago

Interesting... have you invoked shutdown() in your instance?

arvola commented 10 years ago

No, we are using the singleton, which I assume doesn't need shutdown() between activities.

dnkoutso commented 10 years ago

You are correct. You shouldn't need shutdown. Besides you cant invoke shutdown on the singleton instance it will crash.

I will need to investigate. Thanks!

dnkoutso commented 10 years ago

Will investigate for 2.4.

dnkoutso commented 9 years ago

@arvola does this still occur to you? I am moving to Picasso Next. A sample app demonstrating this would be great.

arvola commented 9 years ago

I'm not sure, but I will take a look and see if I can create a sample.