Waiting on asynchronous methods using NSCondition

Question

I am downloading four plist files asynchronously over the internet. I need to wait until all four files are downloaded, until I either on the first run, push a UIViewController, or on all subsequent runs, refresh the data, and reload all my UITableViews.

On the first run, everything works perfectly. When refreshing though, all four url requests are called, and started, but never call their completion or failure blocks, and the UI freezes. Which is odd since I preform all operations in a background thread. I have not been able to figure out why this is happening.

The first load and the refresh methods call the four "update" methods in the same way, and use NSCondition in the same way.

For the first run:

- (void)loadContentForProgram:(NSString *)programPath
{
    NSLog(@"Start Load Program");
    AppDelegate *myDelegate = (AppDelegate *)[UIApplication sharedApplication].delegate;
    hud = [[MBProgressHUD alloc] initWithView:myDelegate.window];
    [myDelegate.window addSubview:hud];
    hud.labelText = @"Loading...";
    hud.detailsLabelText = @"Loading Data";
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{
        //Do stuff here to load data from files

        //Update From online files
        hud.detailsLabelText = @"Updating Live Data";
        resultLock = NO;
        progressLock = NO;
        recallLock = NO;
        stageLock = NO;

        condition = [[NSCondition alloc] init];
        [condition lock];

        [self updateCurrentCompsText];
        [self updateCompetitionResults];
        [self updateCompetitionRecalls];
        [self updateCompetitionProgress];


        while (!resultLock) {
            [condition wait];
        }
        NSLog(@"Unlock");
        while (!stageLock) {
            [condition wait];
        }
        NSLog(@"Unlock");
        while (!recallLock) {
            [condition wait];
        }
        NSLog(@"Unlock");
        while (!progressLock) {
            [condition wait];
        }
        NSLog(@"Unlock");
        [condition unlock];
        updateInProgress = NO;
        //Reset Refresh controls and table views
        self.refreshControlsArray = [[NSMutableArray alloc] init];
        self.tableViewsArray = [[NSMutableArray alloc] init];
        NSLog(@"Finished Loading Program");
        [[NSNotificationCenter defaultCenter] postNotificationName:@"WMSOFinishedLoadingProgramData" object:nil]; //Pushes view controller
        dispatch_async(dispatch_get_main_queue(), ^{
            [MBProgressHUD hideHUDForView:myDelegate.window animated:YES];
        });
    });
}

When refreshing data:

- (void)updateProgramContent
{
    if (!updateInProgress) {
        updateInProgress = YES;
        for (int i = 0; i < self.refreshControlsArray.count; i++) {
            if (!((UIRefreshControl *)self.refreshControlsArray[i]).refreshing) {
                [self.refreshControlsArray[i] beginRefreshing];
                [self.tableViewsArray[i] setContentOffset:CGPointMake(0.0, 0.0) animated:YES];
            }
        }

        resultLock = NO;
        stageLock = NO;
        recallLock = NO;
        progressLock = NO;
        dispatch_sync(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{

            condition = [[NSCondition alloc] init];
            [condition lock];

            [self updateCompetitionProgress];
            [self updateCompetitionRecalls];
            [self updateCompetitionResults];
            [self updateCurrentCompsText];

            while (!resultLock) {
                [condition wait];
            }
            NSLog(@"Unlock");
            while (!stageLock) {
                [condition wait];
            }
            NSLog(@"Unlock");
            while (!recallLock) {
                [condition wait];
            }
            NSLog(@"Unlock");
            while (!progressLock) {
                [condition wait];
            }
            NSLog(@"Unlock");
            [condition unlock];
        });

        for (int i = 0; i < self.refreshControlsArray.count; i++) {
            [self.refreshControlsArray[i] performSelector:@selector(endRefreshing) withObject:nil afterDelay:1.0];
            [self.tableViewsArray[i] performSelector:@selector(reloadData) withObject:nil afterDelay:1.0];
        }
        updateInProgress = NO;
    }
}

The block below that appears in each loading method above, corresponds to a method that will download and update a specific piece of data.

