Objective-C memory management, xml parser and other non-trivial examples

Question

I know the basic principles about memory management (retain count, autorelease pools etc) in Cocoa, but once you go past simple retain/release, it's getting a bit more confusing. I couldn't find decent answers for those, as most tutorials cover simple scenarios. I would like to ask about best practices in how to write the code and avoid leaks.

---

1st question - iterations and temporary assignments:

for (id object in objectArray) {    
     Model *currentItem = object;
    /* do something with currentItem */
    [currentItem release];
}

If I remove the release in last line the code will work fine, but there's leak. What's the rule here? The object is already there in objectArray. I'm assigning it directly, to get the type. Should I do this some other way? Is this assignment increasing retainCount of currentItem? (is it something like [[alloc] initWithObject] ?) How to know if this assignment (object) is autoreleased or not?

---

2nd question - instant retains :

Model *model = [unarchiver decodeObjectForKey:@"ARCHIVED_MODEL_OBJECT"];
// it has to be here, because (I was told) unarchiver will return autorelease object    
[model retain]; 
label.text = model.data;

How someone knew that this particular method works so weird, that I need to instantly call retain on returned value, or I will bump into null on next assignment? I couldn't find anything like this in documentation. According to retain/release rules, I would expect that decodeObjectForKey returns autorelased object, but it will take some time until the control goes back to app and pool claims the model object to be released. Is there any rule for that? How should I search for those?

---

3rd question - autoreleasing and passing variables:

- (IBAction) loadXMLButtonClicked:(id) sender {
    objectArray = [self loadData]; // 1 - objectArray is instance var
    NSArray *objectArray = [self loadData]; // 2 - objectArray is local var

    // loadXMLButtonClicked is called on button click, here the method finishes
    // and control goes back to application, autorelease pool is cleaned?

    // case 1 - objectArray stays retained in instance variable? (because setter was used)
    // case 2 - objectArray is soon to be released, there were no retains? 
    // (ignore the fact that it's local var, just hypothetically)

}

- (NSArray *) loadData {
    NSArray *objectArray = [[NSArray alloc] init];
    // populate array here
    return [objectArray autorelease];
}

---

