221

I have a working app and I'm working on converting it to ARC in Xcode 4.2. One of the pre-check warnings involves capturing self strongly in a block leading to a retain cycle. I've made a simple code sample to illustrate the issue. I believe I understand what this means but I'm not sure the "correct" or recommended way to implement this type of scenario.

  • self is an instance of class MyAPI
  • the code below is simplified to show only the interactions with the objects and blocks relevant to my question
  • assume that MyAPI gets data from a remote source and MyDataProcessor works on that data and produces an output
  • the processor is configured with blocks to communicate progress & state

code sample:

// code sample
self.delegate = aDelegate;

self.dataProcessor = [[MyDataProcessor alloc] init];

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

self.dataProcessor.completion = ^{
    [self.delegate myAPIDidFinish:self];
    self.dataProcessor = nil;
};

// start the processor - processing happens asynchronously and the processor is released in the completion block
[self.dataProcessor startProcessing];

Question: what am I doing "wrong" and/or how should this be modified to conform to ARC conventions?

XJones
  • 21,959
  • 10
  • 67
  • 82

8 Answers8

508

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.

Community
  • 1
  • 1
benzado
  • 82,288
  • 22
  • 110
  • 138
  • 1
    Awesome answer! Thanks, I have a much better understanding about what is going on and how this all works. In this case, I have control over everything so I'll re-architect some of the objects as needed. – XJones Oct 21 '11 at 21:18
  • 18
    O_O I was just passing by with a slightly different problem, got stuck reading, and now leave this page feeling all knowledgeable and cool. Thanks! – Orc JMR Dec 19 '12 at 08:07
  • is is correct, that if for some reason on the moment of block execution `dp` will be released (for example if it was a view controller and it was poped), then line `[dp.delegate ...` will cause EXC_BADACCESS? – peetonn Jan 30 '13 at 01:50
  • Should the property holding the block (e.g. dataProcess.progress) be `strong` or `weak`? – djskinner Jun 10 '13 at 10:30
  • @DanielSkinner If it is `weak` and nothing else is holding a reference to it, it will be almost immediately released and the property set to `nil` before you have a chance to call it. – benzado Jun 11 '13 at 03:12
  • @benzado, here's a question! Say that inside the block you "want to know" that the object has in fact disappeared. how the heck can you do that? thanks! – Fattie Nov 12 '13 at 08:51
  • @JoeBlow I'm not sure I understand. You should post a new question and tweet the link at me (if nobody else answers it first). – benzado Nov 15 '13 at 22:17
  • I use `__weak typeof(self) weakSelf = self;` to keep the code more portable – stepmuel Mar 05 '14 at 21:17
  • 1
    You might have a look at [libextobjc](https://github.com/jspahrsummers/libextobjc/blob/master/extobjc/EXTScope.h) which provides two handy macros called `@weakify(..)` and `@strongify(...)` which allows you to use `self` in block in a non-retaining fashion. –  Apr 05 '14 at 08:19
  • Awesome answer, please guide how do we use in swift? In my case `deinit` gets called still XCode show that memory is increasing even after popping. :( – Nikhil Manapure Oct 12 '17 at 12:48
  • @NikhilManapure This Q&A is so old, and specific to Objective-C, that I think it would be better to post a new question about Swift. – benzado Oct 16 '17 at 19:25
25

There’s also the option to suppress the warning when you are positive that the cycle will get broken in the future:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-retain-cycles"

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

#pragma clang diagnostic pop

That way you don’t have to monkey around with __weak, self aliasing and explicit ivar prefixing.

zoul
  • 102,279
  • 44
  • 260
  • 354
  • 8
    Sounds like a very bad practice that takes more than 3 lines of code that can be replaced with __weak id weakSelf = self; – pronebird Sep 03 '13 at 14:05
  • 3
    There’s often a larger block of code that can benefit from the suppressed warnings. – zoul Sep 03 '13 at 17:56
  • 2
    Except that `__weak id weakSelf = self;` has fundamentally different behavior than suppressing the warning. The question started with "... if you are positive that the retain cycle will get broken" – Tim Jan 23 '14 at 21:34
  • Too often people blindly make variables weak, without really understanding the ramifications. For example, I've seen people weakify an object and then, in the block they do: `[array addObject:weakObject];` If the weakObject has been released, this causes a crash. Clearly that is not preferred over a retain cycle. You have to understand whether your block actually lives long enough to warrant weakifying, and also whether you want the action in the block to be dependent on whether the weak object is still valid. – mahboudz Mar 27 '19 at 17:50
14

For a common solution, I have these define in the precompile header. Avoids capturing and still enables compiler help by avoiding to use id

#define BlockWeakObject(o) __typeof(o) __weak
#define BlockWeakSelf BlockWeakObject(self)

Then in code you can do:

BlockWeakSelf weakSelf = self;
self.dataProcessor.completion = ^{
    [weakSelf.delegate myAPIDidFinish:weakSelf];
    weakSelf.dataProcessor = nil;
};
Damien Pontifex
  • 1,222
  • 1
  • 15
  • 27
  • Agreed, this could cause a problem inside the block. ReactiveCocoa has another interesting solution for this problem which allows you to continue using ````self```` inside your block @weakify(self); id block = ^{ @strongify(self); [self.delegate myAPIDidFinish:self]; }; – Damien Pontifex May 15 '14 at 04:25
  • @dmpontifex it's a macro from libextobjc https://github.com/jspahrsummers/libextobjc – Elechtron Mar 03 '15 at 21:00
11

Combining a few other answers, this is what I use now for a typed weak self to use in blocks:

__typeof(self) __weak welf = self;

I set that as an XCode Code Snippet with a completion prefix of "welf" in methods/functions, which hits after typing only "we".

Community
  • 1
  • 1
Kendall Helmstetter Gelner
  • 74,769
  • 26
  • 128
  • 150
  • Are you sure? This link and the clang docs seem to think both can and should be used to keep a reference to the object but not a link which will cause a retain cycle: http://stackoverflow.com/questions/19227982/using-block-and-weak – Kendall Helmstetter Gelner Feb 06 '15 at 09:09
  • From the clang docs: http://clang.llvm.org/docs/BlockLanguageSpec.html "In the Objective-C and Objective-C++ languages, we allow the __weak specifier for __block variables of object type. If garbage collection is not enabled, this qualifier causes these variables to be kept without retain messages being sent." – Kendall Helmstetter Gelner Feb 06 '15 at 09:10
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/70434/discussion-between-rob-and-kendall-helmstetter-gelner). – Rob Feb 06 '15 at 23:11
11

I believe the solution without ARC also works with ARC, using the __block keyword:

EDIT: Per the Transitioning to ARC Release Notes, an object declared with __block storage is still retained. Use __weak (preferred) or __unsafe_unretained (for backwards compatibility).

// code sample
self.delegate = aDelegate;

self.dataProcessor = [[MyDataProcessor alloc] init];

// Use this inside blocks
__block id myself = self;

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

self.dataProcessor.completion = ^{
    [myself.delegate myAPIDidFinish:myself];
    myself.dataProcessor = nil;
};

// start the processor - processing happens asynchronously and the processor is released in the completion block
[self.dataProcessor startProcessing];
Tony
  • 3,470
  • 1
  • 18
  • 23
  • Didn't realize that the `__block` keyword avoided retaining it's referent. Thanks! I updated my monolithic answer. :-) – benzado Oct 21 '11 at 21:41
  • 3
    According to Apple docs "In manual reference counting mode, __block id x; has the effect of not retaining x. In ARC mode, __block id x; defaults to retaining x (just like all other values)." – XJones Oct 22 '11 at 02:08
