dispatch_group_notify being called as many times as dispatch_group_leave is called in nested blocks

Question

I have this logic below; they are basically 3 nested dispatch group blocks.

1. The first group (group) will perform 3 short async tasks (just download data from a web service) and 1 longer async task: Upload unsynced records to the web service, delete the synced ones locally and finally download the records from the web service (first an array with the IDs and basic info and then every single of these records).
2. The second group (saveGroup) is part on the longer task. It will wait until all of those unsynced records requests to the web service are completed.
3. The third one (downloadGroup) will wait until all of those single records download request to the service are completed.

Everything goes fine until the third dispatch group. As you can see I get the IDs and basic info for the records on the server, loop through the array and call dispatch_group_enter with the downloadGroup and then fire the HTTP request. The dispatch_group_leave is called when the request is completed. I can see the dispatch_group_leave being called for every request and finally dispatch_group_notify is called as many times.

-(void) doTheSync {
    dispatch_group_t group = dispatch_group_create();

    [self syncFirstDataWithGroup: group];
    [self syncSecondDataWithGroup: group];
    [self syncThirdDataWithGroup: group];

    [self syncRecordsWithExternalGroup: group];

    dispatch_group_notify(group, dispatch_get_main_queue(), ^{
        NSLog(@"Finished all the sync");
    });
}

-(void) syncRecordsWithExternalGroup: (dispatch_group_t) externalGroup {
    dispatch_group_enter(externalGroup);

    NSError* error = nil;
    ConfigureDataModelHandler* configDataHandler = [ConfigureDataModelHandler sharedCoredata];
    WebserviceLib* RESTClient = [WebserviceLib sharedInstance];

    //get all unsynced records (f
    NSArray* recordsUnsynced = [configDataHandler getAllRecordsWithSynced: NO ignoreDelete: YES withError: &error];

    if (!error) {
        //upload them to the BE (and mark as synced if succeed

        dispatch_group_t saveGroup = dispatch_group_create();

        //get full record dictionary, save and mark as synced
        for (CMrecord* record in recordsUnsynced) {
            NSDictionary* recordDict = [configDataHandler exportToDictionary: record.recordID.integerValue];

            dispatch_group_enter(saveGroup);

            [RESTClient saverecord: recordDict onComplete:^(AFHTTPRequestOperation *operation, id responseObject) {
                NSLog(@"Saved unsynced record (%@) to BE", record.recordID);

                //mark as synced
                [configDataHandler markrecordAsSynced: record.recordID.integerValue];
                dispatch_group_leave(saveGroup);
            } onError:^(AFHTTPRequestOperation *operation, NSError *error) {
                NSLog(@"Error saving unsynced record to BE %@", error);
                dispatch_group_leave(saveGroup);
            }];
        }

        //** NOTIFY FINISH SAVING **
        dispatch_group_notify(saveGroup, dispatch_get_main_queue(), ^{
            NSLog(@"Finished saving all unsynced records to BE");

            //delete all synced records
            //TODO: Check if this makes sense. Probably better to not delete anything until we got the full record from the BE...
            [configDataHandler deleteRecordsSynced];

            //download small records from BE
            NSString* agentNationalID = [self.coreData getLoginStatus].nationalID;
            [RESTClient getRecordsForAgent: agentNationalID onComplete:^(NSInteger completeCode, NSArray *responseArray) {
                NSLog(@"Success getting the records %@", responseArray);

                if (completeCode == 200) {
                    dispatch_group_t downloadGroup = dispatch_group_create();

                    for (NSDictionary* shortDict in responseArray) {
                            dispatch_group_enter(downloadGroup);

                            //download full records from BE
                            [RESTClient getRecordByCodeAndTimestamp: shortDict onComplete:^(NSInteger completeCode, NSDictionary *responseDictionary) {
                                NSLog(@"Success Downloading record");
                                dispatch_group_leave(downloadGroup);
                            } onError:^(AFHTTPRequestOperation *operation, NSError *error) {
                                NSLog(@"Error downloading record %@", shortDict);
                                dispatch_group_leave(downloadGroup);
                            }];

                            //** NOTIFY FINISH DOWNLOADING **
                            dispatch_group_notify(downloadGroup, dispatch_get_main_queue(), ^{
                                NSLog(@"Finished downloading all the records");

                                //This is CRASHING because this block is being called as many times as the dispatch_group_leave(downloadGroup) is called
                                dispatch_group_leave(externalGroup);
                            });
                        }
                } else {
                    NSLog(@"Error getting the records for Agent %@, with Code %li", @"AGENT ID", (long)completeCode);
                }
            } onError:^(AFHTTPRequestOperation *operation, NSError *error) {
                NSLog(@"Error getting the records for Agent %@,  %@", @"AGENT ID", operation.response);
            }];
        });
    }
}

I use dispatch groups in other parts with a normal behaviour (create, enter, enter, leave, leave, notify) so I don't understand what is going on here. Is it something related to nesting the blocks? Any advice on how to get the dispatch_group_notify being called just once upon completion or, even better, how to achieve this nested async tasks completion dependency in a cleaner way (meaning how to wait for multiple requests to finish, then fire some more and wait again and so on)?

Answer 1

You're notifying the downloadGroup group every time you enter it

for (NSDictionary* shortDict in responseArray) {
    dispatch_group_enter(downloadGroup);

    //download full records from BE
    [RESTClient getRecordByCodeAndTimestamp: shortDict onComplete:^(NSInteger completeCode, NSDictionary *responseDictionary) {
        NSLog(@"Success Downloading record");
        dispatch_group_leave(downloadGroup);
    } onError:^(AFHTTPRequestOperation *operation, NSError *error) {
        NSLog(@"Error downloading record %@", shortDict);
        dispatch_group_leave(downloadGroup);
    }];

    // BUG IS HERE
    dispatch_group_notify(downloadGroup, dispatch_get_main_queue(), ^{
        NSLog(@"Finished downloading all the records");

        dispatch_group_leave(externalGroup);
    });
}

// dispatch_group_notify should be moved HERE

you should only be notifying the group once, move dispatch_group_notify out of the loop.