dispatch_group_wait with GCD

Question

So I'm posting an array of images to my server. I want to use GCD to post the array asynchronously, but I also want to make the method in which this happens synchronous so that I can pass back a single response object. However the method dispatch_group_wait seems to be returning immediately (and not waiting for my blocks to finish). Is this an issue because I'm using a block within a block?

NSArray *keys = [images allKeys];
__block NSMutableDictionary *responses = [NSMutableDictionary dictionaryWithCapacity:[images count]];

dispatch_group_t group = dispatch_group_create();
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
for (int i = 0; i < [keys count]; i++) {
    __block NSString *key = [keys objectAtIndex:i];
    dispatch_group_async(group, queue, ^{
        [self postImage:[images objectForKey:key] completionHandler:^(ServerResponse *response){
            @synchronized(responses){
                if ([response succeeded]) {
                    NSString *value = [[response data] objectForKey:@"image_token"];
                    [responses setObject:value forKey:key];
                    NSLog(@"inside success %@",responses);
                } else {
                    NSString *error = response.displayableError;
                    if (!error) {
                        error = @"Sorry something went wrong, please try again later.";
                    }
                    [responses setObject:error forKey:@"error"];
                    [responses setObject:response forKey:@"response"];
                }
            }
        }];
    });
}
dispatch_group_wait(group, DISPATCH_TIME_FOREVER);
dispatch_release(group);

I want to simply wait for all of the [self postImage] methods to callback from the server and modify the responses dictionary.

Answer 1

Jonathan's semaphore example is on target. However, I mentioned using a condition variable as an alternative, so I thought I'd at least post an example. In general, a CV can be used to wait on more general conditions other than just N workers.

Note that condition variables have their place (though not necessarily here), usually best when a lock is already necessary to mutate shared state, then other threads can just wait for a specific condition.

NSUInteger numKeys = [keys count];

NSConditionLock *conditionLock = [[NSConditionLock alloc] initWithCondition:numKeys];
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
for (NSUInteger i = 0; i < numKeys; i++) {
    __block NSString *key = [keys objectAtIndex:i];
    dispatch_async(queue, ^{
        [self postImage:[images objectForKey:key] completionHandler:^(ServerResponse *response){
            // Basically, nothing more than a obtaining a lock
            // Use this as your synchronization primitive to serialize access
            // to the condition variable and also can double as primitive to replace
            // @synchronize -- if you feel that is still necessary
            [conditionLock lock];

            ...;

            // When unlocking, decrement the condition counter
            [conditionLock unlockWithCondition:[conditionLock condition]-1];
        }];
    });
}

// This call will get the lock when the condition variable is equal to 0
[conditionLock lockWhenCondition:0];
// You have mutex access to the shared stuff... but you are the only one
// running, so can just immediately release...
[conditionLock unlock];

Answer 2

Without seeing the code for -postImage:completionHandler:, it's hard to say where things are getting scheduled, but I'm assuming they call through to something provided by iOS. If so, the handler blocks inside your block are dispatched to the global queue asynchronously, and then iOS-supplied function or method is returning immediately. As far as your dispatch group is concerned, the work is done almost instantly.

There's no easy way to get the group to wait for work that isn't already scheduled by the time to call to dispatch_group_wait() is made. However, we can add a lower-level thingamajigger called a semaphore that ensures our actions complete in the right order, and schedule it outside the scope of the inner (asynchronous) blocks.

NSUInteger numKeys = [keys count];

dispatch_group_t group = dispatch_group_create();
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
for (NSUInteger i = 0; i < numKeys; i++) {
    __block NSString *key = [keys objectAtIndex:i];
    dispatch_group_async(group, queue, ^{
        // We create a semaphore for each block here. More on that in a moment.
        // The initial count of the semaphore is 1, meaning that a signal must happen
        // before a wait will return.
        dispatch_semaphore_t sem = dispatch_semaphore_create(1);
        dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
        [self postImage:[images objectForKey:key] completionHandler:^(ServerResponse *response){
            ...
            // This instructs one caller (i.e. the outer block) waiting on this semaphore to resume.
            dispatch_semaphore_signal(sem);
        }];

        // This instructs the block to wait until signalled (i.e. at the end of the inner block.)
        dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);

        // Done with the semaphore. Nothing special here.
        dispatch_release(sem);
    });
}
dispatch_group_wait(group, DISPATCH_TIME_FOREVER);
// Now all the tasks should have completed.
dispatch_release(group);

