Objective-C block "retain cycle" warning, don't understand why

Question

I've seen several other questions of the same form, but I either a) can't understand the provided answers, or b) don't see how those situations are similar to mine.

I'm writing a Category on UIView to recursively evaluate all the subviews of a UIView and return an Array of subviews passing a test. I've noted where my compiler warning occurs:

-(NSArray*)subviewsPassingTest:(BOOL(^)(UIView *view, BOOL *stop))test {

   __block BOOL *stop = NO;

    NSArray*(^__block evaluateAndRecurse)(UIView*);
    evaluateAndRecurse = ^NSArray*(UIView *view) {
        NSMutableArray *myPassedChildren = [[NSMutableArray alloc] init];
        for (UIView *subview in [view subviews]) {
            BOOL passes = test(subview, stop);
            if (passes) [myPassedChildren addObject:subview];
            if (stop) return myPassedChildren;


            [myPassedChildren addObjectsFromArray:evaluateAndRecurse(subview)];
            // ^^^^ Compiler warning here ^^^^^
            // "Capturing 'evaluateAndRecurse' strongly in this block 
            // is likely to lead to a retrain cycle"
        }
        return myPassedChildren;
    };

    return evaluateAndRecurse(self);
}

Also, I get a bad_access failure when I don't include the __block modifier in my block's declaration (^__block evaluateAndRecurse). If someone could explain why that is, that would be very helpful too. Thanks!

Answer 1

The problem here is that your block evaluteAndRecurse() captures itself, which means that, if it's ever to be copied (I don't believe it will in your case, but in slightly less-trivial cases it may), then it will retain itself and therefore live forever, as there is nothing to break the retain cycle.

Edit: Ramy Al Zuhouri made a good point, using __unsafe_unretained on the only reference to the block is dangerous. As long as the block remains on the stack, this will work, but if the block needs to be copied (e.g. it needs to escape to a parent scope), then the __unsafe_unretained will cause it to be deallocated. The following paragraph has been updated with the recommended approach:

What you probably want to do here is use a separate variable marked with __unsafe_unretained that also contains the block, and capture that separate variable. This will prevent it from retaining itself. You could use __weak, but since you know that the block must be alive if it's being called, there's no need to bother with the (very slight) overhead of a weak reference. This will make your code look like

NSArray*(^__block __unsafe_unretained capturedEvaluteAndRecurse)(UIView*);
NSArray*(^evaluateAndRecurse)(UIView*) = ^NSArray*(UIView *view) {
    ...
        [myPassedChildren addObjectsFromArray:capturedEvaluateAndRecurse(subview)];
};
capturedEvaluateAndRecurse = evaluteAndRecurse;

Alternatively, you could capture a pointer to the block, which will have the same effect but allow you to grab the pointer before the block instantiation instead of after. This is a personal preference. It also allows you to omit the __block:

NSArray*(^evaluateAndRecurse)(UIView*);
NSArray*(^*evaluteAndRecursePtr)(UIView*) = &evaluateAndRecurse;
evaluateAndRecurse = ^NSArray*(UIView*) {
    ...
        [myPassedChildren addObjectsFromArray:(*evaluateAndRecursePtr)(subview)];
};

---

As for needing the __block, that's a separate issue. If you don't have __block, then the block instance will actually capture the previous value of the variable. Remember, when a block is created, any captured variables that aren't marked with __block are actually stored as a const copy of their state at the point where the block is instantiated. And since the block is created before it's assigned to the variable, that means it's capturing the state of the capturedEvaluteAndRecurse variable before the assignment, which is going to be nil (under ARC; otherwise, it would be garbage memory).

In essence, you can think of a given block instance as actually being an instance of a hidden class that has an ivar for each captured variable. So with your code, the compiler would basically treat it as something like:

// Note: this isn't an accurate portrayal of what actually happens
PrivateBlockSubclass *block = ^NSArray*(UIView *view){ ... };
block->stop = stop;
block->evaluteAndRecurse = evaluateAndRecurse;
evaluteAndRecurse = block;

Hopefully this makes it clear why it captures the previous value of evaluateAndRecurse instead of the current value.