macOS blocks and retain cycles

[North Bronson]

If any of you do a lot of work with blocks and completion handlers, maybe you can help with what I am trying to do here.

I am trying to switch around some classes to use blocks instead of delegate callbacks. I have a NBSURLConnection class that downloads a resource from the network. Instead of setting a delegate, the NBSURLConnection is created with a completion handler. This completion handler is called when the connection is finished or fails.

The NBSModel that creates the NBSURLConnection passes in a block that the NBSURLConnection will need to copy to the heap so that it can call it later. If the block that is passed references an instance variable of the NBSModel, the NBSModel will be retained when the block is copied:

NBSModel retains NBSURLConnection
NBSURLConnection retains block
block retains NBSModel

What is the best way to avoid this problem?

typedef void (^NBSURLConnectionCompletionHandler)(NSData *data, NSURLResponse *response, NSError *error);

@interface NBSURLConnection : NSObject
{
@private
    NBSURLConnectionCompletionHandler myCompletionHandler;
    
    NSMutableData *myData;
    NSURLConnection *myConnection;
    NSURLResponse *myResponse;
}

#pragma mark -

- (void)cancel;
- (id)initWithRequest:(NSURLRequest *)request withCompletionHandler:(NBSURLConnectionCompletionHandler)completionHandler;

@end

#import "NBSURLConnection.h"

@implementation NBSURLConnection

#pragma mark -
#pragma mark NSObject
#pragma mark -

- (void)dealloc
{
    Block_release(myCompletionHandler);
    
    [myData release];
    [myConnection release];
    [myResponse release];
    
    [super dealloc];
}

#pragma mark -
#pragma mark NBSURLConnection
#pragma mark -

- (void)cancel
{
    [myConnection cancel];
}



- (id)initWithRequest:(NSURLRequest *)request withCompletionHandler:(NBSURLConnectionCompletionHandler)completionHandler
{
    self = [self init];
    
    if (self)
    {
        myCompletionHandler = Block_copy(completionHandler);
        
        myConnection = [[NSURLConnection alloc] initWithRequest: request delegate: self];
    }
    
    return self;
}

#pragma mark -
#pragma mark NSURLConnectionDelegate
#pragma mark -

- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
{
    myCompletionHandler(nil, myResponse, error);
}



- (void)connectionDidFinishLoading:(NSURLConnection *)connection
{
    myCompletionHandler(myData, myResponse, nil);
}



- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data
{
    if (myData == nil)
    {
        myData = [[NSMutableData alloc] init];
    }
    
    [myData appendData: data];
}



- (void)connection:(NSURLConnection *)connection didReceiveResponse:(NSURLResponse *)response
{
    if (myResponse != response)
    {
        [myResponse release];
        
        myResponse = [response copy];
    }
    
    [myData setLength: 0];
}

@end

@class NBSURLConnection;

@interface NBSModel : NSObject
{
@private
    NBSURLConnection *myConnection;
    NSData *myData;
}

#pragma mark -

- (id)initWithURL:(NSURL *)url;

@end

#import "NBSModel.h"
#import "NBSURLConnection.h"

@implementation NBSModel

#pragma mark -
#pragma mark NSObject
#pragma mark -

- (void)dealloc
{
    [myConnection cancel];
    
    [myConnection release];
    [myData release];
    
    [super dealloc];
}

#pragma mark -
#pragma mark NBSModel
#pragma mark -

- (id)initWithURL:(NSURL *)url
{
    self = [self init];
    
    if (self)
    {
        NSURLRequest *newRequest = [[NSURLRequest alloc] initWithURL: url];
        
        if (newRequest)
        {
            myConnection = [[NBSURLConnection alloc] initWithRequest: newRequest withCompletionHandler: ^(NSData *data, NSURLResponse *response, NSError *error) {
                
                myData = [data copy];
                
            }];
            
            [newRequest release];
        }
    }
    
    return self;
}