4th question - (bear with me, last one) parser xml best practice : (I don't want to use other solutions, using standard parser is to practice objective-c memory management and flow)

Basically this code here works, works good and have no leaks, but I don't really know if this is proper approach. I have separate object that acts as parser, parses an XML to collect an array of objects of type Model. Now, after parsing done, externally I would like to obtain that array, though I don't know how (is copying the array and releasing whole parser good idea?). Please run through the code and see the comments.

I've debug this code many times, using gdb for printing retainCounts, zombies, etc. and while I can manage to get this running and without leaks, I don't know 100% why and would like to hear a good reasoning how should this be done with explanation. That would be much appreciated.

Controller.m

- (NSArray *) loadData {
(...)
NSXMLParser *parser = [[NSXMLParser alloc] initWithData:data];
ModelXMLParser *parserDelegate = [[ModelXMLParser alloc] init];
[parser setDelegate:parserDelegate];
[parser parse];

objectArray = [[parserDelegate objectArray] copy]; 
// is this ok? *i* don't need the parser object so I think I should get rid of it
// and copy the data. How this copy works, is it shallow (only new reference to array)
// or deep copy (objects allocated again as well)?
// how to do deep copy of NSArray?

[parserDelegate release];
[parser release];
}

ModelXMLParser.m (simplified)

@implementation ModelXMLParser

@synthesize objectArray; // array of objects
@synthesize currentObject; // temporary object
@synthesize currentChars; // temporary chars
@synthesize parseChars; // parse chars only when there's need, leave those /t/n etc

- parser didStartElement (...) {
    if ([elementName isEqualToString:@"objects"]) {
        objectArray = [[NSMutableArray alloc] init];
    }
    else if ([elementName isEqualToString:@"object"]) {
        currentObject = [[Model alloc] init];
    }
    else if ([elementName isEqualToString:@"name"]) {
        // do I have to init currentObject.name (NSString) here? I guess not
        [self setParseChars:YES]; // just set the flag to make parse control easier
    }
    else if ([elementName isEqualToString:@"number"]) {
        // int isn't object anyway, no init
        [self setParseChars:YES]; // just set the flag to make parse control easier
    }   
}

- parser foundCharacters (...) {
    if (parseChars) {
        currentChars = [[NSString alloc] initWithString:string];
        // why is currentChars retainCount = 2 here?
        // is it like currentChars = [NSString new] and then currentChars = string? (so retain once more)
        // is it good way to control parser? (please ignore the NSMutableString and appending example, try this one)
        // should I just do currentChars = string here?

        [currentChars autorelease]; // this is currently my solution, because there's no leak, but I feel it's incorrect
    }
}

-  parser didEndElement (...) {
    if ([elementName isEqualToString:@"object"]) {
        [objectArray addObject:[currentObject copy]]; // should I copy here or just addObject, it retains anyway?
        [currentObject release]; // I've initialized currentObject before, now I don't need it, so I guess retainCount goes to 0 here?
    }
    else if ([elementName isEqualToString:@"name"]) {
        currentObject.name = currentChars; // is this correct, or shoud I do [currentChars copy] as well?
        [self setParseChars:NO];
        [currentChars release]; // as before, initialized, now releasing, but is this really correct?
    }
    else if ([elementName isEqualToString:@"number"]) {
        currentObject.number = [currentChars intValue]; // is this correct, or shoud I do [currentChars copy] as well?
        [self setParseChars:NO];
        [currentChars release]; // as before, initialized, now releasing, but is this really correct?
    }   
}

- (void) dealloc {
    // I shouldn't release currentChars or currentObject, those (I suppose) should be freed after parsing done,
    // as a result of earlier releases?

    [objectArray release];
    [super dealloc];
}

Model.m

@implementation Model

@synthesize name; // this is NSString
@synthesize number; // this is int

- (id) copyWithZone:(NSZone *) zone {
    Model *copy = [[[self class] allocWithZone:zone] init];
    copy.name = [self.name copy];
    copy.number = self.number;
    return copy;
}

- (void) dealloc {
    [name release];
    // I don't have to release int, right? it's not an object
    [super dealloc];
}

I'm especially confused with question 4. Sorry for maybe too long question, but this really is about one topic and deeper understanding of it.

Answer 1

1st question - iterations and temporary assigments

You should NOT be releasing the objects here since you are not an owner. You should only ever release an object if you are an owner of it. See the Memory Management Guide for Cocoa. You are only an owner of an object if you call a method whose name begins with init, new, or contains copy in its name.

Since the for loop does not use methods with any of those names, you are not an owner, so you MUST NOT release the objects. This will result in freeing objects before they're done, which will almost certainly result in memory corruption and a crash.

2nd question - instant retains:

You don't need to instantly call retain, you just need to call it before the autorelease pool next empties. This will probably be shortly after your method returns to the main event loop. Since you don't know exactly when this will happen, you have to make sure that if you want to be able to access the object after the function (loadXMLButtonClicked: in this case) returns, then you must retain it before you return.

Since decodeObjectForKey does not begin with init or new or contain copy in its name, you are not becoming an owner. Calling retain makes you an owner.

3rd question - autoreleasing and passing variables:

First of all, it's bad practice to shadow a class member with a local variable of the same name. Secondly, unless loadData is being used as a multi-purpose utility function (which I'm guessing it isn't since it doesn't take any parameters), it should just assign the result directly to the member variable objectArray. It's pointless and error-prone to return the result and then have the calling function assign to the member variable.

Thirdly, you're not using the objectArray property setter -- you're just assigning straight to the member variable. If you want to use the setter, you have to explicitly say self.objectArray = ... (with the self. in front of it). Thus, objectArray is never retained, so it's going to be deallocated the next time the autorelease pool clears itself out, which is not what you want. You must retain it at some point, or conversely, just don't autorelease it at the end of loadData, and assign it the class member variable objectArray.

If the property is declared with the retain attribute, then using the setter will automatically call retain when you assign with it (and it will also release the old value). If instead the property is declared with the copy attribute, then the value will instead be copied every time you assign to it, and you will become an owner of the new object.

4th question - parser xml best practice:

You're making a shallow copy of the object array. If you want to make a deep copy, you can use the initWithArray:copyItems: message.

do i have to init currentObject.name (NSString) here? i guess not?

I don't understand this question, there's no currentObject.name mentioned at all anywhere nearby that code.

why is currentChars retainCount = 2 here?

Probably because during its internal initialization process, it was retained an extra time somewhere, but it was also almost certainly autoreleased an extra time as well. If you follow all of the rules from the Memory Management Guide for Cocoa, you won't have any problems. You should never rely on retain counts, since you don't know how many times an object has been autoreleased. They're a debugging aid, not something that should be used for program control flow.

this is currently my solution, because there's no leak, but i feel it's incorrect?

If you won't need to use currentChars the next time you return to the event loop, then it's fine. If you will need to use it, you should not release or autorelease it here, and then release it when you're sure you're done with it.

should I copy here or just addObject, it retains anyway?

Just addObject: when you add items into an NSArray, NSSet, or NSDictionary, they are automatically retained by the data structure. When you remove them, they're released.

Most of the rest of the questions can be answered by just following the rules or have identical answers to some of the previous questions which I've already answered.

Answer 2

@1st question

You only get a reference to the object in the objectArray. It is still in the objectArray which also has the object retained and releasing it is not good here because you didn't do anything that retained it.

See here for some rules