15

So I'm using recursive blocks. I understand that for a block to be recursive it needs to be preceded by the __block keyword, and it must be copied so it can be put on the heap. However, when I do this, it is showing up as a leak in Instruments. Does anybody know why or how I can get around it?

Please note in the code below I've got references to a lot of other blocks, but none of them are recursive.

__block NSDecimalNumber *(^ProcessElementStack)(LinkedList *, NSString *) = [^NSDecimalNumber *(LinkedList *cformula, NSString *function){
        LinkedList *list = [[LinkedList alloc] init];
        NSDictionary *dict;
        FormulaType type;
        while (cformula.count > 0) {
            dict = cformula.pop;
            type = [[dict objectForKey:@"type"] intValue];
            if (type == formulaOperandOpenParen || type == formulaListOperand || type == formulaOpenParen) [list add:ProcessElementStack(cformula, [dict objectForKey:@"name"])];
            else if (type == formulaField || type == formulaConstant) [list add:NumberForDict(dict)];
            else if (type == formulaOperand) [list add:[dict objectForKey:@"name"]];
            else if (type == formulaCloseParen) {
                if (function){
                    if ([function isEqualToString:@"AVG("]) return Average(list);
                    if ([function isEqualToString:@"MIN("]) return Minimum(list);
                    if ([function isEqualToString:@"MAX("]) return Maximum(list);
                    if ([function isEqualToString:@"SQRT("]) return SquareRoot(list);
                    if ([function isEqualToString:@"ABS("]) return EvaluateStack(list).absoluteValue;
                    return EvaluateStack(list);
                } else break;
            }
        }
        return EvaluateStack(list);
    } copy];
    NSDecimalNumber *number = ProcessElementStack([formula copy], nil); 

UPDATE So in my own research I've discovered that the problem apparently does have to do with the references to the other blocks this block uses. If I do something simple like this, it doesn't leak:

 __block void (^LeakingBlock)(int) = [^(int i){
        i++;
        if (i < 100) LeakingBlock(i);
    } copy];
    LeakingBlock(1);

However, if I add a another block in this, it does leak:

void (^Log)(int) = ^(int i){
   NSLog(@"log sub %i", i);
};

__block void (^LeakingBlock)(int) = [^(int i){
    Log(i);
    i++;
    if (i < 100) LeakingBlock(i);
} copy];
LeakingBlock(1);

I've tried using the __block keyword for Log() and also tried copying it, but it still leaks. Any ideas?

UPDATE 2 I found a way to prevent the leak, but it's a bit onerous. If I convert the passed in block to a weak id, and then cast the weak id back into a the block type, I can prevent the leak.

void (^Log)(int) = ^(int i){
    NSLog(@"log sub %i", i);
};

__weak id WeakLogID = Log;

__block void (^LeakingBlock)(int) = [^(int i){
    void (^WeakLog)(int) = WeakLogID;
    WeakLog(i);
    if (i < 100) LeakingBlock(++i);
} copy];
LeakingBlock(1);

Surely there's a better way?

Aaron Hayman
  • 8,492
  • 2
  • 36
  • 63
  • Thanks for sharing your research, I haven't heard about having to also copy the block. However, it appears that a more recent LLVM emits a warning at the recursive call "capturing LeakingBlock' strongly in this block is likely to lead to a retain cycle". The only way I found to appease the compiler is to use a separate weak ptr to the block somewhat similar to your answer below, though its unwieldy enough that I'm tempted to locally override the warning instead. I'd be interested in seeing your take once you try the latest compiler. – Pierre Houston Jan 11 '13 at 19:03
  • @smallduck Originally, I used `copy` because it causes the block to be copied over to the heap from the stack. For a while that worked just fine and the I also got the compiler "recursive" error. I removed `copy` from my code (as reflected in my answer) and it worked (whereas previously I'd get `EXC_BAD_ACCESS`. I'm guessing that Apple altered the `__block` keyword to create blocks on the heap rather than on the stack...but that's just a guess. – Aaron Hayman Jan 13 '13 at 15:01
  • @smallduck Truthfully, I've given up using blocks for recursion. Yes, it can be done but it's a bit unwieldily and there's too many pitfalls. It's too easy to end up with retain cycles (which can be really bad with recursion) and it becomes difficult to read. So usually I just stick with methods/functions to do recursion. – Aaron Hayman Jan 13 '13 at 15:06

3 Answers3

11

Ok, I found the answer on my own...but thanks to those who tried to help.

If you're referencing/using other blocks in a recursive block, you must pass them in as weak variables. Of course, __weak only applies to block pointer types, so you must typedef them first. Here's the final solution:

    typedef void (^IntBlock)(int);

    IntBlock __weak Log = ^(int i){
        NSLog(@"log sub %i", i);
    };

    __block void (^LeakingBlock)(int) = ^(int i){
        Log(i);
        if (i < 100) LeakingBlock(++i);
    };
    LeakingBlock(1);

The above code doesn't leak.

Aaron Hayman
  • 8,492
  • 2
  • 36
  • 63
  • 1
    You don't need the typedef if you don't want it: `void(^__weak log)(int) = ^(int i){...};` – Matt Wilding Jan 29 '13 at 22:13
  • 2
    @MattWilding Good catch. I think that didn't used to work in previous versions of Xcode. The compiler seems to be constantly changing in regards to Blocks. I just downloaded Xcode 4.6 and it now complains that the above code "may" lead to a retain cycle. I think Apple is still figuring out the whole "Block" thing. – Aaron Hayman Jan 30 '13 at 20:16
  • 1
    I do seem to change my block code with each compiler revision. I had the same warning with 4.6, and you can fix it by marking `LeakingBlock` with the `__weak` modifer as well. – Matt Wilding Jan 30 '13 at 20:27
0

Aaron,

As your code appears to be single threaded, why are you copying the block? If you don't copy the block, you don't have a leak.

Andrew

adonoho
  • 4,339
  • 1
  • 18
  • 22
  • Yes you're correct, the code is single threaded. I'm not sure I understand the point of that declaration though. If I don't copy the block I get EXC_BAC_ACCESS when the block tries to access itself recursively. – Aaron Hayman Jan 17 '12 at 12:57
  • 1
    Actually, I should clarify: Without copying the block, I get EXC_BAD_ACCESS on assignment, not when the block tries to access itself recursively (that occurs if I don't use __block). I'm a little unsure of the details, but I believe it's because the block is initially created as a const object on the stack, while the block referenced within itself is the very same const stack block. This is fixed by copying the block to the heap first, then assigning it to the to the __block var so it can reference a copy of if itself rather than itself literal. – Aaron Hayman Jan 17 '12 at 14:00
-1

Without further context information, I can say this:

You are leaking that block because you are copying it and not releasing it elsewhere. You need to copy it to move it to the heap, that's ok. But the way you've chosen is not entirely ok.

A correct way to do it is to store it as some object instance variable, copy it, and then release it inside dealloc. At least, that's a way to do it without leaking.

Lio
  • 4,225
  • 4
  • 33
  • 40
  • 2
    I cannot release it manually. I'm using ARC (automatic reference counting) which prevents me from doing that. The scope of the block is the method, so using an iVar is code smell. The block should be released at the end of the method. – Aaron Hayman Jan 16 '12 at 12:44