Sdwebimage: FREQUENT CRASH in [SDWebImageDownloaderOperation cancelInternal]

Created on 15 Feb 2017  路  29Comments  路  Source: SDWebImage/SDWebImage

New Issue Checklist

Issue Info

Info | Value |
-------------------------|-------------------------------------|
Platform Name |ios
Platform Version |9.0+
SDWebImage Version | 4.0.0
Integration Method | cocoapods
Xcode Version | Xcode 8.2.1
Repro rate |30%

Issue Description and Steps

Crashing very frequently after switching to latest release

Crashed: com.apple.main-thread
0  libobjc.A.dylib                0x240fba86 objc_msgSend + 5
1  Foundation                     0x25087a5b +[NSConcreteNotification newTempNotificationWithName:object:userInfo:] + 118
2  Foundation                     0x250879af -[NSNotificationCenter postNotificationName:object:userInfo:] + 46
3  Foundation                     0x2508c4fb -[NSNotificationCenter postNotificationName:object:] + 30
4  SDWebImage                     0x145e87d __47-[SDWebImageDownloaderOperation cancelInternal]_block_invoke (SDWebImageDownloaderOperation.m:207)
5  libdispatch.dylib              0x244c3cbf _dispatch_call_block_and_release + 10
6  libdispatch.dylib              0x244c3cab _dispatch_client_callout + 22
7  libdispatch.dylib              0x244c8559 _dispatch_main_queue_callback_4CF + 1532
8  CoreFoundation                 0x248f3755 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 8
9  CoreFoundation                 0x248f1c4f __CFRunLoopRun + 1590
10 CoreFoundation                 0x248401c9 CFRunLoopRunSpecific + 516
11 CoreFoundation                 0x2483ffbd CFRunLoopRunInMode + 108
12 GraphicsServices               0x25e5caf9 GSEventRunModal + 160
13 UIKit                          0x28f7a435 UIApplicationMain + 144
14 justunfollow                   0x65339 main (main.m:14)
15 libdispatch.dylib              0x244ec873 (Missing)
crash important

Most helpful comment

All joking aside: what's going on here? Do we need to file a PR to remove the self reference from the notification?

But more importantly: what is the current status for this project? 5 months with a crash like this out there seems like an awful long time

All 29 comments

@vinayak-parmar-cf can you reproduce this with our demo project?
If not, could you create a demo project of your own where it reproduces constantly?

@bpoplauschi we're having set of crashes with very similar symptoms. Cannot get it to reproduce, happens on 1% of user sessions. I'm attaching some of the crashed threads, maybe this would help you spot the issue.

screen shot 2017-02-21 at 21 07 30
screen shot 2017-02-21 at 21 07 12
screen shot 2017-02-21 at 21 07 02
screen shot 2017-02-21 at 21 06 53
screen shot 2017-02-21 at 21 06 38
screen shot 2017-02-21 at 21 06 12

@bpoplauschi Interestingly enough, in summary we have 1000 crashes like this. We're also getting crashes in random networking foundation places, and none of our code appears in stack traces. Something similar happens in some (but not all) SDWebImage related crashes. Do you think it is just an issue in CFNetwork / Foundation which is just invoked by SDWebImage?

@bpoplauschi We are getting same issue. we have 1000 crashes like this. We have a big number of user. So this is very critical for us.

I'm not familiar with the code base, but the self looks suspicious in here

https://github.com/rs/SDWebImage/blob/00bf467eb79bb97024eba7393169030a17b98018/SDWebImage/SDWebImageDownloaderOperation.m#L207

  • Any hints about in what conditions self could be nil?
  • Would a weakSelf/StongSelf dance in the block fix the issue? but any collateral problems if so?

We are getting same issue. (nearly 2K times). This is critical

I will try to come up with a fix this week. @billyto has a good point. That might be a cause.
Any other ideas/suggestions?

I would suggest changing self to nil, as object doesn't seem to be used anywhere.

[[NSNotificationCenter defaultCenter] postNotificationName:SDWebImageDownloadStopNotification object:nil];

We're seeing this issue in our pre production builds as well.
I'm noticing it in code where I am rapidly swapping out the data in a collection view view about 10 images. Seems like the pointer to self has gone bad.

We could set a flag in the dealloc method of the SDWebImageDownloaderOperation, and if that flag has been set, return from the cancelInternal method prior to posting that notification?

As mentioned above, we've been experiencing massive amounts of crashes like this, and others, that does not seem related to SDWebImage at all. After downgrading to SDWebImage 3.8.1 all of those seem to stop: we're seeing 13.5 thousands crash-free sessions, while with previous trend we'd have around 200 crashes for this number of sessions.

We're getting ~500 crashes since Feb 5. Can you please fix?

Same here: http://crashes.to/s/4da556ee579

Please fix!

+1 for not passing self to notification
Even https://github.com/rs/SDNetworkActivityIndicator/blob/master/SDNetworkActivityIndicator.m does not make use of it and it is the only observer for that notification as far as i can see.

Another crash with kind of the same cause: http://crashes.to/s/6137b904bf2

Has there been any progress on this issue?

I personally preferred to downgrade to 3.x

This is also crashing for me on a new app I am building. I am using sdWebImage to set the image on a cell. If I scroll too quickly it crashes

Too many crashes. But I am not able to replicate the issue. Crashes observed on the end user's device. More than 200 crashes in a month.

screen shot 2017-05-18 at 1 28 02 pm

I have the same issue. Please fix it.

SDWebImageDownloaderOperation.m line 207 in __47-[SDWebImageDownloaderOperation cancelInternal]_block_invoke (SDWebImageDownloaderOperation.m:207)

Unfortunately I'm not able to replicate the issue sistematically, but after upgrading to 4.0 I'm having a lot of crashes like this.

@bpoplauschi The object:self in the NSNotificationCenter postNotificationName call can definitely cause this. It creates a reference to the SDWebImageDownloaderOperation object that gets deallocated on another queue that is listening to the notification (in this case the main queue), which might be unexpected. I have seen similar crashes in the past, although it's typically the other way around (releasing the last reference on a background thread of an object that's doing things that need to happen on main queue). If you don't need the reference to the SDWebImageDownloaderOperation in the notification listener, you should definitely pass nil!
We are also seeing a significant amount of crashes due to this in 4.0.0 in a heavily-used application.

Here is a better stack trace from within Xcode

sdwebimagecrash

I have the same issue. Please fix it.
[SDWebImageDownloaderOperation cancelInternal]_block_invoke (SDWebImageDownloaderOperation.m:207)

Our baby-bug turned five months just a couple of days ago! 馃帀 馃巶

675008-a8787d50-7960-4c0c-9876-359a3f85e123

All joking aside: what's going on here? Do we need to file a PR to remove the self reference from the notification?

But more importantly: what is the current status for this project? 5 months with a crash like this out there seems like an awful long time

issue

+1

+1

+1

Sorry guys this took so long but I did not have the time to properly investigate.
Here are my findings:

  • the issue is probably caused by f4bdae6 - fixing that issued made this one appear. Basically, we used to make sure we call cancel on the same thread as start which means the exact thread that runs the operation. This behaviour stopped working at some point (probably we changed something in starting operations) so f4bdae6 tried to fix it, so cancelInternal was called again. Now it was not called on the same thread, therefore the threading issue.
  • #1940 tries to fix the issue by using weak references. Let's see how that goes. If there are still issues, we can just pass nil, but this fix is better because when the instance exists, we pass it.
  • I will close the other PR's and cut a release very soon
Was this page helpful?
0 / 5 - 0 ratings