views:

409

answers:

3

I have an instance variable defined and synthesized as a property like so:

@interface CardFrontView : GenericCardView {
    UIImage *_letterImage;
}

@property (nonatomic, retain) UIImage *letterImage;

@end

@implementation CardFrontView
@synthesize letterImage = _letterImage;
...

I set the letterImage property in -init:

- (id)initWithFrame:(CGRect)frame {
    self = [super initWithFrame:frame];
    if (self != nil) {
        NSString *filename = [[NSBundle mainBundle] pathForResource:@"fehu" ofType:@"png"];
        self.letterImage = [UIImage imageWithContentsOfFile:filename];
    }   return self;
}

Then, I release it in -dealloc, whereafter the app crashes (EXC_BAD_ACCESS):

- (void)dealloc {
    [super dealloc];
    [_letterImage release];
}

I can't figure out, however, why the app is crashing. Does anyone have any idea what the deal is here?

+1  A: 

Do you ever access this at any other point in your code? Only thing I can think of is that it's being released somewhere else without being retained first.

It could also -- although I highly doubt this -- be due to the fact that your [super dealloc] call is first, rather than last. Although it's good practice to put it last I was under the impression that it made no functional difference -- you doing anything weird in the super class that could be causing it?

Ian Henry
I found that the fail went away when I moved `[super dealloc]`. `GenericCardView` does nothing weird at all (just some preconfiguration that is common to a few different kinds of `Card`s). So, I have no idea why this happened, and it all makes me rather uncomfortable.
Jonathan Sterling
Looking into it a little further I found this question, which doesn't really clear things up... http://stackoverflow.com/questions/909856/why-do-i-have-to-call-super-dealloc-last-and-not-first
Ian Henry
It should make you rather uncomfortable. That it stops crashing when you remove the `[super dealloc]` indicates that you have a much deeper problem here. Namely, something is hanging on to the `CardFrontView` instance that probably shouldn't be.
bbum
He said moved, not REmoved.
Kendall Helmstetter Gelner
You are absolutely correct.
bbum
+3  A: 

Quite apart from the conventions that require you to release or otherwise, the real problem with your code is the [super dealloc] followed by anything else. Once that method is called, you can't do anything in your object any more.

Instead, do your local cleanup first, and only once you've done that, call the super.

It's basically the inverse of the init method, where you can't do anything useful before the super init call.

Alex

AlBlue
Thanks for the explanation! Can't believe I never thought of that before!
Jonathan Sterling
+20  A: 

Ok... cobbling together from the clues left in both the question and comments.

First, this:

- (void)dealloc {
    [super dealloc];
    [_letterImage release];
}

Is just wrong and could be the source of a crash in and of itself. You are telling the instance that owns _letterImage to dealloc and then telling the _letterImage to release. By the time [_letterImage release] is invoked, the object containing _letterImage is already dead and gone.

While that could cause a crash, it probably isn't (but that should still be fixed). In a comment, you say:

I found that the fail went away when I moved [super dealloc]. GenericCardView does nothing weird at all (just some preconfiguration that is common to a few different kinds of Cards). So, I have no idea why this happened, and it all makes me rather uncomfortable.

All dealloc methods should always have a call to [super dealloc]; as the last line of code in the -dealloc method's implementation. Always and without exception*.

That anything starts working when you remove a call to [super dealloc] is 100% guaranteed to indicate that there is a bug in your code.

In this case, I would not be surprised that your GenericCardView class is incorrectly managing instance variables. I would start by reviewing all of the variables released in that class's dealloc method to make sure they were correctly managed in the first place (while also reviewing the dealloc's implementation itself).

In any case, this smells an awful lot like an over-release problem. Get your code back to the state where the crashes happen all the time, then use Instrument's zombie detection tools to see if they catch the crash and identify the object being over-released. Even if the real cause turns out to be only that you didn't call [super dealloc] last in your dealloc methods, confirmation that that is the case should increase your comfort level.

*There is actually one exception; when you are writing your own root class. But that is so exceedingly rare that it might as well not be counted. (Happy now, Guy? :)

bbum
Hah! I saw the "moved" as "removed" in the comment.
bbum
@bbum Thanks for such an extensive and well-researched response! The problem was definitely the misplacement of the `[super dealloc]` message. Can't believe I didn't see that… :)I've checked `GenericCardView`, and it's all right. Thanks again!
Jonathan Sterling