NSMutableArray going through a parameter memory leak
I have a hard problem, I would appreciate any, I mean ANY help =)
I am an experienced developer, I am new to Objective-C / iPhone / Cocoa.
I want to create a class controller that I can pass as an NSMutableArray parameter.
Then we have:
selTimeIntController = [[SingleSelectPickerViewController alloc] initWithSettings: listOfIntervals :kAlarmIntervalStr :myDataHolder.alarmInterval];
[self.navigationController pushViewController: selTimeIntController animated: YES];
[selTimeIntController release];
where this listOfIntervals is already alloc / init NSMutableArray *.
on my SingleSelectPickerViewController, we have:
-(id)initWithSettings:(NSMutableArray*)sourceArray :(NSString*)viewCurrentValue :(NSString*)viewTitle {
if(self = [self initWithNibName: kNibName bundle: [NSBundle mainBundle]]) {
listOfIntervals = [NSMutableArray arrayWithArray: (NSMutableArray*)sourceArray];
currentValue = [[NSString alloc] initWithString: viewCurrentValue];
title = [[NSString alloc] initWithString: viewTitle];
}
return self;
}
Through debugging, I can see that my listOfIntervals are being created on my SingleSelectPickerViewController.
Here we have SingleSelectPickerViewController 'dealloc:
- (void)dealloc {
[super dealloc];
[listOfIntervals release];
[currentValue release];
[title release];
}
But every time I instantiate SingleSelectViewController I immediately get EXEC_BAD_ADDRESS with the following stack:
#0 0x96132688 in objc_msgSend ()
#1 0x00003ee2 in -[SingleSelectPickerViewController tableView:numberOfRowsInSection:] (self=0xd38940, _cmd=0x319a6bc0, tableView=0x102e000, section=0) at /Users/Cadu/iPhone/myApp/Classes/SingleSelectPickerViewController.m:115
#2 0x30a86bb4 in -[UISectionRowData refreshWithSection:tableView:tableViewRowData:] ()
#3 0x30a8879b in -[UITableViewRowData rectForFooterInSection:] ()
#4 0x30a883c7 in -[UITableViewRowData heightForTable] ()
#5 0x3094e8e6 in -[UITableView(_UITableViewPrivate) _updateContentSize] ()
#6 0x30940a7d in -[UITableView noteNumberOfRowsChanged] ()
#7 0x3094a2a0 in -[UITableView reloadData] ()
#8 0x30947661 in -[UITableView layoutSubviews] ()
#9 0x00b41d94 in -[CALayer layoutSublayers] ()
#10 0x00b41b55 in CALayerLayoutIfNeeded ()
#11 0x00b413ae in CA::Context::commit_transaction ()
#12 0x00b41022 in CA::Transaction::commit ()
#13 0x00b492e0 in CA::Transaction::observer_callback ()
#14 0x30245c32 in __CFRunLoopDoObservers ()
#15 0x3024503f in CFRunLoopRunSpecific ()
#16 0x30244628 in CFRunLoopRunInMode ()
#17 0x32044c31 in GSEventRunModal ()
#18 0x32044cf6 in GSEventRun ()
#19 0x309021ee in UIApplicationMain ()
#20 0x000020d8 in main (argc=1, argv=0xbffff0b8) at /Users/Cadu/iPhone/MyApp/
Any idea what is going on?
source to share
The title of the question reads "memory leak". Everything in the question indicates "crasher". This is a crasher, not a memory leak. Or at least you won't know if you have a memory leak or not until you fix the crashers.
The most likely source of failure is mismanaging the listOfIntervals instance variable.
listOfIntervals = [NSMutableArray arrayWithArray: (NSMutableArray*)sourceArray];
In particular, it should be:
listOfIntervals = [[NSMutableArray arrayWithArray: sourceArray] retain];
As Mike pointed out above, going around a mutable collection reference is probably bad. What happens if sourceArray
your class changes? Are you ready to handle this?
A more common idiom would be to declare your method as taking an NSArray * and then copy the array:
listOfIntervals = [sourceArray mutableCopy]; // or -copy, if you don't need it to be mutable
(1) (NSMutableArray*)
No order needed. He knew the harm, but why should he if he is not needed?
(2) You need to keep the listOfIntervals. + arrayWithArray: Will create an auto-implemented array and thus the array will be released
after the object is initialized, which will cause the crash you are seeing.
(3) -copy and -mutableCopy return saved objects, no need to call -retain.
However, you also need to fix your method -dealloc
:
- (void)dealloc {
// move this [super dealloc];
[listOfIntervals release];
[currentValue release];
[title release];
[super dealloc]; // to here
}
[super dealloc]
should always be last. There is nothing magic about -dealloc, and thus by having this call first, you were telling the instance to free itself and then go through and clear the instance variables. This would lead to a second crash (or unexpected behavior).
Overall, I would suggest that you reread the memory management guides.
http://developer.apple.com/iPhone/library/documentation/Cocoa/Conceptual/MemoryMgmt/MemoryMgmt.html
source to share
I'm new to Mac programming, but I think your dealloc method is in the wrong order.
It should be:
- (void)dealloc {
[listOfIntervals release];
[currentValue release];
[title release];
[super dealloc];
}
You should fix this, although I don't think it will solve your problem.
Also I don't understand what you are doing here:
if(self = [self initWithNibName: kNibName bundle: [NSBundle mainBundle]]) {
//...
}
I think it should be:
if ( ! [super initWithNibName: kNibName bundle: [NSBundle mainBundle]] ) {
return nil;
}
//...
source to share
- How about sourceArray - is it stored somewhere? You must save the selected object to use it outside of its scope, otherwise it will auto-implement. You can use a simple reference to it in your class without creating the array again.
Or you can
listOfIntervals = [[NSMutableArray arrayWithArray: (NSMutableArray *) sourceArray] save];
and then use it and release it.
-
The answer above noted that you call [super dealloc] before freeing all your class allocations. [super dealloc] should be called at the end.
-
There are many useful links about Cocoa memory management, especially about using the alloc / keep functions. This is a really important part of Cocoa / iPhone programming. See this for example: Memory Management in Cocoa or just Google for it
Hope this helps, good luck.
source to share
You need to get back to the basics of this.
Problem 1: you are walking around a mutable base object .
This is almost always indicative of poor design. Take a look at Cocoa / CocoaTouch and you will see very little use of mutable classes passed as parameters or returned. This is almost always due to performance limitations.
Why is that bad? Because after making the call, it's pretty easy to end up with two or more objects that use the same mutable object. If someone makes a change to it, others are unaware of it, potentially causing some very strange behaviors further down the line. NOT fun to debug.
Problem 2: you are not storing the array
This is fundamental memory management and you absolutely MUST understand it before trying to internalize yourself too hard. Perhaps Apple can explain this better than I can, but it boils down to this:
If you need access to an object outside of the scope you were provided with, save it. Let it go when you're done later.
So what you are doing here is assigning the auto-implemented array to an instance variable listOfIntervals
, and of course it is freed and freed later, blowing up your application when you try to access it. Instead, here's the correct code:
- (id)initWithIntervals:(NSArray *)sourceArray
currentValue:(NSString *)viewCurrentValue
title:(NSString *)viewTitle
{
if (self = [self initWithNibName:kNibName bundle:nil])
{
listOfIntervals = [sourceArray mutableCopy];
currentValue = [viewCurrentValue copy];
title = [viewTitle copy];
}
return self;
}
Viewpoints:
- This method is correctly named. It has clear names for each argument.
- No need to call
[NSBundle mainBundle]
. As the documentation states, the NSViewController will determine this for itself. - All passed arguments are copied. This has two important effects:
-
NSArray
andNSString
- value objects. That is, your code is interested in their meaning, not the object itself. Cocoa will take great care to make this as effective as possible. -
-copy
and-mutableCopt
return objects with a persistence of +1, so they won't go anywhere until you release them. Perfect for a typical instance variable.
-
source to share