iphoneiosobjective-cmemory-managementnsmutablecopying

MutableCopy AllocLeak MemoryLeak


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 /:

}

Solution

  • 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.