NSOperationQueue and threads problem

Question

Well I have a serious problem. I have a class that manages connections with NSOperationQueue and delegate methods for that class that are actually NSURLConnection delegate methods. Sometimes, I have to pop the viewController that has the answer of my delegate class but it gets trapped in the scope of the delegate function, populating a UITableView and the app crashes just there. I solved it using @synchronized on the viewController dealloc and on the NSURLConnection delegates, but I don't think is the cleanest way since UI freezes sometimes when I'm adding a lot of info to the tableView. So, is there a way to do this clean?

- (void)viewDidLoad:(BOOL)animated {
[myManager startRequest];
}

//myManager class callback
- (void)managerDelegate {
//Doing things and just poped the viewController while in the function scope
//Program crashes, most of logs are "UIViewController message sent to deallocated instance"
}

//viewController dealloc
- (void)dealloc
{
    @synchronized([Manager class])
    {
    alert.delegate = nil;
    searchResultsTableView.delegate = nil;
    searchResultsTableView.dataSource = nil;
    [searchResultsTableView release]; searchResultsTableView = nil;
    [serviceSearch release]; serviceSearch = nil;
    [searchResults release]; searchResults = nil;
    [XMLTag release]; XMLTag = nil;
    [XMLParserServicesKeys release]; XMLParserServicesKeys = nil;
    [XMLParserKeys release]; XMLParserKeys = nil;
    [searchString release]; searchString = nil;
    [__managedObjectContext release]; __managedObjectContext = nil;
    manager.delegate = nil; 
    [manager stopAllRequests];
    [manager release]; manager = nil;
    [super dealloc];
    }
}

Edit: some more code, now of myManager class

- (void) stopAllRequests
{
#ifdef _WSMANAGER_DEBUG_
    NSLog(@"stopAllRequests %d", [connectionsArray count]);
#endif
    [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:NO];

    for(NNSURLConnection* connection in connectionsArray)
    {
        [connection cancel];
        [connection release];
    }


    [connectionsArray removeAllObjects];    
    [queue cancelAllOperations];
}


- (BOOL)startRequest
{   
//Data initialization       
    NSInvocationOperation* operation = [[NSInvocationOperation alloc] initWithTarget:self selector:@selector(beginConnection:) object:[NSDictionary dictionaryWithObjectsAndKeys:request, kRequestKey, keyInfo, kUserInfoKey, nil]];
    [queue addOperation:operation];
    [operation release];
    return YES;
}

-(void) beginConnection:(id)object
{   
    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];

    NNSURLConnection* connection = [[NNSURLConnection alloc] initWithRequest:[object objectForKey:kRequestKey] delegate:self];

    if(connection)
    {
        NSMutableData *requestData = [[NSMutableData alloc] init];

        connection.url = [((NSURLRequest*)[object objectForKey:kRequestKey]) URL];

        connection.userInfo = [NSDictionary dictionaryWithObjectsAndKeys:[object objectForKey:kUserInfoKey], kUserInfoKey, requestData, kRequestDataKey, nil];
        [connectionsArray addObject:connection];

        [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:YES];
    }

    [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:TIMEOUT]];

    if([connectionsArray indexOfObject:connection] != NSNotFound)
    {
        [connection cancel];

        if([delegate conformsToProtocol:@protocol(ManagerDelegate)] && [delegate respondsToSelector:@selector(managerFailed:withKey:errorCode:)]) {
            [delegate managerFailed:self withKey:[connection.userInfo objectForKey:kUserInfoKey] errorCode:ManagerErrorCodeTimeout];
            if([connectionsArray count] < 1)
                [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:NO];
        }

        [connectionsArray removeObject:connection];
    }
    [pool drain];
}

- (void)connectionDidFinishLoading:(NNSURLConnection *)connection {
    @synchronized([Manager class])
    {   
    NSMutableData *requestData = [connection.userInfo objectForKey:kRequestDataKey];
    NSString* responseString = [[NSString alloc] initWithData:requestData encoding:NSUTF8StringEncoding];       
    if([delegate conformsToProtocol:@protocol(PLMWSManagerDelegate)] && [delegate respondsToSelector:@selector(managerSuccess:responseString:withKey:)])
        [delegate managerSuccess:self responseString:responseString withKey:[connection.userInfo objectForKey:kUserInfoKey]];
    [responseString release];
    [requestData release];
    [connectionsArray removeObject:connection];
    [connection cancel];
    [connection release]; connection = nil;

    if([connectionsArray count] < 1)
        [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:NO];
    }
}

Answer 1

When an application fetches data from INTERNET to populate GUI, keeping GUI and INTERNET thread totally separate will be help full. Following options did worked for me in same situation.

1. Use – cancelAllOperations method of NSOperationQueue. this will cancel the quesue after operation that is running at present. If you do not cancel the operation then it will run on background.

2. Keep a bridge data structure between table and INTERNET thread. Keep this in some other class then viewController so that it do not get released when you pop the viewcontroller.

