Objective-c – iOS 5 Twitter Framework & completionHandler block – “Capturing ‘self’ strongly in this block is likely to lead to a retain cycle”

ios5ipadiphoneobjective cobjective-c-blocks

I am very new to programming and Objective-C and I am trying to work out what is wrong with my code. I have read a bit about blocks but I don't know how any of what I have read so far is relevant to my code.

My code is using the iOS 5 Twitter Framework. I use most of the sample code that Apple provides so I actually had no clue at first that I was using a block for the completion handler.

Now I get those two messages from Xcode 4 saying "1. Block will be retained by an object strongly retained by the captured object" and "Capturing 'self' strongly in this block is likely to lead to a retain cycle".

Basically, what I did is to remove the code Apple used in their completion handler (switch statement with TWTweetComposeViewControllerResultCancelled & TWTweetComposeViewControllerResultDone) and used my if statements with [imagePickerController sourceType].

So the sendTweet gets called after an image has been added to the tweet.

I hope someone can explain to me why this is happening and how I can solve it. Also: can I put the completion handler code into a method instead of a block?

- (void)sendTweet:(UIImage *)image
{
    //adds photo to tweet
    [tweetViewController addImage:image];

    // Create the completion handler block.
    //Xcode: "1. Block will be retained by an object strongly retained by the captured object"
    [tweetViewController setCompletionHandler:
                             ^(TWTweetComposeViewControllerResult result) {
            NSString *alertTitle,
                     *alertMessage,
                     *otherAlertButtonTitle,
                     *alertCancelButtonTitle;

            if (result == TWTweetComposeViewControllerResultCancelled) 
            {
                //Xcode: "Capturing 'self' strongly in this block is likely to lead to a retain cycle"
                if ([imagePickerController sourceType])
                {
                    alertTitle = NSLocalizedString(@"TCA_TITLE", nil);
                    alertMessage = NSLocalizedString(@"TCA_MESSAGE", nil);
                    alertCancelButtonTitle = NSLocalizedString(@"NO", nil);
                    otherAlertButtonTitle = NSLocalizedString(@"YES", nil);

                    //user taps YES
                    UIAlertView *alert = [[UIAlertView alloc] 
                                             initWithTitle:alertTitle 
                                                   message:alertMessage 
                                                  delegate:self   // Note: self
                                         cancelButtonTitle:alertCancelButtonTitle 
                                         otherButtonTitles:otherAlertButtonTitle,nil];
                    alert.tag = 1;
                    [alert show];                
                }            
            }

Best Answer

The basic problem is that you're using self within a block. The block is being retained by the object and the block itself retains the object, too. So you have a retain-cycle and thus both will probably never be released because both have a reference pointing towards them. Fortunaly there is a simple workaround:

By using a so-called weak reference to self the block will no longer retain the object. The object then can later be released which will release the block (set MyClass to the appropriate type):

// before your block
__weak MyObject *weakSelf = self;

Inside your block you now can use weakSelf instead of self. Note that this is only for iOS 5 using ARC.

Looking at this there is also a very good long explanation on How do I avoid capturing self in blocks when implementing an API? which also describes how to avoid this on iOS 4 and without ARC.

Short answer

Instead of accessing self directly, you should access it indirectly, from a reference that will not be retained. If you're not using Automatic Reference Counting (ARC), you can do this:

__block MyDataProcessor *dp = self;
self.progressBlock = ^(CGFloat percentComplete) {
    [dp.delegate myAPI:dp isProcessingWithProgress:percentComplete];
}

The __block keyword marks variables that can be modified inside the block (we're not doing that) but also they are not automatically retained when the block is retained (unless you are using ARC). If you do this, you must be sure that nothing else is going to try to execute the block after the MyDataProcessor instance is released. (Given the structure of your code, that shouldn't be a problem.) Read more about __block.

If you are using ARC, the semantics of __block changes and the reference will be retained, in which case you should declare it __weak instead.

Long answer

Let's say you had code like this:

self.progressBlock = ^(CGFloat percentComplete) {
    [self.delegate processingWithProgress:percentComplete];
}

The problem here is that self is retaining a reference to the block; meanwhile the block must retain a reference to self in order to fetch its delegate property and send the delegate a method. If everything else in your app releases its reference to this object, its retain count won't be zero (because the block is pointing to it) and the block isn't doing anything wrong (because the object is pointing to it) and so the pair of objects will leak into the heap, occupying memory but forever unreachable without a debugger. Tragic, really.

That case could be easily fixed by doing this instead:

id progressDelegate = self.delegate;
self.progressBlock = ^(CGFloat percentComplete) {
    [progressDelegate processingWithProgress:percentComplete];
}

In this code, self is retaining the block, the block is retaining the delegate, and there are no cycles (visible from here; the delegate may retain our object but that's out of our hands right now). This code won't risk a leak in the same way, because the value of the delegate property is captured when the block is created, instead of looked up when it executes. A side effect is that, if you change the delegate after this block is created, the block will still send update messages to the old delegate. Whether that is likely to happen or not depends on your application.

Even if you were cool with that behavior, you still can't use that trick in your case:

self.dataProcessor.progress = ^(CGFloat percentComplete) {
    [self.delegate myAPI:self isProcessingWithProgress:percentComplete];
};

Here you are passing self directly to the delegate in the method call, so you have to get it in there somewhere. If you have control over the definition of the block type, the best thing would be to pass the delegate into the block as a parameter:

self.dataProcessor.progress = ^(MyDataProcessor *dp, CGFloat percentComplete) {
    [dp.delegate myAPI:dp isProcessingWithProgress:percentComplete];
};

This solution avoids the retain cycle and always calls the current delegate.

If you can't change the block, you could deal with it. The reason a retain cycle is a warning, not an error, is that they don't necessarily spell doom for your application. If MyDataProcessor is able to release the blocks when the operation is complete, before its parent would try to release it, the cycle will be broken and everything will be cleaned up properly. If you could be sure of this, then the right thing to do would be to use a #pragma to suppress the warnings for that block of code. (Or use a per-file compiler flag. But don't disable the warning for the whole project.)

You could also look into using a similar trick above, declaring a reference weak or unretained and using that in the block. For example:

__weak MyDataProcessor *dp = self; // OK for iOS 5 only
__unsafe_unretained MyDataProcessor *dp = self; // OK for iOS 4.x and up
__block MyDataProcessor *dp = self; // OK if you aren't using ARC
self.progressBlock = ^(CGFloat percentComplete) {
    [dp.delegate myAPI:dp isProcessingWithProgress:percentComplete];
}

All three of the above will give you a reference without retaining the result, though they all behave a little bit differently: __weak will try to zero the reference when the object is released; __unsafe_unretained will leave you with an invalid pointer; __block will actually add another level of indirection and allow you to change the value of the reference from within the block (irrelevant in this case, since dp isn't used anywhere else).

What's best will depend on what code you are able to change and what you cannot. But hopefully this has given you some ideas on how to proceed.

Objective-c – capturing self strongly in this block is likely to lead to a retain cycle

The capture of self here is coming in with your implicit property access of self.timerDisp - you can't refer to self or properties on self from within a block that will be strongly retained by self.

You can get around this by creating a weak reference to self before accessing timerDisp inside your block:

__weak typeof(self) weakSelf = self;
[player addPeriodicTimeObserverForInterval:CMTimeMakeWithSeconds(0.1, 100)
                                     queue:nil
                                usingBlock:^(CMTime time) {
                                                current+=1;

                                                if(current==60)
                                                {
                                                    min+=(current/60);
                                                    current = 0;
                                                }

                                                 [weakSelf.timerDisp setText:[NSString stringWithFormat:@"%02d:%02d",min,current]];
                                            }];

Best Answer

Related Solutions

Ios – How to avoid capturing self in blocks when implementing an API

Short answer

Long answer

Objective-c – capturing self strongly in this block is likely to lead to a retain cycle

Related Topic