Objective-C block warning "save loop", do not understand why

Question

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

I am writing a category on UIView to recursively evaluate all UIView routines and return an array of subordinate objects passing the test. I noticed where the 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 bad_access crashing when I don't add the __block modifier in the block declaration (^__block evaluateAndRecurse) . If someone can explain why this is so, this is also very helpful. Thanks!

Answer 1

The problem is that your evaluteAndRecurse() block is capturing itself, which means if it will ever be copied (I don't believe it will be in your case, but in a few less trivial cases it may be), then it save himself and, therefore, will live forever, as there is nothing to break the cycle of retention.

Edit : Rami Al Zuhuri made a good point using __unsafe_unretained , the only link 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 (for example, it needs to go to the parent area), then __unsafe_unretained will free it. The following paragraph updates the recommended approach:

What you probably want to do here is use a separate variable, marked __unsafe_unretained , which also contains a block, and capture this separate variable. This will prevent its persistence. You can use __weak , but since you know that a block must be alive if called, there is no need to worry about (very minor) overhead of weak links. 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 can grab a pointer to a block that will have the same effect, but allow you to grab a pointer before creating the block, and not after it. This is a personal preference. It also allows you to omit __block :

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

---

Regarding the need for __block , this is a separate issue. If you don't have __block , then the block instance will actually capture the previous value of the variable. Remember that when a block is created, any captured variables that are not marked with __block are actually saved as a copy of their const state at the point where the block is instantiated. And since the block is created before it is assigned to a variable, this means that it captures the state of the variable capturedEvaluteAndRecurse until the destination, which will be nil (in ARC, otherwise it will be garbage memory).

In essence, you can imagine a specific instance of a block as being actually an instance of a hidden class that has ivar for each captured variable. Thus, with your code, the compiler will basically consider it 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; 

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

Answer 2

I did something similar, but reduced the time to allocate new arrays in a different way and had no problems. You can try to adapt your method to look something like this:

 - (void)addSubviewsOfKindOfClass:(id)classObject toArray:(NSMutableArray *)array { if ([self isKindOfClass:classObject]) { [array addObject:self]; } NSArray *subviews = [self subviews]; for (NSView *view in subviews) { [view addSubviewsOfKindOfClass:classObject toArray:array]; } }