3. Populate table from that data structure and fill newly available data in that. Notify table using a NSNotification OR some other method when new data is available. So here in this case if you keep you operation queue running in background it will not go to update the tableView.

EDIT:

Moreover, As you are using NSURLconnection you can use - (void)cancel method to cancel the connection. Delegate will no longer get calls once you cancel the connection. It can be helpful in two ways.

1. Cancel your last operation.

2. Implement something like this in you class that calls child of custom delegate.

Hope above method is helpful.

Answer 2

Standard opening comment, given prominence with an eye to the future: Apple's contributions to the LLVM project suggest that iOS 5 will come with self-zeroing weak references. So objects can hold references to other objects without owning them, and when those objects are deallocated all the pointers to them are magically set to nil. We'll have to wait for the public release of the developer tools to find out whether iOS implements that functionality (it needs some runtime support for obvious reasons) and, if so, whether Apple are using it for delegates. However, that's likely to be quite soon and as a developer you should be able to get the NDA version of the tools that is currently available, so it may be worth considering that route to a solution depending on the other practicalities of your project.

More helpful for the here and now:

The delegate for NSURLConnections is immutable. Assuming the NSURLConnections are created by tasks on the NSOperationQueue that you mention, you may also be creating them on some runloop other than that attached to the main thread, bringing a whole sweep of thread safety issues into the mix.

I'd suggest that the smart thing to do is:

1. ensure you're attaching all NSURLConnections to the main runloop. To do that you should create connections so that they don't start immediately, use scheduleInRunLoop:forMode: to nominate [NSRunLoop mainRunLoop] (and probably NSDefaultRunLoopMode), then start them.
2. simultaneously, keep a list of all connections that you have started. You'll probably want to use an @synchronized block to add then to a suitable NSMutableArray
3. when the object that is nominated to receive delegate methods is to be deallocated, perform waitUntilAllOperationsAreFinished on the operation queue and then send cancel to all ongoing connections (e.g. [self.arrayOfConnections makeObjectsPerformSelector:@selector(cancel)])

Connections are guaranteed not to communicate with their delegates after they have received the cancel message. You want to wait until all operations in the queue are finished to avoid a potential race condition whereby the view controller is deallocated having successfully killed all connections at some relevant time but a not-yet-completed operation then tries to add a new one.

---

Additional, based on our comment discussion on the question:

NSURLConnection has a built-in system to run asynchronously on a run loop. Run loops are Apple's version of an event loop, being in rough terms a list of messages to post. So they let you use a single thread to do lots of things but only if no single thing blocks the thread.

Apple actually recommend that the most efficient way (in terms of processing and battery life) to do an asynchronous URL access is by allowing NSURLConnection to run asynchronously on the main thread. You might adjust your current -(void) startConnection to:

- (BOOL)startRequest
{   
    // we're going to do all this right here on the main thread, so there's
    // no need to package 'request' and 'keyInfo' into a dictionary and create
    // an operation

    // create the connection
    NNSURLConnection* connection = [[NNSURLConnection alloc] initWithRequest:request delegate:self];

    if(connection)
    {
        NSMutableData *requestData = [[NSMutableData alloc] init];

        connection.url = [keyInfo URL];

        connection.userInfo = [NSDictionary dictionaryWithObjectsAndKeys:keyInfo, kUserInfoKey, requestData, kRequestDataKey, nil];
        [connectionsArray addObject:connection];

        [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:YES];
    }

    // We used initWithRequest:delegate: so the request has already started.
    // The connection will now run asynchronously and we can wait for
    // delegate messages, which will be delivered via the runloop on this thread
}

Then put the other things you were doing after the connection has ended into your connectionDidFinishLoading::

- (void)connectionDidFinishLoading:(NNSURLConnection *)connection {

    if([connectionsArray indexOfObject:connection] != NSNotFound)
    {
        [connection cancel];

        if([delegate conformsToProtocol:@protocol(ManagerDelegate)] && [delegate respondsToSelector:@selector(managerFailed:withKey:errorCode:)]) {
            [delegate managerFailed:self withKey:[connection.userInfo objectForKey:kUserInfoKey] errorCode:ManagerErrorCodeTimeout];
            if([connectionsArray count] < 1)
                [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:NO];
        }

        [connectionsArray removeObject:connection];
    }

    /* ...and all the other stuff here... */
}

I've assumed throughout that NNSURLConnection is a subclass of NSURLConnection that just adds a few extra properties, not otherwise affecting behaviour.

Answer 3

There is perhaps a very good reason why you are doing things this way, but why are you not using the third party library ASIHttpConnection?

It manages URL connection details and even comes with a built in operation queue.

One other thing of note, as Tommy Obliquely notes ARC is upcoming (ARC has been publicly announced, therefore it's OK to mention it exists). Since ARC supports 4.0 devices, it would be a REALLY good idea to move to that if at all possible. You may run into some issues but it will probably cure more problems than it will cause.