0

I'm new to Objective-C and can't seem to get the memory management code right. I have the following code:

Media* myMedia = [self.myMediaManager getNextMedia];

self.navigationItem.title = [self.myMediaManager getCategory];
[self.btnImage setImage:myMedia.imageFile forState: UIControlStateNormal];
[self.lblImage setText:myMedia.imageLabel];

//[myMedia release];

My app crashes if I uncomment the above line. Do I need to do something special when I instantiate myMedia?

EDIT:

If myMediaManager is supposed to release it, when would it do that. Here is my code for getNextMedia:

- (Media*) getNextMedia {

    DLog(@"Start");

    Media* nextMedia = [[Media alloc] init];

    [self setNextMediaIndex];

    if (self.mediaIndex > -1)
    {
        nextMedia = [mediaArray objectAtIndex: self.mediaIndex];
    }

    return nextMedia;
}

EDIT2: I fixed the crashing issue (I was releasing an object I didn't own). I still see leaks and can't seem to find what the issue is.

Cœur
  • 37,241
  • 25
  • 195
  • 267
user472292
  • 1,069
  • 2
  • 22
  • 37
  • Please post your the code for `getNextMedia` and `getCategory`. Otherwise, how can we tell? The crash happens because of the interaction of various portions of your code, not just the lines immediately above the crash point. – Yuji Aug 14 '11 at 15:33
  • Since he doesn't retain `[self.myMediaManager getNextMedia]` from `self`, he doesn't need to, its `retainCount` will still be the same. – cutsoy Aug 14 '11 at 15:34
  • 1
    Don't prefix methods with `get`; that should just be `nextMedia`. – bbum Aug 14 '11 at 17:48
  • I read up about memory management. All the posts helped my understanding. Thanks again! – user472292 Aug 14 '11 at 23:11

5 Answers5

4

Since myMedia is not retained here, you don't need to release it. When the origin (self.myMediaManager) releases it, it gets destroyed immediately.

NSString *string = [[NSString alloc] init];
[string release]; // now we have to release the string, since we allocated it.

NSString *string = self.navigationItem.title;
// now we don't, since it's a property of `navigationItem` and we didn't retain it.
cutsoy
  • 10,127
  • 4
  • 40
  • 57
4

Only objects that you own can be released.

You can release objects if you new, alloc, copy, mutableCopy or retain them first.

Since there is no alloc/copy/retain in [self.myMediaManager getNextMedia]; you can't release it.

Community
  • 1
  • 1
akashivskyy
  • 44,342
  • 16
  • 106
  • 116
1

At this point, since you are just learning, you should probably just start off using ARC with the iOS5 beta versions of XCode. It's good to have an understanding but using ARC will save you many potential pitfalls - by the time you learn enough to produce something iOS5 will be out. You can still build applications targeting iOS4, so you'll still be able to reach a lot of people.

Kendall Helmstetter Gelner
  • 74,769
  • 26
  • 128
  • 150
  • 1
    Generally I agree, but ARC doesn't do away with it; just hides it. You still need to be careful with Core foundation objects. Also, you need to understand what is being retained and released in such places as blocks where you could end up with a retain cycle when accessing properties of self, because self is implicitly retained. – Abizern Aug 14 '11 at 15:40
  • Thanks, didn't know that was coming in iOS 5. Sounds like a lot of fun (if it really works as well as Apple describes it) =). – cutsoy Aug 14 '11 at 15:40
  • @Abizem - Between the work that ARC does for you and the compiler warnings, it really eliminates a lot of possible error - also although self is retained in a block, that ends up only being a potential issue in long-lived blocks such as ones you'd pass off as a callback for a long-lived process like a repeated timer. In something like an animation completion self might be retained after you have released something like a view controller but when the animation ends it will still be released, just a little later... it's a good idea to understand memory, but ARC is so much easier day to day. – Kendall Helmstetter Gelner Aug 14 '11 at 15:50
  • @Kendall - don't get me wrong - I think ARC is a HUGE advantage. But you've just demonstrated a knowledge of memory management that a lot of people don't have and they think that ARC means they don't have to think about it. If you have unexpected behaviour because of any of these things you're able to very quickly realise what the problem is. But for somebody else - it means coming on to SO with compiler errors/warnings that they don't understand. – Abizern Aug 14 '11 at 16:22
  • @Abizem: My feeling after using ARC for a bit is that in order to get into situations where ARC will be confused, you'll have to have enough understanding of the system anyway that you will not be confused yourself when you get there. This person will not bump into the ARC walls until they can understand things much better... – Kendall Helmstetter Gelner Aug 16 '11 at 03:13
1

The general rule for memory management is as follows:

For every retain, alloc, copy, or new, you need to call release or autorelease.

Since you did not call any these, you do not need to release myMedia.

For more information, take a look at this other answer I posted that deals with the subject. Also, since you are new to iOS development, I suggest looking at this answer as well.

Community
  • 1
  • 1
Moshe
  • 57,511
  • 78
  • 272
  • 425
1

This updated code is suspicious:

Media* nextMedia = [[Media alloc] init];

[self setNextMediaIndex];

if (self.mediaIndex > -1)
{
    nextMedia = [mediaArray objectAtIndex: self.mediaIndex];
}

Depending on the condition in the if() clause, you assign a new value to nextMedia, which makes the value you just allocated unreachable, i.e. it can't be released.

Also, you don't retain the value you get from the array, so you should not release it either. But if the if() clause does not run, you still have the instance you alloc-ed, and that should be released.

That is not good. Try:

Media* nextMedia = [[Media alloc] init];
[self setNextMediaIndex];
if (self.mediaIndex > -1)
{
    [nextMedia release];
    nextMedia = [[mediaArray objectAtIndex: self.mediaIndex] retain];
}

You could also do (and I would prefer that):

Media *nextMedia;
[self setNextMediaIndex];
if (self.mediaIndex > -1)
{
    nextMedia = [[mediaArray objectAtIndex: self.mediaIndex] retain];
}
else
{
    nextMedia = [[Media alloc] init];
}

Now you can release nextMedia when that is not needed anymore, without any ambiguity about the retain count.

Rudy Velthuis
  • 28,387
  • 5
  • 46
  • 94