CODEWITHSUNDEEP
iosobjective-cobjective-c-blocks
Scott Berrevoets
Scott Berrevoets

Reputation: 16946

Recursive block gets deallocated too early

I have written a recursive block following these guidelines:

NSMutableArray *groups = [NSMutableArray arrayWithArray:@[@"group1", @"group2", @"group3", @"group4"];

__block CommunicationCompletionHandler completion = [^{
    [groups removeObjectAtIndex:0];

    if ([groups count] > 0) {
        // This will send some information to the network, and calls the completion handler when it receives a response
        [mySocket saveGroup:groups[0] completion:completion];
    }
} copy]; // Removing copy here doesn't work either

[mySocket saveGroup:groups[0] completion:completion];

In the saveGroup:completion: method, I add the completion handler to an array:

self.completionHandlers[SaveGroupCompletionHandlerKey] = [completion copy];

And when I receive a response, I call the following method (key is in this case SaveGroupCompletionHandlerKey):

- (void)performCompletionHandlerForKey:(NSString *)key {
    if (self.completionHandlers[key]) {
        ((CommunicationCompletionHandler)self.completionHandlers[key])();
        [self.completionHandlers removeObjectForKey:key];
    }
}

The problem is that the completion handler only gets called once. The removeObjectForKey: line makes the block deallocate. If I uncomment that line, everything works fine. I'm not sure how the array has the last reference to this block, since I add a copy (which I believe is being optimized to a retain).

For clarity, the flow of the app is:

Anybody here who can point out what I'm doing wrong?

Upvotes: 2

Views: 1445

Answers (2)

Darren
Darren

Reputation: 25619

In -performCompletionHandlerForKey: you remove the completion handler from your dictionary after executing the block, which means that the handler will always be removed from the dictionary after one run.

Instead, store the block in a temporary variable and remove it from the dictionary before executing the block.

By the way, the advice to remove the weak reference is wrong. As your code is written now, your block will never be deallocated. The typical block recursion pattern is this:

__weak __block MyBlock weakHandler;
MyBlock handler = ^ { 
    if (foo) {
        MyBlock strongHandler = weakHandler;
        [bar asyncOperationWithCompletion:strongHandler];
    }
};

weakHandler = handler;
[bar asyncOperationWithCompletion:handler];

Upvotes: 3

Brendon
Brendon

Reputation: 882

A popular way to avoid retain retain cycles is to create a weak reference to the object before defining the block, then create a strong reference inside the block and set it to that weak reference. This method is frequently used to avoid strongly capturing self inside of blocks:

- (void)someMethod {
    __weak MyType *weakSelf = self;
    [self someMethodWithABlockArg:^{
        MyType *strongSelf = weakSelf;
        [strongSelf someOtherMethod];
    }];
}

The strong reference created inside the block prevents the object from being deallocated while the block is running. You can, of course, do the same with any object type.

Edit2: Looks like [someBlock copy] is indeed fine. Have you tried running Analyze on the code? It may be that completion is not yet initialized when it is referred to inside of the block.

Upvotes: -1

Related Questions