CODEWITHSUNDEEP
objective-casynchronousgrand-central-dispatchnsoperation
trapper
trapper

Reputation: 11993

What happens with unmatched dispatch_group_enter and dispatch_group_leave?

I have an async NSOperation to download data for multiple ImageFile objects. Since this is all happening asynchronously I am using a dispatch group to track the requests and then dispatch_group_notify to complete the operation when they are all done.

The question I have is what happen when the operation is ended prematurely, either by cancelation or by some other error. The dispatch group will be left with unmatched dispatch_group_enter and dispatch_group_leave so dispatch_group_notify will never be called. Is it the block retained somewhere by the system forever waiting, or will it get released when the NSOperation gets released?

Or is my approach not ideal, how else should I do this?

- (void)main
{
    if (self.cancelled) {
        [self completeOperation];
        return;
    }

    NSManagedObjectContext *context = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSPrivateQueueConcurrencyType];
    context.persistentStoreCoordinator = self.persistentStoreCoordinator;
    context.mergePolicy = NSMergeByPropertyObjectTrumpMergePolicy;

    [context performBlock:^{

        NSFetchRequest *request = [ImageFile fetchRequest];
        request.predicate = ....
        request.sortDescriptors = ...

        NSError *error;
        NSArray *imageFiles = [context executeFetchRequest:request error:&error];
        if (!imageFiles) {
            // Error handling...
            [self completeOperation];
            return;
        }

        dispatch_group_t group = dispatch_group_create();

        for (ImageFile *imageFile in imageFiles) {
            dispatch_group_enter(group);
            @autoreleasepool {
                [self.webService requestImageWithId:imageFile.id completionHandler:^(NSData * _Nullable data, NSURLResponse * _Nullable response, NSError * _Nullable error) {

                    if (self.cancelled) {
                        [self completeOperation];
                        return;
                    }

                    [context performBlock:^{
                        if (data && !error) {
                            imageFile.data = data;
                            NSError *error;
                            if (![context save:&error]) {
                                // Error handling...
                                [self completeOperation];
                                return;
                            }
                        }
                        dispatch_group_leave(group);
                    }];
                }];
            }
        }

        dispatch_group_notify(group, dispatch_get_main_queue(), ^{
            [self completeOperation];
        });
    }];
}

Upvotes: 2

Views: 1876

Answers (1)

Ken Thomases
Ken Thomases

Reputation: 90531

From the docs for dispatch_group_enter():

A call to this function must be balanced with a call to dispatch_group_leave.

From the docs for dispatch_group_t:

The dispatch group keeps track of how many blocks are outstanding, and GCD retains the group until all its associated blocks complete execution.

It talks about outstanding blocks, but what it really means is unmatched calls to dispatch_group_enter().

So, the answer to your question about what happens is that the dispatch group object effectively leaks. The block object passed to dispatch_group_notify() and any objects it has strong references to also leak. In your case, that includes self.

The answer to your question of whether your approach is "ideal" is: no, it's not ideal. It's not even valid by the design contract of GCD. You must balance all calls to dispatch_group_enter() with calls to dispatch_group_leave().

If you want to somehow distinguish between success and failure or normal completion and cancellation, you should set some state that's available to the notify block and then code the notify block to consult that state to decide what to do.

In your case, though, the code paths where you fail to call dispatch_group_leave() just do the same thing the notify block would do. So I'm not even sure why you're not just calling dispatch_group_leave() rather than calling [self completeOperation] in those cases.

Upvotes: 4

Related Questions