MutableCopy AllocLeak MemoryLeak

Question

I have an NSTimer that fires once per second.

And every second I have an NSString that needs to be changed.

I've never tried to deal with memory management before so I'm not sure if what I'm doing is right but instruments is saying under "alloc" that the line of code with stringByReplacingOccurrencesOfString has 45MB of "Live Bytes" after about a minute...

(and the live byte count keeps on rising with every second and eventually crashes the app).

I think my issue lies somewhere with the MutableCopy code?

Here is my code:

-(void)myTimer {
    if (testedit) {
        [testedit release];
        [withString1a release];
        [forString1a release];
    }
    testedit = [[NSString alloc] init];
    withString1a = [[NSString alloc] init];
    forString1a = [[NSString alloc] init];

    testedit = [[NSString alloc] initWithFormat:@"example"];
    withString1a = [[NSString alloc] initWithFormat:@"e"];//this string gets its values randomly from an array in my real code
    forString1a = [[NSString alloc] initWithFormat:@"flk34j"];//this string gets its values randomly from an array in my real code

    testedit = [[testedit stringByReplacingOccurrencesOfString:withString1a withString:forString1a]  mutableCopy];//memory leak /:

}

Answer 1

You are allocating memory for each object twice. When you alloc the second time and assign it to the same variable, the first piece of alloc'd memory becomes inaccessible and unreleasable.

Then you make a mutableCopy of testedit and assign the copy to the original's variable. Again, you leave a piece of inaccessible memory floating around.

The rule with non-ARC memory management is - for every alloc, new, copy or retain you need to have a corresponding release. You have 6 allocs, one copy, and only 3 releases.

Here are some suggestions.

Remove these duplicated allocations:

   testedit = [[NSString alloc] init];
   withString1a = [[NSString alloc] init];
   forString1a = [[NSString alloc] init];

Presumably testedit, withString1a and forString1a are all iVars. (Please declare your iVars as autosynthesized properties and refer to them as self.testedit ... etc. that will make your code so much clearer to stack overflowers).

Take out all of this:

if (testedit) {
        [testedit release];
        [withString1a release];
        [forString1a release];
    }

Assuming these are all iVars, the correct place to release them is in your object's dealloc method

In fact withString1a and forString1a can be local variables, as you get their content from elsewhere:

 NSString*  withString1a = [[[NSString alloc] initWithFormat:@"e"] autorelease];
 NSString*  forString1a =  [[[NSString alloc] initWithFormat:@"flk34j"] autorelease];

You can autorelease them as you don't need them to hang around after the method has finished.

These lines can also be written:

 NSString*  withString1a = [NSString stringWithFormat:@"e"];
 NSString*  forString1a =  [NSString stringWithFormat:@"flk34j"];

(-stringWithFormat is a convenience method that returns an autoreleased object)

That leaves us with these two lines.

  testedit = [[NSString alloc] initWithFormat:@"example"];
  testedit = [[testedit stringByReplacingOccurrencesOfString:withString1a 
                                                  withString:forString1a]  mutableCopy];

It's not clear why you are treating testedit as an immutable string in the first line and a mutable string in the second. You don't need a mutable string here at all, as you are replacing testedit with a new string.

 self.testedit = [[NSString alloc] initWithFormat:@"example"];
 self.testedit = [[testedit stringByReplacingOccurrencesOfString:withString1a 
                                                  withString:forString1a] copy]; 

(you need copy as stringByReplacingOccurrencesOfString:withString: returns an autoreleased object, and here you want to keep hold of it)

THE last piece of the jigsaw is getting rid of your _testedit iVar memory allocation. You do this in the dealloc method of your object:

- (void) dealloc {
    [_testEdit release];
    [super dealloc];
}

(Note that init, accessor, and dealloc methods are the three places where you should not refer to an iVar using property syntax.)

All good, but really, you should be using ARC! You are _far_more likely to introduce memory bugs this way than if you rely on the compiler to manage memory for you.

Answer 2

You are getting a memory leak because you never de-allocate testedit. Whenever you call alloc, that means you need to deallocate it. This usually just means calling release.

Do something like this instead, then be sure to free up the memory you've allocated:

NSString* newString = [[testedit stringByReplacingOccurrencesOfString:withString1a withString:forString1a]  mutableCopy];

Answer 3

If you're using ARC you shouldn't have an issue. If you aren't using ARC you can try adding autorelease:

testedit = [[[testedit stringByReplacingOccurrencesOfString:withString1a withString:forString1a]  mutableCopy] autorelease];

Answer 4

I would suggest you to make use of @property here.

In .h file declare the properties as:

@property (nonatomic, retain) NSString *testedit;
@property (nonatomic, retain) NSString *withString1a;
@property (nonatomic, retain) NSString *forString1a; //if required write the @synthesize as well in .m class

You can write your timer method as:

-(void)myTimer {

    self.testedit = @"example";
    self.withString1a = @"e";//this string gets its values randomly from an array in my real code
    self.forString1a = @"flk34j";//this string gets its values randomly from an array in my real code
    self.testedit = [self.testedit stringByReplacingOccurrencesOfString:self.withString1a withString:self.forString1a];
}

In dealloc method, you can set all the above properties as nil (self.testedit = nil;) or do a release on them([testedit release];).

If possible, try to switch to ARC, you dont have to worry about the memory management. The problem with your code was that you are using a lot of alloc/init statements without releasing the variable before doing it. This causes it to lose the reference of that variable and you will leak it. You dont need that many allocation statements. For every allocation or retain, there should be a corresponding release/auto-release statement.