There's a problem here though. Semaphores are a kernel resource. What if we have 100 tasks to perform, but the kernel can only supply 99 semaphores? Bad Things™ happen. We can rebuild the code to use only one semaphore, though waiting for it will appear a bit wonky. Doing so actually obviates the dispatch group entirely, by the way, so we're essentially replacing the group with the semaphore. Let's watch!

NSUInteger numKeys = [keys count];

// set the count of the semaphore to the number of times it must be signalled before
// being exhausted. Up to `numKeys` waits will actually wait for signals this way.
// Additional waits will return immediately.
dispatch_semaphore_t sem = dispatch_semaphore_create(numKeys);
for (int i = 0; i < numKeys; i++) {
    dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
}

dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
for (NSUInteger i = 0; i < numKeys; i++) {
    __block NSString *key = [keys objectAtIndex:i];
    dispatch_async(queue, ^{
        [self postImage:[images objectForKey:key] completionHandler:^(ServerResponse *response){
            ...;

            // This decrements the semaphore's count by one. The calling code will be
            // woken up by this, and will then wait again until no blocks remain to wait for.
            dispatch_semaphore_signal(sem);
        }];
    });
}

// At this point, all the work is running (or could have already completed, who knows?).
// We don't want this function to continue running until we know all of the blocks
// have run, so we wait on our semaphore a number of times equalling the number
// of signals we expect to get. If all the blocks have run to completion before we've
// waited for all of them, the additional waits will return immediately.
for (int i = 0; i < numKeys; i++) {
    dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
}
// Now all the tasks should have completed.
dispatch_release(sem);

Answer 3

Yes as has been stated, dispatch_group_wait() is not waiting because the postImage:completionHandler: call seems to be asynchronous. And if you REALLY need this block of code to execute synchronously then a semaphore or lock would seem to be an appropriate solution.

If, however you just want to collect all of the responses to one dictionary for processing, I believe that the most appropriate solution is to use GCD to it's full extent. And to use a dispatch queue to manage the mutable dictionary; This seems to be the solution preferred by apple in most of the documents I have seen.

The crux of the solution is to essentially transfer ownership of the mutable dictionary to a single queue and then only modify it from that queue. The 'ownership' I refer to is not object ownership in the memory management sense, but ownership in the right to modify sense.

I would consider doing something like this:

NSArray *keys = [images allKeys];
// We will not be reasigning the 'responses' pointer just sending messages to the NSMutableDictionary object __block is not needed.
NSMutableDictionary *responses = [NSMutableDictionary dictionaryWithCapacity:[images count]];
// Create a queue to handle messages to the responses dictionary since mutables are not thread safe.
// Consider this thread the owner of the dictionary.
dispatch_queue_t responsesCallbackQueue = dispatch_queue_create("com.mydomain.queue", DISPATCH_QUEUE_SERIAL);

for (NSString *key in keys) {
    // This async call may not be needed since postImage:completionHandler: seems to be an async call itself.
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ 
        [self postImage:[images objectForKey:key] completionHandler:^(ServerResponse *response){
            dispatch_async(responsesCallbackQueue, ^{
                [responses setObject:response forKey:key];
                // Perhaps log success for individual requests.
                if (responses.count == keys.count){
                    NSLog(@"All requests have completed");
                    // All responses are available to you here.
                    // You can now either transfer back 'ownership' of the dictionary.
                    dispatch_async(dispatch_get_main_queue(), ^{
                        [self requestsHaveFinished:responses];
                    });
                }
            });
        }];
    });
}