Better Reference Counting (Safe Release)

When it comes to manually managing memory, it's absolutely fantastic to have coding patterns for making your memory management duties...well...manageable.  Reference counting is one such pattern that is widely used throughout computer programming.  COM (which is on top of C++) uses it extensively, and thus the underlying system of memory management in C# uses it, but so does Core Foundation (C level API on Mac OS X and iOS) and, even more noticeably, is that the Objective-C language has the reference counting memory management pattern built in.

At this point, I assume everyone knows what reference counting is and how to use it. If not, you should verse yourself with Apple's Memory Management Overview.  Now since Apple has introduced Automatic Reference Counting (ARC), objective-c memory management has reached near trivial levels as that of managed memory languages like Java and C#.  But for those of us that need to support older platforms, work in legacy code bases or develop public libraries that require non-ARC support, being elegant with reference counting is very important.  And even those of us that are converted over to ARC, we still need to be aware of Core Foundation memory management which does not have ARC.

So that was a serious introduction to nothing it would seem! Well, what this post will actually cover is the pitfall of recursive (aka reentrant) code that can lead to crashing in a ref counted world.   Basically, we'll deal with releasing an object that leads to it being released again.

The problem: reentrant releases!

WARNING: Contrived example coming up!! Don't just dismiss this topic due to the example being impractical, I know it's impractical - it's just an example.

First let's establish the problem with an example. Ignoring that iOS 6 has deprecated viewWillUnload and viewDidUnload (a subject I tackle in this post), it is common practice to have member variables of UIViewController instances that have a life time from viewDidLoad to viewDidUnload. Those member variables are often internal an not exposed as properties, so having self.property = nil; in viewDidUnload is not really necessary because creating a synthesized property is overkill.  Instead we pair the alloc-init or retain that is done in viewDidLoad with a release and an assignment of nil.

- (void) viewDidLoad
{
    [super viewDidLoad];
    // load work ...
    _someObject = [[SomeObject alloc] init];
}

- (void) viewDidUnload
{
    [super viewDidUnload];
    // unload work ...
    [_someObject release];
    _someObject = nil;
}

- (void) dealloc
{
    // remember to release in dealloc
    [_someObject release];
    [super dealloc];
}

Seems reasonable right?  In fact this has been my pattern for releasing and niling an object for over a decade.  So much so, that I made it a best practice to use a helper macro.  [NOTE: I love macros, the pre-processor is my best friend and I will have to do a post on macros sometime in the future]

#define NIL_RELEASE(obj) { [obj release]; obj = nil; }

So now we have:

- (void) viewDidUnload
{
    [super viewDidUnload];
    // unload work ...
    NIL_RELEASE(_someObject);
}

Alright! Nice and clean, and everywhere throughout my code. Seems like we have great pattern for releasing an object's reference and then assigning the reference nil. Now enters the problem of reentrant code. Now first and foremost, a developer should do his diligence to ensure not to create retain cycles. But sometimes we can end up accepting a retain cycle if we know that we can break that cycle somehow. Let's say we've put ourselves in that position. Our _someObject has a property to hold a block for some use. Now let's say at some point we set that block property with a block that uses self (the UIViewController we are dealing with). Since that block now retains self we now have a retain cycle. But we have looked at the code and see that _someObject will have a shorter lifecycle than the UIViewController or we just will rely on a memory warning purging the view which will call viewDidUnload.

Here's the problem as it will unfold. [BTW: I apologize for using dealloc as a verb]

1. Memory warning occurs
2. viewDidUnload is called
3. _someObject has release called
4. _someObject now deallocs due to it's last reference being released
5. _someObject's dealloc releases it's block property
6. the block deallocs and releases the reference to self
7. self now has no more references and needs to dealloc
8. _someObject has release called!
9. EXEC_BAD_ACCESS - _someObject has already started deallocing!

Oh man!  We've just established that our "safe" NIL_RELEASE macro isn't safe at all!  And we've already spread our NIL_RELEASE throughout our codebase!  Let's tackle this.

So we've established that due to reentrancy, the reference _someObject can be compromised on release. We also already know that objective-c will handle sending messages to (aka calling methods on) nil references. So logically, we can note that we want to set the reference to nil before we call release. So let's update our macro!

#define NIL_RELEASE(obj) \
id obj_ = obj; \
obj = nil; \
[obj_ release];

Alright! Now we've solved the reentrancy! But wait, what if we have multiple objects we want to release? The obj_ temporary variable will cause compiler warnings. No problem! We'll scope the NIL_RELEASE (like I had previously). This is also valuable if we use NIL_RELEASE in an if, else, while, or for statement that isn't scoped like this:

if (true)
    NIL_RELEASE(_someObject);

which the precompiler will expand into this (yikes!):

if (true)
    id obj_ = _someObject;
_someObject = nil;
[obj_ release];

It should really always be considered best practice to scope you macros that behave like functions, paren your macro variables, and paren your macro if it behaves like an expression. #define SUM(x,y) ((x) + (y)). I'll have to touch on macros again sometime. So scoped, we now have:

#define NIL_RELEASE(obj) \
{ \
id obj_ = obj; \
obj = nil; \
[obj_ release]; \
}

That should be it, right? Right!?!? Oh so close! Here we come to the point where compilers can really be too smart for their own good! First let's break down what will happen in the NIL_RELEASE macro at a C level.

- (void) viewDidUnload
{
    objc_msgSend(super, @selector(viewDidUnload));  // [super viewDidUnload];
    id obj_ = _someObject;
    _someObject = nil;
    objc_msgSend(obj_, @selector(release));         // [obj_ release];
}

Now the compiler comes along and says, "Hey, this is a waste of operations! There's no need for a temporary variable obj_ since I can directly release _someObject and then set it to nil. Let's do that instead!" So, thanks to optimizations in the compiler to remove unnecessary instructions, we get:

- (void) viewDidUnload
{
    objc_msgSend(super, @selector(viewDidUnload));  // [super viewDidUnload];
    objc_msgSend(_someObject, @selector(release));  // [_someObject release];
    _someObject = nil;
}

Dang it! We now have exactly the same thing as the original NIL_RELEASE! What can be done! Never fear, memory barriers are here! So in order to have the compiler not over optimize the code into a bug, we want to separate the assignment of _someObject to the temporary variable from the function call against the temp variable such that the compiler will treat them as independent. To do this we use the OSMemoryBarrier() function. And with the content kept in the safety of a do/while statement, we now have:

#define SAFE_RELEASE(obj) \
do { \
id obj_ = obj; \
obj = nil; \
OSMemoryBarrier(); \
[obj_ release]; \
} while (0)

// you'll need to include the header
#include <libkern/OSAtomic.h>

There you have it; a complete mechanism for safely releasing an object reference and setting it to nil. In fact, I've renamed the macro to SAFE_RELEASE. I'll also provide a Core Foundation macro remembering that CFRelease cannot be passed NULL.

#define CFReleaseSafe(ref) \
if (ref) \
{ \
CFTypeRef ref_ = ref; \
ref = NULL; \
OSMemoryBarrier(); \
CFRelease(ref_); \
}

// you'll need to include the header
#include <libkern/OSAtomic.h>