[self updateCompetitionProgress];
[self updateCompetitionRecalls];
[self updateCompetitionResults];
[self updateCurrentCompsText];

which runs:

- (void)updateCompetitionResults
{
    __block NSDictionary *competitionResultsData = nil;
    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"Some URL",[self.programName stringByReplacingOccurrencesOfString:@" " withString:@"%20"]]] cachePolicy:NSURLCacheStorageNotAllowed timeoutInterval:20.0];
    AFPropertyListRequestOperation *operation = [AFPropertyListRequestOperation propertyListRequestOperationWithRequest:request success:^(NSURLRequest *request, NSHTTPURLResponse *response, id propertyList) {
        competitionResultsData = (NSDictionary *)propertyList;
        [competitionResultsData writeToFile:[@"SOME LOCAL PATH"] atomically:NO];
        [self updateCompetitionResultsWithDictionary:competitionResultsData];
    } failure:^(NSURLRequest *request, NSHTTPURLResponse *response, NSError *error, id propertyList) {
        competitionResultsData = [NSDictionary dictionaryWithContentsOfFile:[@"SOME LOCAL PATH"]];
        NSLog(@"Failed to retreive competition results: %@", error);
        [self updateCompetitionResultsWithDictionary:competitionResultsData];
    }];
    [operation start];
}

and the completion and failure blocks call the same method to update the data

- (void)updateCompetitionResultsWithDictionary:(NSDictionary *)competitionResultsData
{
    //Do Stuff with the data here
    resultLock = YES;
    [condition signal];
}

So, Why does this work on the first run, but not any of the subsequent runs?

Answer 1

As I mentioned in my comments, above, the most obvious problem is that you're invoking methods that use condition before you initialize condition. Make sure initialize condition before you start calling updateCompetitionResults, etc.

---

In terms of a more radical change, I might suggest retiring NSCondition altogether, and use operation queues:

1. I might use NSOperationQueue (or you can use dispatch groups, too, if you want, but I like the operation queue's ability to configure how many concurrent operations you can operate ... also if you get to a point that you want to cancel operations, I think NSOperationQueue offers some nice features there, too). You can then define each download and processing as a separate NSOperation (each of the downloads should happen synchronously, because they're running in an operation queue, you get the benefits of asynchronous operations, but you can kick off the post-processing immediately after the download is done). You then just queue them up to run asynchronously, but define a final operation which is dependent upon the other four will kick off as soon as the four downloads are done. (By the way, I use NSBlockOperation which provides block-functionality for NSOperation objects, but you can do it any way you want.)

2. And whereas your updateProgramContent might download asynchronously, it processes the four downloaded files sequentially, one after another. Thus, if the first download takes a while to download, it will hold up the post-processing of the others. Instead, I like to encapsulate both the downloading and the post processing of each of the four plist files in a single NSOperation, each. Thus, we enjoy maximal concurrency of not only the downloading, but the post-processing, too.

