views:

195

answers:

4

I've been coding for a while now in objective-c and am comfortable with it... but one thing eludes me. Memory management. I'm releasing as I think is correct, but this bit of code is throwing a "EXC_BAD_ACCESS" and crashes the app.

When I comment out and DON'T release the button and image, it works fine. The function is called to read through an array of image filenames.

for (x=items_start;x<items_stop;x++) {

    UIButton *button;
    UIImage *buttonImage;

    buttonImage = [UIImage imageNamed:[NSString stringWithFormat:@"%i.png", x]];
    button = [UIButton buttonWithType:UIButtonTypeCustom];
    button.tag = x;
    [button setImage:buttonImage forState:UIControlStateNormal];
    [button addTarget:self action:@selector(duplicateImage:) forControlEvents:UIControlEventTouchUpInside];

    [viewBasicItems addSubview:button];     

    [buttonImage release];
    [button release];

}

any ideas? Like i said, when I comment out the last two lines (releasing the button and image) it works OK. Is this normal or should I be able to release them?

Note: I have remove a fair bit of other code to show this example in a smaller chunk!

+3  A: 
buttonImage = [UIImage imageNamed:[NSString stringWithFormat:@"%i.png", x]];

Your buttonImage object is autoreleased so you must not release it in your function.

From Memory management guide:

You only own objects you created using a method whose name begins with “alloc” or “new” or contains “copy” (for example, alloc, newObject, or mutableCopy), or if you send it a retain message.

Edit: As Alex points your button object is autoreleased also.

Vladimir
thanks for the help and the great quote!
Matt Facer
+5  A: 

The instance of button is autoreleased:

button = [UIButton buttonWithType:UIButtonTypeCustom];

You're using the convenience method +buttonWithType: instead of an alloc/init pair. So your app will crash here, as well:

[button release];

Either remove that -release statement or use alloc/init to instantiate the button view.

I would recommend you use alloc/init since you're doing all of this stuff inside a for loop. You could be building up a lot of objects in that loop that need to be autoreleased. It's probably better to manually allocate memory and release it.

And do read Apple's memory management guide.

Alex Reynolds
I see! That explains. Thank you for your help.
Matt Facer
A: 

You have three choices:

Use alloc/init for example;

NSString *imagePath = [[[NSBundle mainBundle] resourcePath] stringByAppendingString:[NSString stringWithFormat:@"/%i.png",x]];
buttonImage = [[UIImage alloc] initWithContentsOfFile:imagePath];

and release it after you are done

[buttonImage release];

Or use retain/copy

buttonImage = [[UIImage imageNamed:[NSString stringWithFormat:@"%i.png", x]] retain ];

and release it

[buttonImage release];

Or use Autoreleased objects like you did but do not release it, because they will be released automatically. You should read Apple memory management guide like others said.

EEE
A: 

Ok, the basics.

Most methods that create objects return objects that are allocated, then "autoreleased."

The autorelease call adds your object to the "autorelease pool", which means that they will receive a release call next time your app visits the event loop.

When you use auto released objects, you can use them, then forget about them. They get released automatically.

The exception, as others have said, is calls that have "init" or "new" in the name, or calls to "copy" methods. These methods return objects that have not been auto released. The owner of these objects needs to release or autorelease these objects in order for them to be deallocated, and not cause a memory leak.

In your example code, you create your buttonImage and button objects using the calls +imageNamed and +buttonWithType.

These are class calls that return an object of the desired type. Since they do not contain "init" or "new" in their names, the objects that they return are already autoreleased, so you should NOT release them.

You pass the image you create to the button, so the button retains the image. You then pass the button object to your viewBasicItems object with the -addSubview call, so the view retains the button.

Thus, you should not do anything else. The button will retain the image, and the view will retain the button.

I hope that helps.

Duncan C