Afnetworking: Crash on AFImageWithDataAtScale when loading image

[matthewvelie]

I'm using "UIImageView+AFNetworking.h" to remotely load in some images into a UITableViewCell. Randomly (about every 1/3 launches) the app will crash. It'll either be a SIGABRT or an EXC_BAD_ACCESS in AFURLResponseSerialization in the method below.

static UIImage * AFImageWithDataAtScale(NSData *data, CGFloat scale) {
    UIImage *image = [[UIImage alloc] initWithData:data];

    return [[UIImage alloc] initWithCGImage:[image CGImage] scale:scale orientation:image.imageOrientation];
}

The crash usually outputs the error: malloc: *** error for object 0x7fe662909050: double free *** set a breakpoint in malloc_error_break to debug

I've had this happen in my project randomly and been able to reproduce it in a small sample project, which I can supply if needed.

In the sample project I'm just doing this inside tableview:CellForRowAtIndexPath:
[cell.cellImageView setImageWithURL:[NSURL URLWithString:@"http://www.nmnathletics.com//pics/160/107.gif"]];

I'm unsure if this is related to: https://github.com/AFNetworking/AFNetworking/issues/2334, I noticed someone on that mentioning that it happened with images for them, but for everyone else it seemed to be a different issue.

So far I've only been able to make this happen in the simulator, on a retina device.

[mattt]

I've had this happen in my project randomly and been able to reproduce it in a small sample project, which I can supply if needed.

Yes, please share that if you can.

[matthewvelie]

Here's a quick sample project. https://www.dropbox.com/s/zb7n7fcw20k4mr1/testApp.zip?dl=0
It worked the first two times I ran it in the simulator, third time it ran, crashed soon as it tried to display the images.

[tewha]

Crashed for me, too, but only once. I wish I could get it to happen again…

[tewha]

It took maybe 50 runs before it crashed again. Here's what I found: The data is correct, even on this failed run, so it isn't a question of the server returning something invalid. The crash is on the first line of AFImageWithDataAtScale, [[UIImage alloc] initWithData:data]: EXC_BAD_ACCESS EXC_I386_GPFLT. I'm going to bang out a quick app to see if the problem is reproducible without AFNetworking.

[tewha]

Can't reproduce it without AFNetworking, but there's a lot of threading going on here. Is it entirely safe to call this routine from so many threads simultaneously? Sorry I couldn't be of more help. :)

[tewha]

I've identified a second type of crash; same exception and code, but this one occurs in one of the com.apple.root.default-qos threads while deallocating the image. (The first one occurs in any of the com.alamofire.networking.http-request.procesesing threads.)

[matthewvelie]

I have the same issue with the crash occurring very infrequently. It took awhile to track down in my main project. Occasionally it would crash often and other times its very rare like you saw. I tried a different ways, such as having the tableviewcell set the image itself, along with using the block method call, and they all had the same issue; I never did get the apple qos crash though.

The crash I would get is the EXEC_BAD_ACCESS (code=EXC_I386_GPFLT), occasionally it would show up as malloc: *** error for object 0x7fe662909050: double free *** set a breakpoint in malloc_error_break to debug. Always occurred on the same line as you found.

[tewha]

Yeah. I'd say (hesitantly, and without much conviction) this is a bug in iOS. There's not a lot of complexity here. However, it may be that AFNetworking is unreasonably stressing something to cause it. There's a lot of threads here, and potentially a very high tide mark. @matthewvelie, maybe try adding a synchronized block on some global around the body of AFInflatedImageFromResponseWithDataAtScale? Might turn performance to crap, but I'm curious if stopping so much simultaneous image processing would solve the problem. If it helps, something more reasonable might be possible.

[bagusflyer]

Any solution or workaround?

[scottasoutherland]

As far as I can tell, this is only happening on iOS 8. Since it looks like a threading crash it's hard to know for sure, but at the very least I have a much harder time reproducing it in iOS 7. This is currently happening to me almost 1 out of every 3 runs on iOS 8. I've yet to repro it on iOS 7. This is super easy to repro on the sample app submitted by @matthewvelie , but again haven't gotten it in iOS 7.

[tewha]

Unfortunately, I can't recreate it at all anymore.

[paperview]

I am also having this problem and it only happens every once in a while.

[paulsfds]

I am also seeing this issue. Same use case, trying to load images for a UITableViewCell. It only crashes every once in a while; seems like a threading issue.

[ishuo]

I have this crash too. I observed that my crash happens within AFImageResponseSerializer, when two of them are executing concurrently. My assumption is that AFImageResponseSerializer is not thread-safe. Thus the crash would happen, if multiple requests with this serializer finish at once.

