3

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?

lucaconlaq
  • 1,511
  • 4
  • 19
  • 36
  • Compare: http://stackoverflow.com/a/15536473/1187415: `Sema::checkRetainCycles()` in the static analyzer warns if the methods name starts with "set" or "add", but *not* for "addAsynchronousOperationWithBlock". – Martin R May 09 '13 at 02:25
  • ... correction: The static analyzer warns if the methods name starts with "set" or "add", but *not* for "addOperationWithBlock". – Martin R May 09 '13 at 08:05
  • I assume that you meant "addOperationWithBlock" and not "addAsynchronousOperationWithBlock" in the last code example? – Martin R May 09 '13 at 08:20
  • I changed the question to make it clearer – lucaconlaq May 09 '13 at 08:37
  • _queue == self, so self retains block, block retains self == _queue ERGO a cycle – Daij-Djan May 13 '13 at 14:53

3 Answers3

9

When you use self within a block, it is captured by the block and could lead to a retain cycle. To cycle occurs when self (or something it has a strong reference to) has a strong reference to the block. To avoid the potential cycle, declare a weak pointer and use that in the block instead:

YourClassName * __weak weakSelf = self;

[_queue addAsynchronousOperationWithBlock:^(block signal) {
    [weakSelf foo:nil];
}];
jszumski
  • 7,430
  • 11
  • 40
  • 53
  • 2
    Specifically, if the block is owned by self, even indirectly. Otherwise it's fine. – Catfish_Man May 08 '13 at 21:54
  • Excellent point, I updated the answer to be more explicit. – jszumski May 08 '13 at 22:01
  • Ok but why don't I have the same problem with addOperationWithBlock? where is here the strong reference to the block? – lucaconlaq May 08 '13 at 22:17
  • 1
    But the code snippets both *do* capture self. The reason that the analyzer complains in one case but not in the other is that some method names are hard-coded into the analyzer, see my comment to the question above. – Martin R May 09 '13 at 02:49
  • 1
    Note also that capturing self is not generally bad - it can be used to keep the object alive until the block is called, compare http://stackoverflow.com/questions/12218964/keep-object-alive-until-a-background-task-finishes/12220731#12220731. – Martin R May 09 '13 at 03:08
  • thanks for the answer, there is something I still don't get and I thought editing the question would be the more appropriate way to express my doubts. – lucaconlaq May 09 '13 at 13:25
4

The answer from jszumski is correct in essence, but it is important to get the form of the "weak self" dance correct. The form (building on his code) is:

YourClassName * __weak weakSelf = self;

[_queue addAsynchronousOperationWithBlock:^(block signal) {
    YourClassName * strongSelf = weakSelf;
    if (strongSelf)
         [weakSelf foo:nil];
}];

Thus we capture weakSelf through a strong reference. If you don't do that, weakSelf can go out of existence while you are in the middle of using it (because your reference to it is weak).

See my book for the dance, and for other things you can do about potential retain cycles caused by blocks:

http://www.apeth.com/iOSBook/ch12.html#_unusual_memory_management_situations

matt
  • 515,959
  • 87
  • 875
  • 1,141
  • 2
    I would also always use this pattern, but actually the object can *not* be deallocated if you just use `[weakSelf foo:nil];`, compare http://stackoverflow.com/a/15267291/1187415: Accessing `weakSelf` in the block is done via `objc_loadWeak()`, which retains and autoreleases the object. – Martin R May 09 '13 at 02:47
  • thanks for the answer, and also for the link. I'm learning a lot reading it. I still have some doubts about my problem and I edited the questions to express them – lucaconlaq May 09 '13 at 13:28
4

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.

Jim Matthews
  • 1,181
  • 8
  • 16