2

I have this code to store all contacts image in dictionary. But, in some cases when it is interrupted, the image for contacts just disappear.

dispatch_async(dispatch_get_main_queue(), ^{            

        if (ABPersonHasImageData(_personObj)) {

            // UIImage *image = [UIImage imageWithData:(__bridge NSData*) ABPersonCopyImageDataWithFormat(_personObj, kABPersonImageFormatThumbnail)];
            NSData *data = (__bridge NSData *) ABPersonCopyImageDataWithFormat(_personObj, kABPersonImageFormatThumbnail);
            UIImage *image = [UIImage imageWithData:data scale:1];
            int recordId = ABRecordGetRecordID(_personObj);
            [contactImagesDi setValue:image forKey:[NSNumber numberWithInt:recordId]];

        }

});
Omarj
  • 1,151
  • 2
  • 16
  • 43
  • 2
    Two things that are not the solution to your problem, but that would improve your code: (1) You can leave out `UIImage * image=[[UIImage alloc]init];` and instead directly write `UIImage *image = [UIImage imageWithData...`. (2) It's better to use an NSNumber as the dictionary key, i.e use `[NSNumber numberWithInt:recordId]` instead of `[NSString stringWithFormat...]`. – mrueg Mar 17 '13 at 09:52
  • 2
    Why would this be interrupted, as you state in the question? Additionally, why do you have to use a try-catch block for this? – Jack Humphries Mar 19 '13 at 21:32

1 Answers1

6

A single ABPerson is not threadsafe. You cannot pass an ABPerson to a background queue using dispatch_async(). If you want to do background processing, you must generate a new ABAddressBook on each thread and use ABPerson records fetched from that address book on that thread.

If you need to logically pass an ABPerson between threads, you need to fetch its ID with ABRecordGetRecordID(). You can pass that and reconstruct a new ABPerson record on the other thread (with its own address book) using ABAddressBookGetPersonWithRecordID().

@try/@catch is very rare in ObjC, and you should have a very good reason for doing it. Under ARC, you will generally leak memory. Exceptions are meant to indicate that the program is in trouble and should crash shortly.

You are leaking data. You should be using CFBridgingRelease() here, not __bridge. You need to balance the Copy.

Your modification of contactImagesDi is very dangerous, assuming this is a dictionary. NSMutableDictionary is not threadsafe. If it is an object that you are using KVC on, then it could be thread-safe, but only if you have taken some pains to ensure that. Typically the better solution is to use dispatch_async to put that kind of update back onto the main thread.

Rob Napier
  • 286,113
  • 34
  • 456
  • 610
  • First at all thx. for fetch using ID idea it is make it faster. but i don't understand "Your modification of contactImages...." and i feel like i'm not clear when i say the image deleted. What really happened the image delete even from native addressbook – Omarj Mar 20 '13 at 12:46
  • You're modifying `contactImagesDi` on a background thread, which can cause corruption in that data structure (which I assume is a dictionary) unless you're very careful. My first suspicion from this code on why you're damaging the contacts is your accessing the same `ABAddressBook` (or its Records) on multiple threads. Are you now making a fresh `ABAddressBookCreateWithOptions()` on every thread you use it on? If that isn't it, I would look for if/where you're writing to the `ABAddressBook` or records and suspect that code rather than reading code. – Rob Napier Mar 20 '13 at 13:01
  • mmmmm i will look at this point. i think i use the same addressbook ref for all my thread. thx 4 explain. – Omarj Mar 20 '13 at 14:27
  • If you use the addressbook or its records on multiple threads, you would expect it to cause random problems. It won't necessary; it's just undefined behavior. – Rob Napier Mar 25 '13 at 13:36