How to handle setDelegate: when using multipe threads

Question

Hi all,

I've come across an issue with using a third party library, and am not sure what the common pattern to solve it is.

I'm using the asi-http-request class, which fetches http objects asynchronously using a thread.

In my objects dealloc() method, I do

[request setDelegate:nil];
[request release];

However the delegate is sometimes still called after this has happened. (I can see when this happens the delegate field of the request object is nil.) This sometimes causes a crash if the delegate has been destroyed already.

I believe this is a race condition. The code from ASIHTTPRequest that calls the delegate looks like this:

// Let the delegate know we are done
if ([self didFinishSelector] && [[self delegate] respondsToSelector:[self didFinishSelector]]) {
    [[self delegate] performSelectorOnMainThread:[self didFinishSelector] withObject:self waitUntilDone:[NSThread isMainThread]];
}

The problem happens if the performerSelectorOnMainThread has been called (but not completed) when the setDelegate call happens on the main thread.

One solution would be to add a wrapper around 'didFinishSelector' that checks (on the main thread) that the delegate is still non-nil before calling the selector, but this would result in a lot of wrappers.

There is some background here:

http://groups.google.com/group/asihttprequest/browse_thread/thread/721220b9645f4a42

All suggestions on the "normal" solution for this appreciated!

Thanks

Joseph

Answer 1

For dealing with objects across threads, you should almost always retain them. Basically,

id delegate = [[self delegate] retain];
if ([self didFinishSelector] && [delegate respondsToSelector:[self didFinishSelector]]) {
    [delegate performSelectorOnMainThread:[self didFinishSelector]
                               withObject:self 
                            waitUntilDone:[NSThread isMainThread]];
}
[delegate release];

Technically, the delegate could be dealloc-ed in between [self delegate] and the subsequent retain, and I'm not at all sure if Apple's @synthesized atomic accessors protect against this or not, but I believe the only way to solve this is in the accessor,

[[delegate retain] autorelease];

best of luck, race conditions get the best of us!

Answer 2

My original thoughts (wrapper around 'didFinishSelector' that checks on the main thread that the delegate is still non-nil before calling the selector) turned out to be the correct solution, as confirmed by helpful folks over on the apple dev forums:

https://devforums.apple.com/message/255935#255935

To avoid my worry of ending up with lots of wrappers, I managed to create only a single wrapper:

- (void)callSelectorCallback:(SEL *)selectorPtr withTarget:(id *)targetPtr
{
    id target = *targetPtr;
    SEL selector = *selectorPtr;

    if (!selector || !target)
        return;

    if ([target respondsToSelector:selector])
    {
        [target performSelector:selector withObject:self];
    }
}

- (void)callSelector:(SEL *)selector withDelegate:(id *)target
{
    if (!*selector || !*target)
        return;

    SEL callback = @selector(callSelectorCallback:withTarget:);
    NSMethodSignature *signature = [ASIHTTPRequest instanceMethodSignatureForSelector:callback];
    NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature];
    [invocation setSelector:callback];
    [invocation setTarget:self];
    [invocation setArgument:&selector atIndex:2];
    [invocation setArgument:&target atIndex:3];

    [invocation performSelectorOnMainThread:@selector(invoke) withObject:nil waitUntilDone:[NSThread isMainThread]];
}

then when I want to call the delegate, the code just looks something like this:

[self callSelector:&didFinishSelector withDelegate:&delegate];

As best I can tell from experiments & code analysis (and assuming setDelegate is only called from the main thread), this is 100% safe. It could be made safe for the non-main thread calls to setDelegate by taking the object lock inside callSelectorCallback.