Creating an autoreleased object in a method and returning it to an instanced variable causes crash

Question

I'm making a mess of creating an object in a method and returning it to a variable. As in this post I know I should autorelease an object in this case, but when I do, it crashses.

I have written a method to create an array of images, and return this array. It looks like this:

- (NSMutableArray *)createImagesFor:(NSString *)animName withFrames:(int)numberFrames {
    NSMutableArray *imageArray = [[NSMutableArray alloc] initWithCapacity:numberFrames];
    for (int i = 1; i <= numberFrames; ++i) {
        NSString *imageName = [[NSString alloc]initWithFormat:@"%@%i.png", animName, i];
        [imageArray addObject:[UIImage imageNamed:imageName]];
        [imageName release];
    }
    return imageArray;
}

I call it like this:

NSMutableArray *imageArray;
imageArray = [self createImagesFor:@"jumping" withFrames:2];
self.animationImages = imageArray;
[imageArray release];

However, when I run the build analyzer, it compiles but with the following complaint:

Potential leak of an object allocated on line 109
1. Method returns an Objective-C object with a +1 retain count (owning reference)
2. Object returned to caller as an owning reference (single retain count transferred to caller)
3. Object allocated on line 109 is returned from a method whose name ('createImagesFor:withFrames:') does not contain 'copy' or otherwise starts with 'new' or 'alloc'. This violates the naming convention rules given in the Memory Management Guide for Cocoa (object leaked)

I've had a look at the memory management document but other than autoreleasing the variable (which crashes it), I'm not sure where I'm going wrong. This is how I autoreleased it:

NSMutableArray *imageArray = [[[NSMutableArray alloc] initWithCapacity:numberFrames]autorelease];

I've tried retaining the *imageArray as suggested here like so:

NSMutableArray *imageArray;
[imageArray retain];
    imageArray = [self createImagesFor:@"jumping" withFrames:2];
    self.animationImages = imageArray;
    [imageArray release];

But this also crashes.

The analyzer suggests I change the name of the method to something like 'newCreateImagesFor:withFrames:' but I don't see how this fixes things?

Thanks for the help.

Michael

grahamparks · Answer 1 · 2010-11-13T17:33:59.680

5

You should change the last line of first block of code to return [imageArray autorelease] and get rid of the release in the second part (you normally don't ever need to release objects returned by method calls). That's what the analyzer complaints are about. However, I don't see why it would cause a crash.

How is the animationImages property defined? That might be the source of your problems.

edited Nov 13 '10 at 17:33

answered Nov 13 '10 at 13:30

grahamparks

16,130
5
49
43

Great - that makes sense. I was over-releasing the object which must have been causing the crash. And it works fine now, especially when I declare the property accessors, rather than instantiating on the fly. Thanks! – Smikey Nov 13 '10 at 15:22

koo · Accepted Answer · 2010-11-13T13:48:59.850

    - (NSMutableArray *)createImagesFor:(NSString *)animName withFrames:(int)numberFrames {
    NSMutableArray *imageArray = [[NSMutableArray alloc] initWithCapacity:numberFrames];
    for (int i = 1; i <= numberFrames; ++i) {
        NSString *imageName = [[NSString alloc]initWithFormat:@"%@%i.png", animName, i];
        [imageArray addObject:[UIImage imageNamed:imageName]];
        [imageName release];
    }
    return imageArray;
}

returns an imageArray object that has a retain count of +1 but not auto-released, clang static analyser will warn you about this, and it is all to do with naming convention, because your method is not named to be like newImagesFor... or allocImagesFor... or copyImagesFor....

NSMutableArray *imageArray;

[imageArray retain];
// sending message to nil, does nothing

imageArray = [self createImagesFor:@"jumping" withFrames:2];
// imageArray has a retain count of 1
// and it is an autoreleased object

self.animationImages = imageArray; 

[imageArray release]; 
// retain count = 0, will be dealloc'd, 
// however it is already in the autorelease pool, 
// it will be over-released at the end of current event run loop

When you declare imageArray, it is a null pointer to the class NSMutableArray, sending a message, in your case retain, to a null pointer in Objective-C is possible and will not throw an exception.

If you use a property accessor to cache your imageArray object, your property accessor should be declared to retain the object you assign to

@property (retain) NSMutableArray *imageArray;

Now that your method properly return an autoreleased imageArray, and you have a correct property accessor, all that's needed is

NSMutableArray *imageArray;
imageArray = [self createImagesFor:@"jumping" withFrames:2];
self.animationImages = imageArray;

Ok I see - I should be using properties to hold the autoreleased objects, and not over-releasing them. And since I'm using properties, I don't need to worry that they will be autoreleased too soon. Thanks! — Smikey, Nov 13 '10 at 15:23
"When you declare imageArray, it is a null pointer to the class NSMutableArray". Only un-initialized instance variables are guaranteed to be nil. Un-initialized local variables may be zero or may be some random number from the last time the block of memory it's stored in was used. — grahamparks, Nov 13 '10 at 17:40
@Adam Ko : is it because the variable imageArray is a stack variable for the function and once the function call finishes the stack pointer is deallocated and further access using the deallocated pointer caused it to crash. — prajul, Sep 11 '12 at 10:04

Creating an autoreleased object in a method and returning it to an instanced variable causes crash

2 Answers2