So my solution was to execute all such RequestOperations sequentially. I created a serial NSOperationQueue and add these RequestOperations into it instead of directly starting them. In my brief testing, this seems to have fixed the crash bug.

Maybe give it a try on your code and see if this helps?

See http://stackoverflow.com/questions/21978951/does-afhttprequestoperationmanager-run-requests-on-the-operationqueue for more info about NSOperationQueue.

[nartex]

Same issue for me, happens once in a while too, using setImageWithURL in an UITableViewCell.

[ignazioc]

same issue here, imageview on collectionviewcell

[cmds4410]

+1, haven't figured out how to reliably reproduce yet.

[paulosotu]

Hi Guys,

Been with the exact problem using Haneke and found the problem. Looking at the code here it seems to be the exact same thing. So it seems UIImage imageWithData is not thread safe. Meaning if we have several threads accessing the static builder it will crash. I've made a pull request that serialises the call, https://github.com/AFNetworking/AFNetworking/pull/2815.

I also made a simple example that will always replicate this behaviour, in case you guys are curious (swift code):

let data = NSData(contentsOfURL: NSURL(string: "https://encrypted-tbn3.gstatic.com/images?q=tbn:ANd9GcTf4P0V2TrszykwPguyeulcSt4vwQPcvvWKKq_RePhUUVFoHevG9lw8GVcw")!)

var test: Array<UIImage> = []
let lock = NSLock()
for i in 0...10000 {
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), { () -> Void in
    NSLog("Processing \(i)")
    let a = UIImage(data:data!)
    NSLog("Processed \(i)")
    lock.lock()
    test.append(a!)
    test.removeLast()
    lock.unlock()
    })
}

[kcharwood]

This should be fixed with #2860!

Thanks all!

:beers:

[steipete]

Has anyone file a radar on this? I'd love to dupe.

[kcharwood]

Not to my knowledge.

I also didn't consider the impact of someone else calling imageWithData correctly from the main thread within their own application or from another framework, leaving the possibility of this still being an issue. I'm assuming we'll have to force this through the main thread... :(

[steipete]

I can't name the source, but it's being taken care of. (or at least a radar has been filed by someone inside Apple)

Side note: I've looked at the UIKit disassembly and that API calls up traitCollection and a few other things and really is everything but thread safe; more so it really should only be called on the main thread ideally. We could swizzle and add the lock directly _into_ the implementation, that would at least remove the fact that any non-AFNetworking consumer would still call this without the lock and might trigger the race condition; but it's also very much not pretty.

Using CGImageSource directly is more work but might be the better solution.

[kcharwood]

@matthewvelie Do you still have that sample project you posted above on Dropbox? Seems like there are other people this week who would like to take a look at it.

[matthewvelie]

@kcharwood Sorry, I've restored the zip file, the link should work again.

[hpique]

@steipete Mind posting here if you ever find out the bug is fixed?

[scspijker]

Has this radar been solved / is UIImage imageWithData already thread safe?

[steipete]

Did anyone actually file a radar?

I do not see any locking in the UIKit assembly... my hopes are not up.

[steipete]

I wrote a small sample. iOS 9.3:

Wasn't able to reproduce the crash on iOS 10. Can somebody else run this a few times to test it?

http://cl.ly/3f250F3X3m2Y

[steipete]

The crash only happens rarely on iOS 9.3, so this needs more testing - help welcome!

[steipete]

Since it looks like there's a race in over-releasing UITraitCollection (maybe a racy cache that over-releases it) I set a conditional breakpoint, and we still have the behaviour on iOS 10:

Thinking more about this - is UIImage safe to _create_ on any thread? We do not get such a guarantee:

https://developer.apple.com/library/ios/documentation/UIKit/Reference/UIImage_Class/index.html

Because image objects are immutable, you cannot change their properties after creation. Most image properties are set automatically using metadata in the accompanying image file or image data. The immutable nature of image objects also means that they are safe to use from any thread.

(note the _use_ - not create, although I see that it is a bit vague)

I do remember reading something about this in the iOS 4 release notes. Apple's docs don't go back so far anymore - they stop at iOS 5:

https://developer.apple.com/library/ios/releasenotes/General/WhatsNewIniOS/Introduction/Introduction.html#//apple_ref/doc/uid/TP40008246

However, wayback machine helps!

https://web.archive.org/web/20100812204946/http://developer.apple.com/iphone/library/releasenotes/General/WhatsNewIniPhoneOS/Articles/iPhoneOS4.html

