How to prevent dispatch group to not crash?

Question

I am using the below code to wait for async tasks to be completed. It works a couple of times and crashes. The updateFromTable always invokes callback() so that the group calls are balanced, but it still crashes.

- (void)updateFromTable:(Table *)table env:(Env *)env callback:(void (^)(void))callback {
    [someasync usingBlock:^{
        callback()
    }];
}

- (NSString * _Nullable)process {
    JSL * __weak weakSelf = self;
    NSString __block *ret = nil;
    dispatch_group_enter(_dispatchGroup);
    dispatch_async(_repQueue, ^{
        JSL *this = weakSelf;
        [this updateFromTable:[this->_env table] env:this->_env callback:^{
            ret = [some op .. ];
            dispatch_group_leave(this->_dispatchGroup);
        }];
    });
    dispatch_group_wait(_dispatchGroup, dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_SEC));
    info(@"%@", @"done");
    return ret;
}

Any idea why it crashes randomly and how to fix this? Basically, what I am trying to achieve is invoke couple of async tasks, wait for all of them to complete and then proceed with the rest.

Referring: How to wait past dispatch_async before proceeding?

It would be better if you didn't attempt to convert an async call into a synchronous call. Change `process` to take a completion handler instead of directly returning a value. — rmaddy, May 26 '19 at 15:55
This is an interactive command line app, so the user waits for the result. — John Doe, May 26 '19 at 15:59
That doesn't change the fact that you should properly work with async calls. — rmaddy, May 26 '19 at 16:01
In synchronous mode, the main function takes a string calls process, which executes a bunch of function and returns. If process is async, it returns and result is not available to the user. — John Doe, May 26 '19 at 16:04

Rob · Accepted Answer · 2019-05-26T17:01:03.420

You cannot dereference ivars with -> if this is nil. So, the typical solution is to create strong reference that can’t be deallocated while the closure runs, and return if it’s nil:

- (NSString * _Nullable)process {
    typeof(self) __weak weakSelf = self;
    [self asynchronousMethodWithCompletion:^{
        typeof(self) strongSelf = weakSelf;
        if (!strongSelf) { return; }

        // can now safely use `strongSelf` here
    });

    ...
}

This is “weakSelf-strongSelf dance”. You use it in situations where you need to make sure that self isn’t nil when you use it, e.g. dereferencing ivars (strongSelf->ivar) .

Thus:

- (NSString * _Nullable)process {
    typeof(self) __weak weakSelf = self;
    NSString __block *ret = nil;
    dispatch_group_t group = dispatch_group_create();
    dispatch_group_enter(group);
    dispatch_async(_repQueue, ^{
        typeof(self) strongSelf = weakSelf;
        if (!strongSelf) { return; }

        [strongSelf updateFromTable:[strongSelf->_env table] env:strongSelf->_env callback:^{
            ret = [some op .. ];
            dispatch_group_leave(group);
        }];
    });
    dispatch_group_wait(group, DISPATCH_TIME_FOREVER);
    info(@"%@", @"done");
    return ret;
}

A few other observations:

The dispatch group should be a local variable of the method rather than an ivar. There’s no need for anything else in your code referencing this group.
Make sure that your dispatch_group_leave calls don’t exceed the number of dispatch_group_enter calls (i.e. that this completion handler block isn’t called multiple times).
I’d suggest waiting for DISPATCH_TIME_FOREVER (assuming you want it to really wait for it to finish).
Also, if these are properties (which I’m guessing they are on the basis of the underscores), then using self.env rather than self->_env is safer, as it won’t crash if self is nil, but rather will just return nil.

I must confess that this still doesn’t look right (e.g. if updateFromTable is asynchronous already, why bother dispatching this asynchronously to _repQueue; if it is synchronous, then again, why dispatch this asynchronously only to wait for it). But it’s impossible to comment further without seeing the updateFromTable implementation.

Or, better, make the method asynchronous:

- (void)processWithCompletion:(void (^)(NSString *))callback {
    typeof(self) __weak weakSelf = self;
    dispatch_async(_repQueue, ^{
        typeof(self) strongSelf = weakSelf;
        if (!strongSelf) { return; }

        [strongSelf updateFromTable:[strongSelf->_env table] env:strongSelf->_env callback:^{
            NSString *ret = [some op .. ];
            callback(ret);
        }];
    });
}

Looks like the crash due to async dispatch is fixed by using local dispatch group var and checking callback count. — John Doe, May 26 '19 at 16:19

How to prevent dispatch group to not crash?

1 Answers1