Returning Retained Properties in Objective-C

Posted on by Ben

In Objective-C, retained object properties are quite common:

@class MyOtherObject;

@interface MyObject : NSObject
{
    MyOtherObj* mOtherObj;
}

@property (nonatomic, retain) MyOtherObject* otherObj;

- (void) doSomeRandomThings;

@end

@implementation MyObject

@synthesize otherObj = mOtherObj;

// ...

@end

Well, that’s all well and good, but what does that @synthesize actually do?

You might think that it does something like this:

// The "getter"
- (MyOtherObject*) otherObj
{
    return mOtherObj;
}

// The "setter"
- (void) setOtherObj:(MyOtherObject*)otherObj
{
    [otherObj retain];
    [mOtherObj release];
    mOtherObj = otherObj;
}

And you’d be at least close. But that getter has a “gotcha” that might be obvious to some coders, and invisible to others.

The Bug

If we use this code as-is, we are subject to the following bug:

// Fetch myObj.otherObj:
MyOtherObj* otherObj = myObj.otherObj;
// Do some other things, including maybe:
[myObj doSomeRandomThings];
// Now use otherObj:
[otherObj doCoolStuff];

Looks fine, right?

Nope.

It crashes (sometimes?) on that last line, with something like EXC_BAD_ACCESS or an NSInvalidArgumentException where an unrecognized selector (guess which selector? right: it’s @selector(doCoolStuff)) was sent to some random object you’ve never heard of.

A zombie dereference.

How It Happens

But how is otherObj a zombie here?

Here’s how. Let’s take a look at -[MyObject doSomeRandomThings]:

- (void) doSomeRandomThings
{
    self.otherObj = [[[MyOtherObject alloc] init] autorelease];
}

Whoops. The old MyOtherObject that used to be referenced by otherObj has now been released, and the user has had it yanked out from under them.

How To Fix It

Let’s adjust the property getter:

- (MyOtherObject*) otherObj
{
    return [[mOtherObj retain] autorelease];
}

Problem solved.

Explanation

The return mOtherObj; idiom is very common, but it’s unsafe, for exactly the reason pointed out here. You’re returning the object you have now, and you know it’s valid now because you’re using it internally, but you really can’t make any guarantees about how long it will be valid, since something else might mutate your internal state.

The return [[mOtherObj retain] autorelease]; idiom is safer, because it adds a temporary extra reference on the object, in addition to the one you have internally, which is guaranteed to be valid at least until the current autorelease pool drains (which is certainly going to be at least until your caller returns, unless he is managing his own autorelease pools, in which case we hope he knows what he’s doing).

Pros and Cons

Some will argue that this idiom is wasteful, because it fills up the autorelease pools with objects that may live longer than they need to; that instead of making your getters do the [[mOtherObj retain] autorelease] dance, the callers should instead retain the objects they receive from getters and release them manually (or autorelease them themselves), since they know better what their needs are regarding the lifetime guarantees of objects they get. I disagree strongly, because I think that that is a brittle model. I think that the only time that this is likely to cause a real memory problem (a temporary increase in working set size) is when the caller is doing something in a loop for a potentially large number of iterations without returning to the main event loop, and if they’re doing that, they probably ought to be creating and draining their own autorelease pools for the loop anyway.

Ben

More Posts