How do I prevent from a dispatch_group from getting stuck?

Question

How do I prevent from a dispatch_group from getting stuck? I have found to be possible to get stuck in the following code (with or without the dispatch_group_wait call) if one of the images I attempt to load is not loaded (e.g. due to bad url). The block in dispatch_group_notify is never called.

dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0);
dispatch_group_t group = dispatch_group_create();

for (...) {
        if (...) {
            dispatch_group_enter(group);
            dispatch_async(queue, ^{

               [self loadImageWithUrl:url onCompletion:^{
                     dispatch_group_leave(group);
               }];
            });
        }
    }

    dispatch_group_wait(group, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2.0 * NSEC_PER_SEC)));
    dispatch_group_notify(group, queue, ^{
        NSLog(@"load image complete");
    });

Answer 1

dispatch_group_notify queues its block when the group is complete. Your group never completes. So don't use dispatch_group_notify. Just use dispatch_group_wait as you are to wait with a timeout, then dispatch your block:

    ...    
    dispatch_group_wait(group, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2.0 * NSEC_PER_SEC)));
    dispatch_async(queue, ^{
        NSLog(@"load image complete");
    });

If you want to mimic a dispatch_group_notify with a timeout, just do the above in its own async block:

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
  dispatch_group_wait(group, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2.0 * NSEC_PER_SEC)));
  dispatch_sync(queue, ^{
      NSLog(@"load image complete");
  });
});

Note that you can use the return value of dispatch_group_wait to determine if everything completed or if it timed-out if that is useful information to you.

Keep in mind that the previous blocks will not be cancelled, so they may eventually run their completion blocks. You may need to add cancellation logic to the system if that's a problem.

Answer 2

I do not think the issue is with your group notify process. For me, the issue that leaps out at me is that, rather than trying to handle the scenario where the completion block is not called, that you change loadImageWithUrl to ensure that it always calls the completion block, whether successful or not. You might even want to add a NSError parameter to the block or something like that, so the caller will be notified if something failed (for example to warn the user, or initiate Reachability process that will wait for the connection to be re-established before attempting a retry, etc).

So, it might look like:

- (void)loadImageWithUrl:(NSURL *)url loadImageWithUrl:(void (^)(NSError *error))block
{
    BOOL success;
    NSError *error;

    // do your download, setting `success` and `error` appropriately

    // then, when done, call the completion block, whether successful or not

    if (block) {
        if (success) {
            block(nil);
        } else {
            block(error);
        }
    }
}

Clearly, the details of the above are entirely dependent upon how you're doing these requests, but that's the basic idea. Then, you just make sure that your caller is changed to include this extra parameter:

for (...) {
    if (...) {
        dispatch_group_enter(group);
        dispatch_async(queue, ^{

            [self loadImageWithUrl:url onCompletion:^(NSError *error){
                if (error) {
                    // handle the error however you want, if you want
                }

                dispatch_group_leave(group);
            }];
        });
    }
}

I care less about how you choose to handle the error than I do in encouraging you ensure your completion block is called regardless of whether the download was successful or not. This ensures that the number of times you enter the group is perfectly balanced with the number of times you leave the group.

---

Having said that, when downloading many resources, GCD is ill-suited for this task. The issue is that it's non-trivial to constrain GCD to how many concurrent tasks can be performed at one time. Generally, you want to constrain how many requests that can run concurrently. You do this because (a) there's a limit as to how many NSURLSessionTask or NSURLConnection requests can run concurrently anyway; (b) if you run more than that, on slow connections you run serious risk of requests timing-out unnecessarily; (c) you can reduce your app's peak memory usage; but (d) you still enjoy concurrency, striking a balance between memory usage and optimal network bandwidth optimization.

To accomplish this, a common solution is to use operation queues rather than GCD's dispatch queues. You can then wrap your download requests in NSOperation objects and add these network operation to a NSOperationQueue for which you have set some reasonable maxConcurrentOperationCount (e.g. 4 or 5). And instead of a dispatch group notify, you can add a completion operation which is dependent upon the other operations you've added to your queue.

If you don't want to implement this yourself, you can use AFNetworking or SDWebImage, which can facilitate the downloading of images using operation queues to manage the download process.

---

And one final thought is that many apps adopt a lazy loading process, where images are seamlessly loaded as they're needed. It avoids consuming too much of the user's data plan performing some bulk download (or risking that the image the user needs first is backlogged behind a bunch of other images they don't immediately need). Both AFNetworking and SDWebImage offer UIImageView categories that offer an incredibly simple lazy loading of images.