0

I created a custom NSString Category which lets me find all strings between two other strings. I'm now running into the problem of finding that there are a lot of kBs leaking from my script. Please see code below:

    #import "MyStringBetween.h"

@implementation NSString (MyStringBetween)

-(NSArray *)mystringBetween:(NSString *)aString and:(NSString *)bString;
{
    NSAutoreleasePool *autoreleasepool = [[NSAutoreleasePool alloc] init];

    NSArray *firstlist = [self componentsSeparatedByString:bString];
    NSMutableArray *finalArray = [[NSMutableArray alloc] init];


    for (int y = 0; y < firstlist.count - 1 ; y++) {
        NSString *firstObject = [firstlist objectAtIndex:y];
        NSMutableArray *secondlist = [firstObject componentsSeparatedByString:aString];
        if(secondlist.count > 1){

            [finalArray addObject:[secondlist objectAtIndex:secondlist.count - 1]];
        }
    }

    [autoreleasepool release];

    return finalArray;
}
@end

I admit that I'm not super good at releasing objects, but I had believed that the NSAutoreleasePool handled things for me.

The line that is leaking:

NSMutableArray *secondlist = [firstObject componentsSeparatedByString:aString];

Manually releasing secondlist raises an exception.

Thanks in advance!

Daniel Amitay
  • 6,677
  • 7
  • 36
  • 43
  • http://stackoverflow.com/questions/65427/how-does-the-nsautoreleasepool-autorelease-pool-work – Jason Rogers Nov 15 '10 at 06:51
  • Btw, your function returns an array instead of a string. – Mihai Damian Nov 15 '10 at 07:14
  • Also, unless you're calling this method on a thread without an autorelease pool encapsulating it, you shouldn't need to make a new autorelease pool just for this method. If you're calling it on the main thread, you've likely got one set up for you. – Sam Nov 15 '10 at 07:26

2 Answers2

3

No, this is the line that is leaking:

NSMutableArray *secondlist = [[NSMutableArray alloc] init];

And it isn't that big of a leak (just an empty mutable array). Still, don't do that.

In particular, the line:

    secondlist = [[firstlist objectAtIndex:y] componentsSeparatedByString:aString];

Is assigning over the reference to the empty mutable array.

Also FinalArray should be named finalArray.

bbum
  • 162,346
  • 23
  • 271
  • 359
  • Please view my new code. The same secondlist line is being marked by instruments as leaking. By "big leak", in my own project, it is causing a 40kB leak. – Daniel Amitay Nov 15 '10 at 07:08
  • It's not a just an empty array — objects are added later. It will also leak all those objects since the array retains them. And `finalArray` needs to be autoreleased too. – Chuck Nov 15 '10 at 07:13
  • @Chuck Stylistically, finalArray should absolutely be autoreleased. However, the author could be releasing it outside the method (although they may not be.) – Sam Nov 15 '10 at 07:17
1

finalArray is leaking. You should autorelease it before returning it but make sure you do it either before allocating the autorelease pool or after releasing it.

JeremyP
  • 84,577
  • 15
  • 123
  • 161