NSMutableArray passing by parameter memory leak

▲
2
▼

I'm experiencing a hard issue here, would appreciate any, I mean ANY help =)

I'm a experienced developer by I'm new to Objective-C/iPhone/Cocoa.

I wanna create a class controller which I'm able to pass a NSMutableArray as a parameter.

Then, we have:

selTimeIntController = [[SingleSelectPickerViewController alloc] initWithSettings: listOfIntervals :kAlarmIntervalStr :myDataHolder.alarmInterval];
[self.navigationController pushViewController: selTimeIntController animated: YES];
[selTimeIntController release];

where this listOfIntervals is an 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'm able to see my listOfIntervals being created on my SingleSelectPickerViewController.

Here we have the SingleSelectPickerViewController' dealloc:

- (void)dealloc {
    [super dealloc];

    [listOfIntervals release];
    [currentValue    release];
    [title           release];
}

But, everytime I instantiate my SingleSelectViewController, I receive right afterwards an 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 of what's going on?

objective-c
cocoa-touch
memory-leaks
nsmutablearray
asked on Stack Overflow Sep 14, 2009 by Cadu • edited Sep 14, 2009 by mipadi

4 Answers

▲
7
▼

The title of the question says "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 the crash is the incorrect management of the listOfIntervals instance variable.

listOfIntervals = [NSMutableArray arrayWithArray: (NSMutableArray*)sourceArray];

Specifically, this needs to be:

listOfIntervals = [[NSMutableArray arrayWithArray: sourceArray] retain];

As Mike indicated above, passing around a mutable collection reference is probably a bad idea. What happens if sourceArray changes out from under your class? Are you prepared to deal with that?

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) The (NSMutableArray*) cast was unnecessary. Did know harm, but why have it if it isn't needed?

(2) You need to retain listOfIntervals. +arrayWithArray: will create an autoreleased array and, thus, the array was being released after the object was initialized, which would lead to the crash you see.

(3) -copy & -mutableCopy return retained objects, no need to call -retain.

However, you also need to fix your -dealloc method:

- (void)dealloc {
    // move this [super dealloc];

    [listOfIntervals release];
    [currentValue    release];
    [title           release];
    [super dealloc]; // to here
}

The [super dealloc] must always be last. There is nothing magic about -dealloc and, thus, by having that call first, you were telling the instance to deallocate itself and then going through and cleaning up the instance variables. That would have led to a second crash (or unexpected behavior).

Overall, I would suggest you re-read the memory management guidelines.

http://developer.apple.com/iPhone/library/documentation/Cocoa/Conceptual/MemoryMgmt/MemoryMgmt.html

answered on Stack Overflow Sep 14, 2009 by bbum
▲
0
▼

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 that, altough I don't think that this will solve your problem.

Also I don't get what you are doing here:

if(self = [self initWithNibName: kNibName bundle: [NSBundle mainBundle]]) {
    //...
}

I think that should be:

if ( ! [super initWithNibName: kNibName bundle: [NSBundle mainBundle]] ) {

       return nil;
}

//...
answered on Stack Overflow Sep 14, 2009 by André Hoffmann • edited Sep 14, 2009 by André Hoffmann
▲
0
▼
  1. What about sourceArray - is it retained somewhere? You have to retain allocated object in order to use it beyond its scope, otherwise it's autoreleased. You can use the simple reference to it in your class without creating the array again.

Or, you can

listOfIntervals = [[NSMutableArray arrayWithArray: (NSMutableArray*)sourceArray] retain];

and then use it and release it.

  1. The answer above has noticed, that you call the [super dealloc] before releasing all your class allocations. [super dealloc] should be called in the end.

  2. There are plenty of useful links about Cocoa memory management, especially about usage of alloc/retain functions. This is really essential part of Cocoa/iPhone programming. See this one for example: Memory management in Cocoa or just google for it

Hope it helps, good luck

answered on Stack Overflow Sep 14, 2009 by Nava Carmon
▲
0
▼

You need to go back to basics on this one.

Problem 1: You are passing around a mutable foundation object.

This is almost always indicative of bad design. Look at Cocoa/CocoaTouch and you'll see very few uses of mutable classes being passed as parameters or returned. Doing so is almost always because of performance constraints.

Why is it bad? Because after making the call, it's quite easy to end up with 2 or more objects all sharing the same mutable object. If one makes a change to it, the others have no way of knowing about it, potentially causing some very strange behaviours further down the line. NOT fun to debug.

Problem 2: You are not retaining the array

This is a memory management fundamental and you absolutely MUST understand it before trying to dig yourself in too much. Apple can probably explain it better than I, but it boils down to:

If you need access to an object outside of the scope where you were given it, retain it. Release it when you are done later.

So what you're doing here is assigning an autoreleased array into the listOfIntervals instance variable, and of course it gets released & deallocated later, blowing up your app when you next 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;
}

Points of note:

  • This method is properly named. It has clear names for each argument.
  • There's no need to call [NSBundle mainBundle]. As the documentation states, NSViewController will figure this out for itself.
  • All the arguments passed in are copied. This has two important effects:
    • NSArray and NSString are value objects. That is, your code is interested in their value, not the actual object itself. Cocoa will nicely take care of making this as efficient as possible.
    • -copy and -mutableCopt both return objects with a +1 retain count, so they won't be going anywhere until you release them. Perfect for a typical instance variable.
answered on Stack Overflow Sep 14, 2009 by Mike Abdullah

User contributions licensed under CC BY-SA 3.0