0

In Instruments the tempPlayer object is showing a leak. In this code, in every for loop I keep on allocating a new tempPlayer instance, set its playerCode variable with a string and add it to an NSMutableArray in each iteration. Instruments shows me a leak in the alloc statement. Is there any way to prevent tat leak in the tempPlayer object ?

 for(int i = 0 ; i < [homeLineupArray count] ; i++) {   
    NSArray * tildeSeperator = [[homeLineupArray objectAtIndex:i] componentsSeparatedByString:@"~"];

    [self.tempPlayer release];
    self.tempPlayer = [[LineUpsPlayer alloc] init];
    tempPlayer.playerCode = [tildeSeperator objectAtIndex:0];

    [matchLineUp.homeTeamPlayingEleven addObject:tempPlayer ];
}

Thanks Harikant Jammi

5 Answers 5

1

I would simply do this.

for(int i = 0 ; i < [homeLineupArray count] ; i++) {   
    NSArray *tildeSeperator = [[homeLineupArray objectAtIndex:i] componentsSeparatedByString:@"~"];

    LineUpsPLayer *player = [[[LineUpsPlayer alloc] init] autorelease];
    player.playerCode = [tildeSeperator objectAtIndex:0];
    [matchLineUp.homeTeamPlayingEleven addObject:player];
}

You can also replace your loop with this:

for (NSString *lineup in homeLineupArray) {
    NSArray *tildeSeparator = [lineup componentsSeparatedByString:@"~"];
    ...
}

You don't usually want to save each item while iterating through an array to an instance variable since it keeps changing and you only reference it in the method.

1
  • 1
    The autorelease there should fix the leak - but I'd argue that it's better to explicitly release after adding to the array (although in practice its unlikely to make a big difference in this case - its more a matter of a style). Jan 21, 2010 at 8:58
0

Rather than using the property, tempPlayer, use a local variable, and release after adding to the array:

for(int i = 0 ; i < [homeLineupArray count] ; i++) {   
    NSArray * tildeSeperator = [[homeLineupArray objectAtIndex:i] componentsSeparatedByString:@"~"];

    LineUpsPlayer* tempPlayer = [[LineUpsPlayer alloc] init];
    tempPlayer.playerCode = [tildeSeperator objectAtIndex:0];

    [matchLineUp.homeTeamPlayingEleven addObject:tempPlayer ];
    [tempPlayer release];
}

The array retains the objects you add - that's why you need to release the temporary object too.

0

Things may depend on how do you declare your tempPlayer property in your class (and as it looks that it is temporary object, consider do you need a property accessor for it?)

for(int i = 0 ; i < [homeLineupArray count] ; i++) {   
    NSArray * tildeSeperator = [[homeLineupArray objectAtIndex:i] componentsSeparatedByString:@"~"];

    [self.tempPlayer release]; // Decrease retain count 
    self.tempPlayer = [[LineUpsPlayer alloc] init]; // retain count increase by 1 or 2
    tempPlayer.playerCode = [tildeSeperator objectAtIndex:0];

    [matchLineUp.homeTeamPlayingEleven addObject:tempPlayer]; // retain count increase
}

So as you can see you retain your object more times then you release it so it eventually leaks. Probably your code may be rewritten this way:

for(int i = 0 ; i < [homeLineupArray count] ; i++) {   
        NSArray * tildeSeperator = [[homeLineupArray objectAtIndex:i] componentsSeparatedByString:@"~"];

       LineUpsPlayer* tempPlayer = [[LineUpsPlayer alloc] init]; // object's retain count is 1
        tempPlayer.playerCode = [tildeSeperator objectAtIndex:0];

        [matchLineUp.homeTeamPlayingEleven addObject:tempPlayer]; // container takes ownership of the object
        [tempPlayer release]; // we do not need to own this object as it is in container now
    }
2
  • 1
    Looks like we reached the same conclusion at the same time ;-) Probably tempPlayer should be made a local variable, too. Jan 21, 2010 at 8:51
  • Yes, forgot to add tempPlayer declaration
    – Vladimir
    Jan 21, 2010 at 8:54
0

As said above, I normally use a local variable to populate the mutable array. In other cases Instruments didnt show me any leak. So is this the right way to go about ?

for(int i =0 ; i <  ([arrayOfHomePlayers count]); i++ ) //creating home players detials object
{

    localString = [NSMutableString string];
    localString = [arrayOfHomePlayers objectAtIndex:i] ;
    NSArray * localPlayersArray = [localString componentsSeparatedByString:@"~"] ;
    localPlayerPosition = [ [PlayerPosition  alloc] init] ;

    NSArray * playerNameArray = [[localPlayersArray objectAtIndex:0] componentsSeparatedByString:@" "] ;
    localPlayerPosition.globalID   = [localPlayersArray objectAtIndex:1];

    [(teamFormationDetails.homePlayerList) addObject:localPlayerPosition] ;
    [localPlayerPosition release] ;
5
  • you do not need localString = [NSMutableString string];
    – Vladimir
    Jan 21, 2010 at 9:19
  • As Vladimir says, you don't need the localString = [NSMutableString string]; - and that will cause a leak. You'll need to declare localString, though. What are you doing with playerNameArray ? Jan 21, 2010 at 9:36
  • won't [NSMutableString string] return autoreleased object?
    – Vladimir
    Jan 21, 2010 at 9:38
  • yeah, sorry, scratch that - there's no leak there - but it is redundant. Jan 21, 2010 at 9:39
  • Does the Instruments tool give out false positives sometimes? Because in all methods i have used local variables, it dint show a leak only in this function its showing leaks. Any clues? Harikant Jammi Jan 21, 2010 at 10:20
-1

First, you could change the release to an autorelease and move it to the line after the self is assigned.

Second, the addobject is adding it to a collection which retains it, and I don't see you removing it from that collection, so that's your 'leak'. However, I'm not sure there even is a leak, if you intended to leave it in the collection.

5
  • It's only a one time leak, though, it doesn't leak more each time around. I have updated my answer to fix that.
    – Alex Brown
    Jan 21, 2010 at 8:52
  • changing to an autorelease will just delay when the release happens. It won't impact the leak Jan 21, 2010 at 8:54
  • "It's only a one time leak, though" - that depends how the property is implemented - which we can't see. Also, the OP doesn't say that it leaks on every iteration - just that Instruments shows a leak at the alloc. Jan 21, 2010 at 8:55
  • I have declared the property like this @property (nonatomic, retain) LineUpsPlayer * tempPlayer ; Jan 21, 2010 at 9:02
  • @unknown - so it will continue to hold its reference after the last iteration (but the other iterations should be fine) Jan 21, 2010 at 9:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.