These are the UIKit Framework Enhancements we got:

Drawing to a graphics context in UIKit is now thread-safe. Specifically: The routines used to access and manipulate the graphics context can now correctly handle contexts residing on different threads; String and image drawing is now thread-safe.; Using color and font objects in multiple threads is now safe to do.

This only talks about drawing - not creating.

I do think that this bug is fairly new at at some point imageWithData: was meant to be thread safe, since the underlying CoreGraphics calls are thread safe. But then, retina came, and deadlines, and the code got more complex, and suddenly images had a scale and an implicit trait environment (so they can swap out when the env changes...) and somewhere needed to create such a UITraitCollection object...

Let's set a breakpoint on who creates this object:

(lldb) bt
* thread #2: tid = 0xcbd57, 0x0000000102264afe UIKit`-[UITraitCollection _initWithBuiltinTraitStorage:clientDefinedTraits:], queue = 'com.apple.root.default-qos', stop reason = breakpoint 2.1
  * frame #0: 0x0000000102264afe UIKit`-[UITraitCollection _initWithBuiltinTraitStorage:clientDefinedTraits:]
    frame #1: 0x0000000102267342 UIKit`+[UITraitCollection traitCollectionWithDisplayScale:] + 78
    frame #2: 0x00000001017433c3 UIKit`-[UIImage(UIImagePrivate) _initWithData:preserveScale:cache:] + 211
    frame #3: 0x000000010173e4e9 UIKit`+[UIImage imageWithData:] + 69
    frame #4: 0x00000001006e60f5 UIImageWithDataMultithreaded`__29-[ViewController viewDidLoad]_block_invoke((null)=0x00007fff5f517a28, idx=11) + 69 at ViewController.m:25

So this object is definitely created on a background thread (also... wasteful, why is this not cached?).

I also added my main thread guard that checks mostly for view access and that one didn't trigger, so there are no views involved.

https://gist.github.com/steipete/5664345

So, doesn't seem to crash anymore - but it certainly did in iOS 9.3 - see my testcase. Let's enable zombie objects to see what's up here.

The funny stuff is that the code is so racy that the system sometimes even races on creating zombie classes:

ThreadSanitizer debugger support is active.
objc[47357]: Class _NSZombie_UIImage is implemented in both ?? and ??. One of the two will be used. Which one is undefined.
objc[47357]: Class _NSZombie_UIImage is implemented in both ?? and ??. One of the two will be used. Which one is undefined.

(This is no big deal, since it's just for debugging, but certainly funny)

Annoyingly enough, as soon as we enable the thread sanitizer and/or zombies, it does not crash anymore. Disabling that and playing with it some more showed something interesting in the stack trace:

UITraitCollectionCacheForBuiltinStorage. The fun thing is - this method lazily initializes a dictionary, but it uses dispatch_once and that part is thread safe... but then it calls

void CFDictionarySetValue ( CFMutableDictionaryRef theDict, const void *key, const void *value );

Where value is the new trait collection that is created and then set. Now we race in the dictionary setter and an object gets over-released and we crash in object_dispose.

In iOS 9.3, traitCollectionWithDisplayScale: simply calls through to traitCollectionWithDisplayScale. Now how do things look in iOS 10? The cache is gone... things are more wasteful BUT we do no longer race since it seems the UITraitCollection objects do not share common data or access the main thread (and if they are just dumb model objects - why shouldn't they!)

So, long story story, I do believe that this has been fixed (since clearly you don't delete a cache for fun - especially not when there's so much to do around UIKit.). However, since we were all a bit lazy here there's no radar where I could cross-check that, and Apple's documentation is inherently vague.

If someone from Apple is reading this - a clear stand on UIImage creation thread safety would be great. I do believe that it's highly beneficial to keep this thread safe and it seems it is the case, but we're a bit in the dark here and if this suddenly regresses again I'd love to know if this is at least a bug :)

I filed rdar://problem/26954460 to enhance the documentation.

I wrote a workaround that patches UIKit at the correct place to fix the locking application-wide. The current fix for AFNetworking does not fetch other system and the lock can't be shared - so it's unsafe as soon as somebody else calls imageWithData: on a background thread. I'm not sure if it's a good idea and the crash is probably quite rare, but depending on the scale of your app it might still save thousands of crashers each day.

https://github.com/PSPDFKit-labs/PSTModernizer

[haifengkao]

Will it crash on iOS 9.3 if we use [[UIImage alloc] initWithData:data] instead?

[steipete]

Yes. https://pspdfkit.com/blog/2016/investigating-thread-saftey-of-UIImage/