6

warning => "capturing self inside the block is likely to lead a retain cycle"

when you referring self or its property inside a block which is strongly retain by self than it shows above warning.

so for avoiding it we have to make it a week ref

__weak typeof(self) weakSelf = self;

so instead of using

blockname=^{
    self.PROPERTY =something;
}

we should use

blockname=^{
    weakSelf.PROPERTY =something;
}

note:retain cycle is usually occurs when some how two object referring to each other by which both has reference count =1 and their delloc method is never get called.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
Anurag Bhakuni
  • 2,379
  • 26
  • 32
1

The new way to do this is by using @weakify and @strongify marco

@weakify(self);
[self methodThatTakesABlock:^ {
    @strongify(self);
    [self doSomething];
}];

More Info about @Weakify @Strongify Marco

BananZ
  • 1,153
  • 1
  • 12
  • 22
-1

If you are sure that your code will not create a retain cycle, or that the cycle will be broken later, then the simplest way to silence the warning is:

// code sample
self.delegate = aDelegate;

self.dataProcessor = [[MyDataProcessor alloc] init];

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

[self dataProcessor].completion = ^{
    [self.delegate myAPIDidFinish:self];
    self.dataProcessor = nil;
};

// start the processor - processing happens asynchronously and the processor is released in the completion block
[self.dataProcessor startProcessing];

The reason that this works is that while dot-access of properties is taken into account by Xcode's analysis, and therefore

x.y.z = ^{ block that retains x}

is seen as having a retain by x of y (on the left side of the assignment) and by y of x (on the right side), method calls are not subject to the same analysis, even when they are property-access method calls that are equivalent to dot-access, even when those property access methods are compiler-generated, so in

[x y].z = ^{ block that retains x}

only the right side is seen as creating a retain (by y of x), and no retain cycle warning is generated.

Iris Artin
  • 372
  • 2
  • 5