Search code examples
iosobjective-cobjective-c-blocksnsoperationqueue

block is likely to lead a retain cycle


I've written the following category for NSOperationBlock

@implementation NSOperationQueue (Extensions)

-(void)addAsynchronousOperationWithBlock:(void (^)(block))operationBlock
{
    dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);

    block signal = ^ {
        dispatch_semaphore_signal(semaphore);
    };

    [self addOperationWithBlock:^{
        operationBlock(signal);
        dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
        dispatch_release(semaphore);
    }];
}

@end

it seems to work properly but when I call it (as shown in the following snippet) I get a warning:

block is likely to lead a retain cycle

[_queue addAsynchronousOperationWithBlock:^(block signal) {
        [self foo:nil];
         signal();
}];

foo is a method of the class that uses this category.

The same code with addOperationWithBlock: (from NSOperationQueue) doesn't show the warning:

[_queue addOperationWithBlock:^ {
        [self foo:nil];
}];

I really don't understand it. Particularly what I don't understand is: should I actually use the weak pointer in both the cases? will actually the two snippet bring to a retain cycle in case I don't use the weak pointer?


Solution

  • To distill what others have written here:

    1. Neither code example creates a long-lasting retain cycle of the sort that will strand memory.
    2. Xcode complains about your addAsynchronousOperationWithBlock method because it has a suspicious name. It doesn't complain about addOperationWithBlock because it has special knowledge about addOperationWithBlock that overrides its suspicions.
    3. To get rid of the warning, use __weak (see the answers by jszumski and matt) or rename addAsynchronousOperationWithBlock to not start with "add" or "set".

    To elaborate a bit on these:

    1. If self owns _queue, you will have a short-lived retain cycle. self will own _queue, which will own the blocks, and the block that calls [self foo:] will own self. But once the blocks have finished running, _queue will release them, and the cycle will be broken.

    2. The static analyzer has been programmed to be suspicious of method names starting with "set" and "add". Those names suggest that the method may retain the passed block permanently, possibly creating a permanent retain cycle. Thus the warning about your method. It doesn't complain about -[NSOperationQueue addOperationWithBlock:] because it's been told not to, by someone who knows that NSOperationQueue releases blocks after running them.

    3. If you use __weak the analyzer won't complain because there won't be the possibility of a retain cycle. If you rename your method the analyzer won't complain because it won't have any reason to suspect your method of permanently retaining the block passed to it.