NSTimer and memory leaks.

Discussion in 'iOS Programming' started by XcodeNewb, Jul 27, 2009.

  1. XcodeNewb macrumors member

    Joined:
    Feb 6, 2009
    #1
    For some background, I have read the Docs multiple times but for some reason I still suck at trying to control my memory usage.

    I am setting up an NSTimer to call a method every .2 seconds. In that method I check to see if the current time on the device has changed at all and if so I walk through each element of the time and check what has changed. It could be a second, minute or both etc...

    The app runs for right about 10 minutes and crashes every time. I believe it is due to memory issues. Can anyone offer and advice on this code, or an alternate method for doing this? i believe the issues is with the variable current_time. I'm not sure exactly when to release it or re-create it.

    Thanks

    Code:
    -(void)runTimer 
    {  
       myTimer = [NSTimer scheduledTimerWithTimeInterval: 0.2  
       target: self  
       selector: @selector(showActivity)  
       userInfo: nil  
       repeats: YES];  	
    }
    
    -(void)showActivity
    {	
    	NSLog(@"showActivity");
    	NSDateFormatter *formatter = [[NSDateFormatter alloc] init];  
    	// This will produce a time that looks like "12:15:00 PM".
    	[formatter setTimeStyle:NSDateFormatterMediumStyle];  
    	tempTime = [[NSString alloc] initWithString:[formatter stringFromDate:[NSDate date]]];
    	[formatter release];
    	
    	if([tempTime length] == 10)
    	{
    		NSString *newStr = [NSString stringWithFormat:@"0%@", tempTime];
    		tempTime = [NSString stringWithFormat:@"%@", newStr];
    	}
    	
    	if(ftime==TRUE)
    	{
    	   current_time = [[NSString alloc] init];
    	   current_time = [[NSString alloc] initWithString:tempTime];		
    	}
    	
    
       NSLog(@"TT[%@] CT[%@]", tempTime, current_time);  
    	if (![current_time isEqualToString:tempTime] || ftime==TRUE)
    	{
    		int i=0;
    		for(i=0; i<[tempTime length]; i++)
    		{
    			if (([tempTime characterAtIndex:i] != [current_time characterAtIndex:i]) || ftime==TRUE)
    			{
    				.... call my method here if needed				
    			}
    		}
    	   [current_time release];
    		current_time = [[NSString alloc] initWithString:tempTime];		
    	}
    	ftime=FALSE;
    	[tempTime release];
    
    }
      
    
    
     
  2. XcodeNewb thread starter macrumors member

    Joined:
    Feb 6, 2009
    #2
    I noticed that the bulk of my memory issues comes from a method I got off the net that will round the corners of an image for you. It seems to leave many UIImage objects out in memory. Any ideas on this as well?

    Code:
    static void addRoundedRectToPath(CGContextRef context, CGRect rect, float ovalWidth, float ovalHeight)
    {
        float fw, fh;
        if (ovalWidth == 0 || ovalHeight == 0) {
            CGContextAddRect(context, rect);
            return;
        }
        CGContextSaveGState(context);
        CGContextTranslateCTM (context, CGRectGetMinX(rect), CGRectGetMinY(rect));
        CGContextScaleCTM (context, ovalWidth, ovalHeight);
        fw = CGRectGetWidth (rect) / ovalWidth;
        fh = CGRectGetHeight (rect) / ovalHeight;
        CGContextMoveToPoint(context, fw, fh/2);
        CGContextAddArcToPoint(context, fw, fh, fw/2, fh, 1);
        CGContextAddArcToPoint(context, 0, fh, 0, fh/2, 1);
        CGContextAddArcToPoint(context, 0, 0, fw/2, 0, 1);
        CGContextAddArcToPoint(context, fw, 0, fw, fh/2, 1);
        CGContextClosePath(context);
        CGContextRestoreGState(context);
    }
    
    +(UIImage *)makeRoundCornerImage : (UIImage*) img : (int) cornerWidth : (int) cornerHeight
    {
    	UIImage * newImage = nil;
    	
    	if( nil != img)
    	{
    		NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
    		int w = img.size.width;
    		int h = img.size.height;
        
    		CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
    		CGContextRef context = CGBitmapContextCreate(NULL, w, h, 8, 4 * w, colorSpace, kCGImageAlphaPremultipliedFirst);
        
    		CGContextBeginPath(context);
    		CGRect rect = CGRectMake(0, 0, img.size.width, img.size.height);
    		addRoundedRectToPath(context, rect, cornerWidth, cornerHeight);
    		CGContextClosePath(context);
    		CGContextClip(context);
        
    		CGContextDrawImage(context, CGRectMake(0, 0, w, h), img.CGImage);
        
    		CGImageRef imageMasked = CGBitmapContextCreateImage(context);
    		CGContextRelease(context);
    		CGColorSpaceRelease(colorSpace);
    		[img release];
        
    	
    		newImage = [[UIImage imageWithCGImage:imageMasked] retain];
    		CGImageRelease(imageMasked);
    		
    		[pool release];
    	}
    	
        return newImage;
    }
    
    
     
  3. moral-hazard macrumors regular

    Joined:
    Jul 27, 2009
    Location:
    Palo Alto, CA
    #3
    What exactly is the purpose of the autorelease pool in your second post? It looks like you're just allocating it and realeasing it.

    Are you familiar with the leaks utility? It will certainly show you (at the very least) how much memory you are using as time goes on while the program is running. If you are blatantly leaking blocks of memory, it will also flag them with an orange indicator so that you can click on it and see where they are coming from.

    It does NOT catch all types of leaks - for example, if you forget to release a CGContextRef, it will not tell you, but it does a great job with objective-C object leaks.

    EDIT:
    Right of the bat, it *looks* like the "makeRoundCornerImage" does not autorelease the image - try changing the last line to:
    You can retain it again in whatever function calls "makeRoundCornerImage" (so long as you release it again when you're done...). Assuming that you don't explicitly release the object that is being passed back from this image, you will always have a retain count of at least 1 on the object, and it will never get freed from memory.
     
  4. XcodeNewb thread starter macrumors member

    Joined:
    Feb 6, 2009
    #4
    Thanks for the reply. As I stated though, I received that code for rounding the image off of a forum on the net. I am not sure what the purpose of the autorelease pool is.

    i had since tried the [newImage autorelease] and it seems to be working much better.

    I am running this through Instruments and it does not show any UIImage's leaking anymore but it sure does show that I am using a large amount or Overall Bytes for all of my allocations. It increases so rapidly because the timer calls the showActivity method every .2 seconds. I am trying to make sure I get rid of all of my NSString objects.

    In Instruments does the total allocation amount reflect exactly how much memory your app is taking on the phone at that time?

    Thanks again
     
  5. moral-hazard macrumors regular

    Joined:
    Jul 27, 2009
    Location:
    Palo Alto, CA
    #5
    I looked a bit closer and noticed this:

    Code:
    newImage = [[UIImage imageWithCGImage:imageMasked] retain];
    
    That's likely the culprit. The UIImage passed back from imageWithCGImage is (I believe) autoreleased. The retain call here is fine - but you DO need to balance it with a release or autorelease call sometime in the future. As I stated above, a good time to do this would be to autorelease when you return the object. That way it will stick around in memory long enough for you to retain it again in whichever function calls the roundedImage function.
     
  6. moral-hazard macrumors regular

    Joined:
    Jul 27, 2009
    Location:
    Palo Alto, CA
    #6
    You can click on the little chart of memory usage at any given point, and a little diamond/triangle shape will pop up and tell you how much you are using (for example, 1.5MB).

    How much memory are we talking here? Is it continually increasing, or does it level off at some point?

    If you are caching alot of data to avoid redundant computation, try releasing whatever that data is in the "didReceiveMemoryWarning" function, HOWEVER, your primary concern should be making sure that you are not in fact leaking anywhere.
     
  7. moral-hazard macrumors regular

    Joined:
    Jul 27, 2009
    Location:
    Palo Alto, CA
    #7
    There are a few places I see what looks like leaky code. Example:

    Code:
    if(ftime==TRUE)
    	{
    	   current_time = [[NSString alloc] init];
    	   current_time = [[NSString alloc] initWithString:tempTime];		
    	}
    You are allocating memory for current time in the first call, then immediately leaking it by allocating new memory directly afterwards. The first alloc/init call is unnecessary. You also need to make sure that every time you call alloc/init for an object, you also release or autorelease it later.
     
  8. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #8
    In the second block of code I recommend that you remove the autorelease pool. It's pointless in this code. Also, change the line that creates the image to remove the retain. In that way the code will be returning an autoreleased UIImage. The retain is apparently there to protect the image from being reaped but I don't see any point to the autorelease pool. Also, I have no clue why it releases img.

    In your first block of code the memory management is completely muddled. I recommend you rewrite it all to use only autoreleased strings and use a property to safely retain the string that you're keeping around. Also, don't use characterAtIndex.
     
  9. moral-hazard macrumors regular

    Joined:
    Jul 27, 2009
    Location:
    Palo Alto, CA
    #9
    This is a good piece of advice. Coming from a C background i avoided using properties for a while, but they take care of alot for you.

    When you set a property (getters/setters are generated automatically with @synthesize), it automatically retains the object. If you then set it to something else (a new object with a new value), it releases the object that was already there. Other than that, you just need one final release in your dealloc method. It's alot easier then trying to balance calls to retain/release on your own, especially if you are creating new objects in conditionals. This way, if you want to allocate a new object, you can store it in a property as follows:

    Code:
    NSObject newObj = [[NSObject alloc] init];
    self.property = newObj; //this retains if the property declaration specifies it
    [newObj release];
     
  10. XcodeNewb thread starter macrumors member

    Joined:
    Feb 6, 2009
    #10
    Thanks for all of the replies. I will change the code accordingly and let you know.

    I already scraped the entire first block and rewrote it without allocating any strings at all. It runs much smoother now.

    I will change the UIImage code now.

    thanks again.

    Also, Phoney:

    If I shouldn't use characterAtIndex what else do you suggest?
     
  11. Troglodyte macrumors member

    Joined:
    Jul 2, 2009
    #11
    Isn't that all you ever do with an autorelease pool?
     
  12. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #12
    Your code that uses characterAtIndex is a bit obscured so I'm not exactly sure what it's doing. Basically it looks like you're just comparing two strings. You can just use isEqualToString: If you need more control you can use one of the compare: methods that allows you to specify a range and options.

    In general you shouldn't use characterAtIndex to break down strings into characters. Unicode strings are more complicated than simple Ascii C strings. If you need to chop up a string you should make substrings. Whenever possible work with strings not characters. Your code will be cleaner and more likely to be correct if you do.
     

Share This Page