Question
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?
Answer
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
](https://stackoverflow.com/questions/7080927/what-does-the-block-
keyword-mean).
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.