Crash with dispatch_block

Question

I have been trying to understand the reason behind this crash for the sake of understanding more about how blocks behave. I have a really simple class to trigger this crash.

@implementation BlockCrashTest

- (void)doSomething
{
    dispatch_queue_t queue = dispatch_queue_create("com.queue.test", DISPATCH_QUEUE_SERIAL);

    __weak typeof(self) weakSelf = self;

    dispatch_block_t block = ^{

        __strong typeof(weakSelf) strongSelf = weakSelf;

        dispatch_group_t group = dispatch_group_create();

        dispatch_time_t time = dispatch_time(DISPATCH_TIME_NOW, 2 * NSEC_PER_SEC);

        dispatch_group_enter(group);
        [strongSelf performSomethingAsync:^{
            dispatch_group_leave(group);
        }];

        if(dispatch_group_wait(group, time) != 0) {
            NSLog(@"group already finished");
        }
    };
    dispatch_async(queue, block);
}

- (void)performSomethingAsync:(void(^)(void))completion
{
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        sleep(5);
        completion();
    });
}

- (void)dealloc
{
    NSLog(@"released object");
}

@end

Now, if I allocate the class and simply call method doSomething to it,

BlockCrashTest *someObject = [[BlockCrashTest alloc] init];
[someObject doSomething];

It crashes with the exception, EXC_BAD_INSTRUCTION and following stack traces,

#0  0x000000011201119a in _dispatch_semaphore_dispose ()
#1  0x0000000112013076 in _dispatch_dispose ()
#2  0x0000000112026172 in -[OS_dispatch_object _xref_dispose] ()
#3  0x000000010ef4c2fd in __29-[BlockCrashTest doSomething]_block_invoke at /Users/Sandeep/Desktop/Test Block Crash/Test Block Crash/ViewController.m:35
#4  0x0000000112005ef9 in _dispatch_call_block_and_release ()

If I modify the method doSomething, such that it do not use weak but uses self then the crash do not occur and the methods seem to execute as expected,

- (void)doSomething
{
    dispatch_queue_t queue = dispatch_queue_create("com.queue.test", DISPATCH_QUEUE_SERIAL);

    dispatch_block_t block = ^{

        dispatch_group_t group = dispatch_group_create();

        dispatch_time_t time = dispatch_time(DISPATCH_TIME_NOW, 2 * NSEC_PER_SEC);

        dispatch_group_enter(group);
        [self performSomethingAsync:^{
            dispatch_group_leave(group);

        }];

        if(dispatch_group_wait(group, time) != 0) {
            NSLog(@"group already finished");
        }
    };
    dispatch_async(queue, block);
}

Why does it crash, my understanding is that using weak inside the block would make sure that the method would not be called, if the object is released and I thought that weak is safer than using self inside the block.

The above code works fine with weakSelf, if I retain the object BlockCrashTest and call the method to it.

I would be really happy if someone could explain the reason behind the crash and what exactly happens with these 3 different variants of code above that one crash and other seem to work fine.

Note: This is in one way or other related to the crash listed in thread, Objective-C crash on __destroy_helper_block_. I have been able to reproduce the exact same stack traces with my code above.

Bluebill

Answer 1

A couple of observations:

1. You cannot have a dispatch group with unbalanced "enter" and "leave" when the dispatch_group_t object is deallocated. And as ilya pointed out, because of your pattern, strongSelf is nil, so you're entering the group, but not leaving it.

   A very common pattern in the weakSelf and strongSelf dance is to just check to see if strongSelf was nil or not, resolving the imbalance. Thus, if strongSelf is nil, it bypasses the dispatch group stuff altogether, but if it is not nil, both "enter" and "leave" will be called:

   - (void)doSomething {
       dispatch_queue_t queue = dispatch_queue_create("com.queue.test", DISPATCH_QUEUE_SERIAL);
   
       typeof(self) weakSelf = self;
   
       dispatch_async(queue, ^{
           typeof(self) strongSelf = weakSelf;
   
           if (strongSelf) {
               dispatch_group_t group = dispatch_group_create();
   
               dispatch_group_enter(group);
               [strongSelf performSomethingAsync:^{
                   dispatch_group_leave(group);
               }];
   
               dispatch_group_wait(group, DISPATCH_TIME_FOREVER);
           }
       });
   }
   

   Clearly, you must make sure that performSomethingAsync method, itself, always calls the block which leaves the group.

