views:

96

answers:

7

I recently (after hours of debugging) tracked down a segfault in an Objective C iPad app. To boil it down, I had an object TOP which owned MIDDLE which owned BOTTOM. MIDDLE and BOTTOM had retain counts of 1. MIDDLE passed BOTTOM to a method in TOP which ended up releasing MIDDLE, thereby causing BOTTOM to be released and dealloc'd. When the same method in TOP continued working with BOTTOM, it segfaulted. (Note that there are multiple layers of indirection that I left out of the description to keep it simple, but which made debugging a chore.)

Is there a name for what happened? Is there a pattern I can follow to prevent it from happening in the future? Why doesn't the runtime retain objects on the callstack by essentially wrapping methods with [self retain] and [self release] (or retaining arguments the same way, or both)?

Edit:

To be clear, when TOP releases MIDDLE, it sets the pointer to nil. MIDDLE is never accessed through an invalid pointer.

Edit 2: I should have posted actual code to begin with. This is essentially what I have:

// also known as TOP
@interface MyAppDelegate : NSObject <UIApplicationDelegate> {
  UIViewController* controller;
}
@end

@implementation MyAppDelegate
- (void)displayDoc:(Document*)doc {
  DocController* c = [[DocController alloc] initWithNibName:@"DocController" bundle:nil doc:doc];
  [controller release];
  // controller was previously an instance of HomeController
  controller = c;
}

- (void)displayBookmark:(Bookmark*)bookmark {
  [self displayDoc:bookmark.document];
  [controller setPage:bookmark.page];
}

- (void)dealloc {
  [controller release];
  [super dealloc]
}
@end


// also known as MIDDLE
@interface HomeController : UIViewController {
}
@end

@implementation HomeController
- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath {
  Bookmark* b = ...; // pull out of existing data structure, not created here
  MyAppDelegate* app = ...;
  [app displayBookmark:b];
}
@end


// also known as BOTTOM
@interface Bookmark : NSObject {
}
@property NSString* name;
// etc.
@end
A: 

In TOP you probably have a pointer to MIDDLE. When releasing the pointer to MIDDLE do you set the pointer to NULL so that you know that the memory could be invalid?

No one in particular
That's not the issue, the issue is that releasing MIDDLE causes BOTTOM to be released. TOP is finished with MIDDLE but still wants to use BOTTOM.
dreamlax
Time to show some code.
No one in particular
A: 

I‘d call it an inverted Münchhausen (http://en.wikipedia.org/wiki/Baron_M%C3%BCnchhausen).

And: why doesn’t your TOP retain the object, it is willing to own (at least for the time, when it is working on and with it)? Because it is owned by another object, owned by TOP? Expand this image: every object is owned by the application in some way. So why should one do all this retain-release games? A count of one should be enoug! Or not?

Greetings

Objective Interested Person
My fix was to retain/release BOTTOM in the method in TOP. Your question about TOP temporarily owning the object lines up with my question of why the runtime doesn't retain method arguments automatically.
Adam Crume
A: 

“why the runtime doesn't retain method arguments”? Nobody told me your name is Yossaran: http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmObjectOwnership.html#//apple_ref/doc/uid/20000043-1044135

Greetings

Objective Interested Person
I've already read that. I'm familiar with how memory management works in Objective-C; I just happen to think it's broken.
Adam Crume
A: 

Have you actually tried to retain BOTTOM right at the beginning of TOP's method (before releasing MIDDLE)?

If so, and if this is not working, check if you're just releasing MIDDLE and BOTTOM or if you're forcing one or both to dealloc right away (by calling dealloc directly instead of release). While you're just releasing BOTTOM in MIDDLE's dealloc method, it should not be dealloced by the system if there's still another object retaining it.

I hope I haven't missed the point - if so, maybe you should clarify your problem by posting some code, including the release chain from TOP to BOTTOM.

While I'd still try to solve this issue, why don't you delay the release of MIDDLE until TOP doesn't need BOTTOM anymore?

Toastor
+3  A: 

For the general question, there isn't a very good answer except that's the way it works. Non garbage-collected Objective-C is an explicitly memory-managed language. This means it's up to you to take care of the object ownership semantics. There are conventions to help you do this, but ultimately you have to pay attention to who owns what.

Now, you can argue that mere presence on the call stack is a kind of de facto ownership, and it would certainly be possible to design the language that way. However, in practice this would entail a lot of superfluous retaining and releasing that is unnecessary in all well-designed code and would only provide a specious safety buffer in a very few edge cases. It would probably also make a lot of genuine memory management bugs harder to find.

You are, of course, entitled to disagree with that analysis, but neither you nor I have any say in the matter.

In the specific case you describe, it seems to me the problem is that you haven't properly defined the ownership semantics when passing BOTTOM out from MIDDLE. There are two possibilities:

  1. MIDDLE returns a weak reference, which may subsequently go away. In this case it is up to TOP to retain before doing anything that might change the status of BOTTOM -- and calling release on MIDDLE surely falls into that category.
  2. MIDDLE confers temporary ownership on the caller, with the reference expected to remain good until the autorelease pool drains: return [[BOTTOM retain] autorelease]; In this case, it is safe to release MIDDLE afterwards.

Both of these are valid approaches as Objective-C stands, and both will work. You just need to decide explicitly which you're using and act accordingly. Your bug arose from doing the first but expecting it to behave like the second.

walkytalky
MIDDLE calls TOP, not the other way around. See my edit. MIDDLE is the middle in terms of ownership.
Adam Crume
@Adam OK, but it makes little material difference to the argument. The fundamental problem remains the same, you've just wrapped it in a more opaque flow of control. If it were me, I don't think I'd be claiming that as a point in my favour.
walkytalky
It does make a difference, because returning an object (and transferring ownership) is different from passing one as an argument and only conferring temporary ownership. I never claimed that I did things right; I admitted to having a bug, after all.
Adam Crume
+2  A: 

There are a number of reasons the runtime can't reasonably retain and release every argument for you.

  1. People already complain about Objective-C's inefficiency. Forcing arity*2 messages for every method call in addition to the method's actual code would be nuts.

  2. An arbitrary object cannot be assumed to respond to retain and release. That's unique to classes conforming to NSObject.

  3. Arguments can be and often are rebound. In order to do this, the runtime would have to track the initial values of the arguments.

  4. There are reasons not to want this behavior, which seem at least as valid as the reasons for wanting it (essentially, it could simplify a design in cases where you don't want to go to the trouble of following the memory management contract). There could be a method that intentionally releases an argument and allocates a new one in its place (along the lines of an init method) and in the case of large objects, this could result in a much larger memory footprint. This would be especially troublesome on the iPhone, which is the primary platform for retain and release.

Chuck
#2 is one thing that keeps tripping me up. I find it very strange that there is no unique root class in Objective C.
Adam Crume
@Adam Crume: It's a Smalltalkism that Objective-C inherited.
Chuck
A: 

Is there a pattern I can follow to prevent it from happening in the future?

Yes. Your TOP object needs to claim ownership of the BOTTOM object. You own any object that you obtain by calling 'alloc', 'new' or 'copy'. We can probably assume that you aren't getting a reference to the BOTTOM object by calling one of those three methods, which means that if you want BOTTOM to stick around, you need to call 'retain' on it.

It sucks that you had to spend the time tracking down your bug, but from the sounds of it your objects were behaving exactly as conventions dictate.

kubi
BOTTOM is passed into a method (but not stored). Are you saying I should retain method arguments?
Adam Crume
TOP is getting a reference to BOTTOM via a method call, right? If TOP wants to keep that object around, it needs to retain it.
kubi