Nat! bio photo

Nat!

Senior Mull

Twitter Github Twitch

Argh! Wasted two hours on stupid bug.

I am using the DragNDropOutlineView from the developer examples as the basis for my IDE organization screen. Now I didn't do anything spectular, but I already ran into a dreaded retain, release crasher.

This function in ImageAndTextCell.m is wrong:

- copyWithZone:(NSZone *)zone {
    ImageAndTextCell *cell = (ImageAndTextCell *)[super copyWithZone:zone];
    cell->image = [image retain];
    return cell;
}

Do you know what could go wrong ? Sorry, no price money for this.

Hint 1: This is also defined in ImageAndTextCell a subclass of NSTextFieldCell:

- (void)setImage:(NSImage *)anImage {
    if (anImage != image) {
        [image release];
        image = [anImage retain];
    }
}

- (NSImage *)image {
    return image;
}

Hint 2: Copy would not solve the problem.

Hint 3: The superclass calls setImage: during copyWithZone:

Hint 4: NSCopyObjectdoes a memcpy of the source object into the copy.

The solution

ImageAndTextCell ist declared thusly:

@interface ImageAndTextCell : NSTextFieldCell {
@private
    NSImage     *image;
}

Notice the clever use of @private there.

NSTextFieldCell subclasses eventually from NSCell. NSCell implements setImage: but has it's own instance variable _support which it uses for setImage: (amongst others).

Now NSCopyObject, which is apparently used, copies both instance variables over to the new cell. When -[NSCell copyWithZone:] calls setImage: to copy over the information (yet again) it calls -[ImageAndTextCell setImage:] with an argument of nil because _support isn't set. This will now release our image and of course that is no good.

Who's to blame ? The use of an accessor in copyWithZone: is probably wrong. The original coder should have known that _support is already copied and just retain it. The demo code that overrides setImage: but uses a different instance variable (for good reason) is no piece of genius either. Better would have been a renamed accessor.

OK it wasn't easy. ZNeK got it about 99% right.

If you assume that _support is always nil, and that doesn't seem right, a solution would be:

- copyWithZone:(NSZone *)zone {
    [image retain];
    ImageAndTextCell *cell = (ImageAndTextCell *)[super copyWithZone:zone];
    cell->image = [image retain];
    return cell;
}

The real solution is to not override setImage:.


Post a comment

All comments are held for moderation; basic HTML formatting accepted.

Name:
E-mail: (not published)
Website: