CODEWITHSUNDEEP
iphonememory-management
phil swenson
phil swenson

Reputation: 8904

memory leak in iPhone Obj C code

I'm parsing an XML string and have a memory leak. I know this code is leaking, but not sure what the fix is:

http://pastie.org/580694

Code like this appears to be fundamentally flawed:

- (void)parser:(NSXMLParser *)parser foundCharacters:(NSString *)value{
    if ([currentElement isEqualToString:@"problem_id"]){
        currentProblem.problemId = [[value copy] intValue];
    } else if ([currentElement isEqualToString:@"rule_instance_id"]){
        currentProblem.ruleInstanceId = [value copy];
    } else if ([currentElement isEqualToString:@"description"]){
        currentProblem.desc = [value copy];
    } else if ([currentElement isEqualToString:@"name"]){
        currentProblem.name = [value copy];

but not sure how I should deal with grabbing the found characters and retaining/releasing them.

thanks

Upvotes: 1

Views: 284

Answers (2)

Oren Trutner
Oren Trutner

Reputation: 24218

In the definition of the Problem class, let the compiler deal with property memory management for you:

  • Define string properties with @property (retain) NSString *desc;. This will make sure your class increments the reference count on string values it stores (and decrement it if another value is stored later on).
  • Define int properties with @property (assign) int problemId. Ints don't need to be copied of ref-counted.

In the dealloc: method, make sure to release all the retained properties, e.g. [desc release].

Finally, you do not need to copy value before assigning to currentProblem's properties. currentProblem.desc = value will do the right thing. If you leave [value copy] in place, that would keep on leaking.

As for the code in the question, it leaks in a couple of places:

  • [[value copy] intValue] leaks each time parser:foundCharacters: is called, as the copy is never released.
  • The string members of currentProblem leak when currentProblem is released, as the current implementation of dealloc: does not release them.

Upvotes: 1

Rob Napier
Rob Napier

Reputation: 299605

currentProblem.problemId = ... is equivalent to [currentProblem setProblemId:...]. You have almost certainly declared problemId to have a retain or copy setter, so setProblemId: retains the object passed. This is normal and good.

currentProblem.desc = [value copy];

-copy is one of the three magic words, which means the returned value is retained, and then you retain it again in the setter. So you are double-retaining; memory leak.

value is an NSString, so is immutable. There is no reason to make a copy of it. Many pointers may share an immutable object safely. This code should be:

currentProblem.desc = value;

This line is a little different:

currentProblem.problemId = [[value copy] intValue];

In this case, problemId is clearly an assign property (or you would quickly crash), but you're still calling -copy on value, causing its retain count to go up by one, thus leaking it. This code should be:

currentProblem.problemId = [value intValue];

In short, stop copying the objects here and your memory leak will go away. Copying is somewhat rare in ObjC.

Upvotes: 1

Related Questions