@end

 

[jiminaus]

You'll break the cycle by sending release to myConnection and then setting myConnection to nil in the completion block.

When I did that (and added logging to everything), I get the following output:

2011-11-23 18:19:26.358 BlockTest[87293:407] -[MYAppDelegate applicationDidFinishLaunching:]
2011-11-23 18:19:26.359 BlockTest[87293:407] -[NBSModel initWithURL:]
2011-11-23 18:19:26.360 BlockTest[87293:407] -[NBSURLConnection initWithRequest:withCompletionHandler:]
2011-11-23 18:19:26.436 BlockTest[87293:407] -[NBSURLConnection connection:didReceiveResponse:]
2011-11-23 18:19:26.436 BlockTest[87293:407] -[NBSURLConnection connection:didReceiveData:]
2011-11-23 18:19:26.532 BlockTest[87293:407] -[NBSURLConnection connectionDidFinishLoading:]: start
2011-11-23 18:19:26.533 BlockTest[87293:407] __24-[NBSModel initWithURL:]_block_invoke_0: start
2011-11-23 18:19:26.533 BlockTest[87293:407] __24-[NBSModel initWithURL:]_block_invoke_0: end
2011-11-23 18:19:26.533 BlockTest[87293:407] -[NBSURLConnection connectionDidFinishLoading:]: end
2011-11-23 18:19:26.533 BlockTest[87293:407] -[NBSURLConnection dealloc]
2011-11-23 18:19:26.533 BlockTest[87293:407] -[NBSModel dealloc]

 

[North Bronson]

Thanks for looking through that. I could see this working, but I was looking for a slightly different approach. I could have an NBSController that is responsible for owning the NBSModel. If the controller needs to go away, it seems like it would be a clean approach to simply release the NBSModel and let that destroy the connection. If the NBSModel is waiting for the completion block to release the NBSURLConnection that is retaining the block that is retaining the NBSModel, simply releasing the NBSModel from the controller would not release the objects until the completion block was called.

If the NBSURLConnection uses a delegate (that is not retained) for when the connection is finished, all that I need to do in the NBSModel is to cancel the NBSURLConnection and release the NBSURLConnection and both objects go away. This means that my NBSController could just release its NBSModel and all the objects will just go away like they should. I was looking for something like this that I could use with completion handlers instead of delegates.

 

[jiminaus]

What about adding a cancel method to NBSModel like so

- (void)cancel
{
    [myConnection cancel];
    [myConnection release];
    myConnection = nil;
}

and then have you controller send cancel to its NBSModel object before it releases it?

 

[North Bronson]

That could be a good idea. I was thinking about something like that. The issue is that I was trying to incorporate blocks so that I could remove code from the version with delegates. If adding completion handlers means that I add extra API that wasn't there before, I would keep looking for an approach that would keep things clean.

I am also trying to avoid making the NBSController have to know too much about the inner-workings of the NBSModel. Ideally, I would like for the NBSController to just send one release message to the NBSModel and just have that clean things up.

 

[jiminaus]

I tried to look for a pattern in iOS to establish a precedence for this pattern. Of the many results found by searching for completionHandler, what I found was that completion handlers were being used only from synchronous methods.

Perhaps you've stumbled on the reason why.

 

[North Bronson]

So far, my ideas are to:

[1]
Make the completion handler an instance variable of the NBSModel. The NBSURLConnection does not copy the block. This seems like it should solve the problem with the NBSURLConnection retaining the NBSModel, but now it seems like the NBSModel will be retaining itself. It also does not seem like a very Cocoa-way of doing things.

[2]
Declare an __block variable to hold self before the completion block is declared and set the data with an accessor method instead of accessing the instance variable directly. This seems like it should work, but something doesn't feel right that I can't quite put my finger on.

Would ARC solve all of this?

 

[jiminaus]