3. Rather than using the AFNetworking (which I'm generally a big fan of) plist-related method, I might be inclined to use NSDictionary and NSArray features that allow you to download a plist from the web and load them into the appropriate structure. These dictionaryWithContentsOfURL and arrayWithContentsOfURL run synchronously, but because we're doing this in a background operation, everything runs asynchronously like you want. This also bypasses the saving them to files. If you wanted them saved to files in your Documents directory, you can do that easily, too. Clearly, if you're doing something sophisticated in your downloading of the plist files (e.g. your server is engaging in some challenge-response authentication), you can't use the convenient NSDictionary and NSArray methods. But if you don't need all of that, the simple NSDictionary and NSArray methods, ___WithContentsOfURL make life pretty simple.

Pulling this all together, it might look like:

@interface ViewController ()

@property (nonatomic, strong) NSArray *competitions;
@property (nonatomic, strong) NSDictionary *competitionResults;
@property (nonatomic, strong) NSDictionary *competitionRecalls;
@property (nonatomic, strong) NSDictionary *competitionProgress;

@end

@implementation ViewController

- (void)viewDidLoad
{
    [super viewDidLoad];

    [self transfer];
}

- (void)allTransfersComplete
{
    BOOL success;

    if (self.competitions == nil)
    {
        success = FALSE;
        NSLog(@"Unable to download competitions");
    }

    if (self.competitionResults == nil)
    {
        success = FALSE;
        NSLog(@"Unable to download results");
    }

    if (self.competitionRecalls == nil)
    {
        success = FALSE;
        NSLog(@"Unable to download recalls");
    }

    if (self.competitionProgress == nil)
    {
        success = FALSE;
        NSLog(@"Unable to download progress");
    }

    if (success)
    {
        NSLog(@"all done successfully");
    }
    else
    {
        NSLog(@"one or more failed");
    }
}

- (void)transfer
{
    NSURL *baseUrl = [NSURL URLWithString:@"http://insert.your.base.url.here/competitions"];
    NSURL *competitionsUrl = [baseUrl URLByAppendingPathComponent:@"competitions.plist"];
    NSURL *competitionResultsUrl = [baseUrl URLByAppendingPathComponent:@"competitionresults.plist"];
    NSURL *competitionRecallsUrl = [baseUrl URLByAppendingPathComponent:@"competitionrecalls.plist"];
    NSURL *competitionProgressUrl = [baseUrl URLByAppendingPathComponent:@"competitionprogress.plist"];

    NSOperationQueue *queue = [[NSOperationQueue alloc] init];
    queue.maxConcurrentOperationCount = 4; // if your server doesn't like four concurrent requests, you can ratchet this back to whatever you want

    // create operation that will be called when we're all done

    NSBlockOperation *completionOperation = [NSBlockOperation blockOperationWithBlock:^{

        // any stuff that can be done in background should be done here

        [[NSOperationQueue mainQueue] addOperationWithBlock:^{

            // any user interface stuff should be done here; I've just put this in a separate method so this method doesn't get too unwieldy

            [self allTransfersComplete];
        }];
    }];

    // a variable that we'll use as we create our four download/process operations

    NSBlockOperation *operation;

    // create competitions operation

    operation = [NSBlockOperation blockOperationWithBlock:^{

        // download the competitions and load it into the ivar
        //
        // note, if you *really* want to download this to a file, you can 
        // do that when the download is done

        self.competitions = [NSArray arrayWithContentsOfURL:competitionsUrl];

        // if you wanted to do any post-processing of the download
        // you could do it here.            
        NSLog(@"competitions = %@", self.competitions);
    }];
    [completionOperation addDependency:operation];

    // create results operation

    operation = [NSBlockOperation blockOperationWithBlock:^{

        self.competitionResults = [NSDictionary dictionaryWithContentsOfURL:competitionResultsUrl];

        NSLog(@"competitionResults = %@", self.competitionResults);
    }];
    [completionOperation addDependency:operation];

    // create recalls operation

    operation = [NSBlockOperation blockOperationWithBlock:^{

        self.competitionRecalls = [NSDictionary dictionaryWithContentsOfURL:competitionRecallsUrl];

        NSLog(@"competitionRecalls = %@", self.competitionRecalls);
    }];
    [completionOperation addDependency:operation];

    // create progress operation

    operation = [NSBlockOperation blockOperationWithBlock:^{

        self.competitionProgress = [NSDictionary dictionaryWithContentsOfURL:competitionProgressUrl];

        NSLog(@"competitionProgress = %@", self.competitionProgress);
    }];
    [completionOperation addDependency:operation];

    // queue the completion operation (which is dependent upon the other four)

    [queue addOperation:completionOperation];

    // now queue the four download and processing operations

    [queue addOperations:completionOperation.dependencies waitUntilFinished:NO];
}

@end

Now, I don't know which of your plist's are arrays and which are dictionaries (in my example, I made competitions an array and the rest were dictionaries keyed by the competition id), but hopefully you get the idea of what I was shooting for. Maximize concurrency, eliminate NSCondition logic, really make the most of NSOperationQueue, etc.

This may be all to much to take in, but I only mention it as an alternative to NSCondition. If your current technique works, that's great. But the above outlines how I would tackle a challenge like this.