2. The other way of solving this (if you don't have assurances that all of the "enter" and "leave" will be balanced), is to use semaphores:

   - (void)doSomething {
       dispatch_queue_t queue = dispatch_queue_create("com.queue.test", DISPATCH_QUEUE_SERIAL);
   
       typeof(self) weakSelf = self;
   
       dispatch_async(queue, ^{
           typeof(self) strongSelf = weakSelf;
   
           dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
   
           [strongSelf performSomethingAsync:^{
               dispatch_semaphore_signal(semaphore);
           }];
   
           dispatch_time_t time = dispatch_time(DISPATCH_TIME_NOW, 2 * NSEC_PER_SEC);
   
           if (dispatch_semaphore_wait(semaphore, time) != 0) {
               NSLog(@"semaphore not received in time");
           }
       });
   }
   

   Frankly, even when using semaphores, like I have above, I still think one would want to check to confirm that strongSelf was not nil. Concurrent programming is confusing enough without adding the possibility of the message to a nil object resulting in an no-op.

   - (void)doSomething {
       dispatch_queue_t queue = dispatch_queue_create("com.queue.test", DISPATCH_QUEUE_SERIAL);
   
       typeof(self) weakSelf = self;
   
       dispatch_async(queue, ^{
           typeof(self) strongSelf = weakSelf;
   
           if (strongSelf) {
               dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
   
               [strongSelf performSomethingAsync:^{
                   dispatch_semaphore_signal(semaphore);
               }];
   
               dispatch_time_t time = dispatch_time(DISPATCH_TIME_NOW, 2 * NSEC_PER_SEC);
   
               if (dispatch_semaphore_wait(semaphore, time) != 0) {
                   NSLog(@"semaphore not received in time");
               }
           }
       });
   }
   

Matta

Can you also give me a clue on why it does not crash, if I use self inside the block ? Is it only because that the block retains self ? – Bluebill

@GeneratorOfOne - Because the presence of self establishes strong reference inside the block, which isn't resolved until the block is done executing. Thus weakSelf is not nil and strongSelf is not nil either. Thus, your completion handler block in performSomethingAsync is getting called, ensuring that the "enter" is now balanced by a "leave". But if you wanted the block to retain the object, you obviously wouldn't have gone through the weakSelf/strongSelf dance. – Matta

This is good explanation. So, I can have some autoreleased object which can still do some long asynchronous call and still be around if I used self, instead of using weak / strong. While if the object is if retained itself, then it makes sense to have weak / strong, is that gist of this thing ? – Bluebill

If you need the object retained while the block executes (e.g. self refers to a database controller object or something like that, where you really want to do something with the result), then use self (as long as you don't have any persistent strong reference cycle). If you don't need the object retained while the block executes (e.g. a view controller for a view that may have long since been dismissed ... no point in updating), do weakSelf/strongSelf dance. And if you want to cancel the task when self is deallocated, then even different approach is needed. – Matta

Answer 2

You'll get the same crash even if you remove call of self performSomethingAsync:. This crash caused by libdispatch semaphore API. You can see assembly trace of crashed function _dispatch_semaphore_dispose in Xcode:

If we try to figure out what happens in this code we'll see that you explicitly mark current block entered the group by calling dispatch_group_enter. Then performSomethingAsync doesn't call because strongSelf == nil. Which means that dispatch_group_enter is not balanced with dispatch_group_leave this cause the group couldn't dispose properly and crashed (see asm listing).

If you use self this code also crashed because dispatch_group_leave(group); called from the different thread with dispatch_group_enter Which is also cause the same crash but in another perspective: calls not balanced in the same thread. performSomethingAsync called completion block in different queue not in yours "com.queue.test".

This example is just wrong using of dispatch_groups APIs. To see how properly use it see apple doc.

Aftershaft

Answer 3

MacOS 10.8 and iOS 6.0 introduced ARC to dispatch objects. From the documentation GCD Objects and Automatic Reference Counting:

When you build your app using the Objective-C compiler, all dispatch objects are Objective-C objects. As such, when automatic reference counting (ARC) is enabled, dispatch objects are retained and released automatically just like any other Objective-C object. When ARC is not enabled, use the dispatch_retain and dispatch_release functions (or Objective-C semantics) to retain and release your dispatch objects. You cannot use the Core Foundation retain/release functions.

If you need to use retain/release semantics in an ARC-enabled app with a later deployment target (for maintaining compatibility with existing code), you can disable Objective-C-based dispatch objects by adding -DOS_OBJECT_USE_OBJC=0 to your compiler flags.

In your case ARC is happily managing the life cycle of your dispatch_group_t. And, unfortunately, your code is causing the group to be released while the lock is still waiting. When the group times out it is released - so when dispatch_group_leave is called it crashes, as the group has already been released.

I would suggest at the very least checking wether the group is NULL before attempting to leave it.

Additionally, your wait result logic is reversed. A zero result indicates the group was emptied before the timeout, a non zero result indicates the timeout was hit.

Metro