CODEWITHSUNDEEP
objective-cioscrashthread-safetyblock
suhail
suhail

Reputation: 63

Objective-C thread safe code to prevent a crash

I got an iPhone crash report with a SIGSEGV and I think I've narrowed down the possible cause and a solution. Since crashes caused by threads are hard to debug I can't repro this problem, but could use some help with my hypothesis - is it sound?

My code uses ASIHttpRequest to download a set of files using an ASINetWorkQueue. Here is a simplified sample

//initialize download queue and do this code block in a loop for each file

NSURL *fileURL = [NSURL URLWithString:...
__block ASIHTTPRequest *fileRequest = [ASIHTTPRequest requestWithURL:fileURL];

[fileRequest setCompletionBlock:^{
   //do some stuff   
}];
[fileRequest setFailedBlock:^{
    NSString *someError = [NSString stringWithFormat:...
    [self someErrorMethod:someError];       
}];

[downloadQueue addOperation:...

-(void)someErrorMethod(NSString *errorMessage) {
    DDLogWarn(errorMessage);

    if ([self downloadQueue]) {
        for (ASIHTTPRequest *request in [[self downloadQueue] operations]) {
            [request clearDelegatesAndCancel];
        }
        [[self downloadQueue] reset];
    }
 }

The top 2 lines of the crash report are

My thinking for why this happened

Does this make sense? Since I am newish to Objective-C, is my analysis correct or am I missing something obvious?

I am thinking of using a lock to make the errorMethod thread safe with the hope that it will fix this issue. Does that sound like the right solution based on the code above?

Thanks

Upvotes: 2

Views: 1251

Answers (1)

Rob Napier
Rob Napier

Reputation: 299275

This doesn't sound likely. ASIHttpRequest likely performs all of its callbacks on the same thread (I'm fairly certain on this one).

If I had to guess, your error is more likely in this line:

DDLogWarn(errorMessage);

The first parameter to DDLogWarn is a format, not a string. This will likely crash in any case that errorMessage includes a %. What you meant is:

DDLogWarn(@"%@", errorMessage);

Since DDLogWarn() is a varags method, it will start substituting the (random) values it finds on the stack for any % substitutions in the string. It will read the stack until you run out of % substitutions. If any of the % substitutions are pointer-based (like %s or %@), then it will follow the pointer to a random location.

SEG_ACCERR means that you've requested a piece of memory you don't own. SEG_MAPERR means you've requested a piece of memory that is not mapped. Either is an expected result of following a totally random pointer.

Upvotes: 4

Related Questions