1

Anyone able to help re what is the source of the leaky object for this code:

My application compiles without any ANALYZE issues. When I run PROFILER and look at Memory Leaks I see leaks appears. One of these leaks is in my WeekendEvent object. There are 3 Leaked Block row items in instruments per the below (I've noted in the code where these point to):

  • Malloc +1
  • Retain +2
  • Release +1

Question - I assume this implies there is a release, however where would this leak be from. The segments of my code that clicking through on instruments highlights is below. To me it seems OK in that:

  1. the WeekendEvent I create I release after passing into the addWeekendEvent method
  2. in the addWeekendEvent it just adds it to a NSMutableArray, and hence I thought the arrange does any memory management for it's object it contains
  3. I do release the NSMutableArray in the dealloc too

Key Source Code Bits & What Instuments Highlights

// ------Weekends Method (plural)-----
WeekendEvent *weEvent = [[WeekendEvent alloc] init]; // [INSTRUMENTS - 87.5%]
[week addWeekendEvent:weEvent];                      // [INSTRUMENTS - 12.5%]
[weEvent release];                  


//------Weekend *.h ------------
@interface Weekend : NSObject {
    NSMutableArray*     _events;     
}
- (void)addWeekendEvent:(WeekendEvent*)weEvent;
@property (nonatomic, retain) NSMutableArray* events;
@end


//------Weekend *.m -------------
- (void)addWeekendEvent:(WeekendEvent*)weEvent {
    [self.events addObject:weEvent];
}
- (void) dealloc {
    [_events release];
    [super dealloc];
}

EDIT: Some additional code re how the "week" variable above was created/used - so in the Weekends Method the code I posted was within a for loop - the code with the for loop shown therefore was:

for (Weekend *week in self.items) {
   // do pass "week.start" to some methods (which is an NSDate) - don't think this would be relevant though?
   WeekendEvent *weEvent = [[WeekendEvent alloc] init]; // [INSTRUMENTS - 87.5%]
   [week addWeekendEvent:weEvent];                      // [INSTRUMENTS - 12.5%]
   [weEvent release];   
}               
// Note - self.items I checked is "released" in the dealloc method

EDIT 2 - Just to confirm, it is an "WeekendEvent" instance that Instruments highlights in it's "leaked objects" column. Just in case this wasn't clear.

EDIT 3 - Re how I setup the items variable - key code bits are:

@interface Weekends : NSObject {
    NSMutableArray* _items;
}
@property (nonatomic, retain) NSMutableArray* items;

@synthesize items = _items;
- (void) dealloc {
    [_items release];
    [super dealloc];
}
Greg
  • 34,042
  • 79
  • 253
  • 454

1 Answers1

1

The memory management in the code you show is correct, assuming that the rest of your Weekend class looks something like this:

@synthesize events = _events;

- (id)init {
    if ((self = [super init]) == nil) { return nil; }
    _events = [[NSMutableArray alloc] initWithCapacity:0];
    return self;
}

Also, the instruments results match all of your code:

Malloc  +1  ==  WeekendEvent *weEvent = [[WeekendEvent alloc] init];
Retain  +2  ==  [week addWeekendEvent:weEvent];
Release +1  ==  [weEvent release];

Based on that logic, the most likely candidate is that your week object is not properly released. You haven't shown the code that explains how it was created, but I do notice that the code you did post is for a Weekend class. Are you sure week is not of a different type?

e.James
  • 116,942
  • 41
  • 177
  • 214
  • @e.James curiousity: why `_events = [[NSMutableArray alloc] initWithCapacity:0];`, and not simply `_events = [NSMutableArray new];`? – justin May 25 '11 at 03:47
  • @Justin: the short answer is "convention". A slightly longer answer is that the `alloc/init...` idiom conforms to Apple's coding standards, and also allow you to add arguments as required. More details here: http://stackoverflow.com/questions/719877/use-of-alloc-init-instead-of-new-objective-c/719882#719882 – e.James May 25 '11 at 04:02
  • sorry - i should have worded that differently: why do you prefer `initWithCapacity:0` over `init`? – justin May 25 '11 at 04:15
  • No good reason `:)` The first time I needed `NSMutableArray`, I looked up the docs, and since `initWithCapacity:` was the only listed initializer (the others are inherited from `NSArray`), that's the one I went with. – e.James May 25 '11 at 04:40
  • `initWithCapacity:` is a hint and not really terribly useful. The chances of it ever mattering in a real world application are slim to none. – bbum May 25 '11 at 04:40
  • @e.James - (just back) - I posted some additional code to answer your question re "weekend", and whether this could be the culprit. I noted it created in a loop - not sure if I'm doing something wrong here? Also if this was the cause why wouldn't Instruments highlight the "for" line? – Greg May 25 '11 at 04:45
  • @bbum: no disagreement here. I rarely pass in anything other than `:0`, which really screams "why do this at all?". Again, this is just the designated initializer for `NSMutableArray` in the docs, but it is by no means required. – e.James May 25 '11 at 04:55
  • @e.James - PS: could it be the case that the issue could be in how the "items" array is used through out the life of the application (even through I know I can see it does have an "[_items release]" in the dealloc method? That is, even though Instruments seems to say the issue is the "WeekendEvent", that if an instance of one is buried within other parent objects, then the issue could be triggered by bad memory management code in any of the parent objects that carry it? And if this is the case Instruments doesn't seem to help track it down then? – Greg May 25 '11 at 04:58
  • @Greg: everything still looks legit in the code you have posted, but clearly the `addWeekendEvent:weEvent` line is causing a `retain` that is never balanced out by a call to `release`, as indicated by instruments. I would definitely start looking higher up in the chain to figure out which object is not being released. *some*thing is keeping a reference to that `_events` array. – e.James May 25 '11 at 05:01
  • @Greg: absolutely, yes. Instruments is showing that `_events` retains the object, (resulting in +2) but that it never releases it. It could easily mean that `_events` is improperly handled somewhere else. – e.James May 25 '11 at 05:03
  • @Greg: or, of course, `items`, which contains all of those Weekend objects (just to be clear). – e.James May 25 '11 at 05:04
  • @e.James - ok :( - could be a challenge then. So there's no tool to help trackdown the root cause then? (i.e. Instruments helps with where the leaky object was created, but doesn't really help with finding the root cause). Besides Instruments any good techniques/approaches or is it really role the sleeves up and do code review? – Greg May 25 '11 at 05:14
  • @Greg: Not much, I'm afraid. Instruments points you to the object that was leaked, and gives you the retain/release history so you can see where the extra retain comes from. Beyond that, you have to determine why that extra retain happened. In this case, the `_events` array is your only clue, since it is the object that takes ownership of the leaked object and there are no other retains/releases in the chain. It is curious that instruments does not list the `_events` array as being a leaked object itself. – e.James May 25 '11 at 05:23
  • Hang on: one quick thought: how *exactly* did you write the property definition for events? The one inside Weekend.m? – e.James May 25 '11 at 05:26
  • I ask because if you have accidentally omitted the `= _events`, there will actually be an `events` mutablearray created automagically, which will *not* be released. – e.James May 25 '11 at 05:47
  • @e.James - Just put the details in EDIT 3 in the post - I think I've covered this off ok, if I get what you're suggesting? - i.e. the "@synthesize items = _items;" line – Greg May 25 '11 at 07:05
  • 1
    @e.James `initWithCapacity` is the only initializer in `NSMutableArray` documentation, because inheriting from `NSArray`, it already has all the `NSArray` constructors, which are ONLY in `NSArray` documentation. Apple docs generally doesn't redescribe inherited methods. But that doesn't mean it is the designed one. My bet is that `[[NSMutableArray alloc] init]` use `initWithCapacity:` with a default (and reasonable) value, like most languages does. – Vincent Guerci May 25 '11 at 07:38
  • @Vincent G: That's pretty much exactly what I said (regarding methods inherited from NSArray). I am by no means saying that using `-init` with `NSMUtableArray` is wrong. As far as `initWithCapacity:` being the designated initializer, that is absolutely true. The designated initializer is usually the longest initializer method, and it is the one that other initializers must call in the end. http://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/CocoaFundamentals/CocoaObjects/CocoaObjects.html#//apple_ref/doc/uid/TP40002974-CH4-SW55 – e.James May 25 '11 at 13:54
  • @e.James I see your point of view, which I guess can save one message send and it is generally a good idea. After it is a question of coding style preference, and imho, `initWithCapacity:0` is far less readable than `init` and brings too many questions :) plus the fact the very little improvement it brings *might* be annihilated by the first resizing of the inner structures, that a whatever value greater that your number of items would avoid. Would love to see the Foundation source! :) – Vincent Guerci May 25 '11 at 18:17
  • @e.James - I'm still scratching my head trying to identify the issue with code review :( Just wondering if any of the other memory management tools add any more value than Instruments Memory Leaks would? (I might create a question on this) – Greg May 25 '11 at 23:01
  • ps posted the question here - perhaps I should close this one off? – Greg May 25 '11 at 23:07
  • @Greg: closing would be fine by me. I wasn't able to help you out, so there's not much benefit to be had for future readers. – e.James May 26 '11 at 01:19
  • @e.James - actually when I said closing I actually meant "accepting" as answer as the info has helped me, however I guess whilst it has helped it hasn't answered the question. But then on the same hand short to posting my whole application code I'm not sure I could expect anyone to add any more value than you already have no? – Greg May 27 '11 at 00:15