[1]
Make the completion handler an instance variable of the NBSModel. The NBSURLConnection does not copy the block. This seems like it should solve the problem with the NBSURLConnection retaining the NBSModel, but now it seems like the NBSModel will be retaining itself. It also does not seem like a very Cocoa-way of doing things.

I'm very uncomfortable with this. The ownership feels wrong. But that's just a impulsive gut reaction.

[2]
Declare an __block variable to hold self before the completion block is declared and set the data with an accessor method instead of accessing the instance variable directly. This seems like it should work, but something doesn't feel right that I can't quite put my finger on.

There's a possibility of a dangling pointer to the NBSModel object with this approach. But it seems to work.

Would ARC solve all of this?

Generally, ARC doesn't resolve the issue of retain cycles. However if you do #2 and turn ARC on, you'll want to read the Use Lifetime Qualifiers to Avoid Strong Reference Cycles section of the Transitioning to ARC Release Notes because it talks about this very pattern.


BTW You don't need to use Block_copy and Block_release in Objective-C. They're for dealing with block in C. In Objective-C you can treat a block as an Objective-C object and sent it copy and release (even autorelease) messages just like other objects.

 

[North Bronson]

What about:

- (id)initWithURL:(NSURL *)url
{
    self = [self init];
    
    if (self)
    {
        NSURLRequest *newRequest = [[NSURLRequest alloc] initWithURL: url];
        
        if (newRequest)
        {
            NSData **reference = &myData;
            
            myConnection = [[NBSURLConnection alloc] initWithRequest: newRequest withCompletionHandler: ^(NSData *data, NSURLResponse *response, NSError *error) {
                
                *reference = [data copy];
                
            }];
            
            [newRequest release];
        }
    }
    
    return self;
}

Is something like this safe? It doesn't look like the reference is forcing the block to retain self. It looks like setting the data through the reference is working like it should. Would there be any problems with this approach? Is this just a hack?

 

[jiminaus]

I can't see it being any worse than assigning self to a __block local variable.

 

[North Bronson]

In a more arbitrary case, would something like:

__block NSData *reference = nil;

[self fetchData: ^(NSData *data) {
    
    reference = data;
    
}];

be functionally equivalent to something like:

NSData **reference = nil;

[self fetchData: ^(NSData *data) {
    
    *reference = data;
    
}];

or I am missing some subtle differences between the way that blocks handle the two?

 

[jiminaus]

In a more arbitrary case, would something like:

__block NSData *reference = nil;

[self fetchData: ^(NSData *data) {
    
    reference = data;
    
}];

be functionally equivalent to something like:

NSData **reference = nil;

[self fetchData: ^(NSData *data) {
    
    *reference = data;
    
}];

or I am missing some subtle differences between the way that blocks handle the two?

In the second example (assuming the assign of the double-pointer to nil is an error), you're getting a copy of reference being "passed" into the block because reference is not a __block scoped variable.

In the first example, the address to the __block local variable reference is "passed" into the block, so modification inside the block are visible outside of the block and modifications to the local variable after the block is created are visible inside the block.


For example:

int variable = 1;
void (^myBlock)() 
    = ^{
        NSLog(@"variable = %d", variable);
    };
variable = 2;
myBlock();

prints variable = 1 and not variable = 2 because variable is not __block scoped.

__block int variable = 1;
void (^myBlock)() 
    = ^{
        NSLog(@"variable = %d", variable);
    };
variable = 2;
myBlock();

prints variable = 2 because variable is __block scoped.


Going back to your examples, it's not so much __block verses not __block, but singular indirection verses double indirection. You could validly __block scope reference in the second example.


Um, have I answered anything or just prattled on?

 

[North Bronson]

In the second example (assuming the assign of the double-pointer to nil is an error), you're getting a copy of reference being "passed" into the block because reference is not a __block scoped variable.

Right. I would have needed to point the double-pointer to an instance variable or another object not just on the local stack.