Memory Leak

Discussion in 'iOS Programming' started by hotkarl, Dec 1, 2010.

  1. hotkarl macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #1
    I'm trying to prevent a memory leak. I have the code below but it crashes the phone when I place [theAudio release]; in there. If I take it out everything works fine but then it leaks. Can anyone see what is the problem?

    .h
    Code:
    #import "FlipsideViewController.h"
    #import <AVFoundation/AVAudioPlayer.h>
    
    @interface MainViewController : UIViewController <FlipsideViewControllerDelegate> {
    	NSString *titleForButton;
    	NSString *imagePath;
    	NSString *musicPath;
    	UIImageView *imageView;
    	UIScrollView *scrollView;
    	AVAudioPlayer *theAudio;
    	AVAudioPlayer *newAudio;
    	NSURL *fileURL;
    	UIButton *playBtn;
    }
    
    @property (nonatomic, retain) IBOutlet UIScrollView *scrollView;
    @property (nonatomic, retain) IBOutlet UIButton *playBtn;
    @property (nonatomic, retain) AVAudioPlayer *theAudio;
    
    - (IBAction)showInfo:(id)sender;
    - (IBAction) selectLick:(id)sender;
    - (IBAction) playLick: (id)sender;
    
    @end
    
    .m
    Code:
    #import "MainViewController.h"
    
    @implementation MainViewController
    
    @synthesize scrollView;
    @synthesize theAudio;
    @synthesize playBtn;
    
    -(IBAction) selectLick:(id)sender {
    	titleForButton = [sender titleForState: UIControlStateNormal];
    	imagePath = [[NSString alloc] initWithFormat:@"%@.png", titleForButton];
    	imageView = [[UIImageView alloc] initWithImage:[UIImage imageNamed:imagePath]];
    	[imagePath release];
        [scrollView setCanCancelContentTouches:NO];
        scrollView.clipsToBounds = YES; 
        scrollView.indicatorStyle = UIScrollViewIndicatorStyleWhite;
    	[[scrollView subviews] makeObjectsPerformSelector:@selector(removeFromSuperview)];
    	[scrollView addSubview:imageView];
        [scrollView setContentSize:CGSizeMake(imageView.frame.size.width, imageView.frame.size.height)];
    	[imageView release];
        [scrollView setScrollEnabled:YES];
    	[newAudio stop];
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    	musicPath = [[NSBundle mainBundle] pathForResource:titleForButton ofType:@"mp3"];
    	fileURL = [[NSURL alloc] initFileURLWithPath: musicPath];
    	theAudio = [[AVAudioPlayer alloc] initWithContentsOfURL: fileURL error: nil];
    	newAudio = theAudio;
    	[COLOR="Red"][theAudio release];[/COLOR]
    	[fileURL release];
    	[newAudio setDelegate: self];
    	[newAudio prepareToPlay];
    }
    
    -(IBAction) playLick: (id) sender {
    	if (newAudio.isPlaying) {
    		[newAudio pause];
    		[sender setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    	}
    	else {
    		[newAudio play];
    		[sender setTitle:@"Pause" forState:(UIControlState)UIControlStateNormal];
    	}
    }
    
    - (void) audioPlayerDidFinishPlaying:(AVAudioPlayer *)newAudio successfully:(BOOL)flag 
    { 
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    }
     
  2. wlh99 macrumors 6502

    Joined:
    Feb 7, 2008
    #2
    I'm still not very good at this, but I'll throw out my suggestion. Don't release theAudio where you currently do.

    But, it still needs to be released before you create another theAudio, which will happen when that method is called again.

    To prevent that, earlier in the selectLick method check if theAudio is non nil. If it is non-nil, then release it.
     
  3. dejo Moderator

    dejo

    Staff Member

    Joined:
    Sep 2, 2004
    Location:
    The Centennial State
    #3
    And remember, it's okay to call release (or any other method) on a nil.
     
  4. vocaro macrumors regular

    Joined:
    Mar 5, 2004
    #4
    Yeah, that's because you allocate the object and then immediately release it. The "newAudio = theAudio" assignment statement does nothing to preserve the memory.
     
  5. hotkarl thread starter macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #5
    Well guys, I tried testing if theAudio was nil and moved it up in the action. It seems to make no difference rather it's nil or not. By moving [theAudio release] up higher in the action it makes it so the program does not crash until another button is pressed. Anyone got any clues here?
     
  6. hotkarl thread starter macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #6
    Ok, then I'm clearly on the wrong path. How can I release theAudio ?
     
  7. dejo Moderator

    dejo

    Staff Member

    Joined:
    Sep 2, 2004
    Location:
    The Centennial State
    #7
    You've defined a property and accessors for theAudio. It's important then to make sure you use them instead of referencing the instance variable. That is, self.theAudio and not just theAudio. I find it helps if you name the related ivar slightly differently, such as _theAudio and then have your property synthesized like so:
    Code:
    @synthesize theAudio = _theAudio;
    Then you release your ivar in the dealloc method.
     
  8. wlh99 macrumors 6502

    Joined:
    Feb 7, 2008
    #8
    I saw a similar convention in some source just today and wondered why. I was also curious about the @synthesize statement. Now it make lots of sense.

    So the ivar is really named _theAudio, but the accessors methods are still named without the underscore. So you access the ivar with self.theAudio and can't accidently use theAudio by itself. Nice.

    Thanks for that.
     
  9. dejo Moderator

    dejo

    Staff Member

    Joined:
    Sep 2, 2004
    Location:
    The Centennial State
    #9
    Yeah, and if you write code that just accesses theAudio, you get a compile-time warning. Much preferred to some obscure run-time issue.
     
  10. hotkarl, Dec 1, 2010
    Last edited: Dec 1, 2010

    hotkarl thread starter macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #10
    Like this? It works but it's still leaking. I'm trying to make sense of what you said but I'm really new. Did I miss something?

    Code:
    //
    //  MainViewController.m
    
    #import "MainViewController.h"
    
    @implementation MainViewController
    
    @synthesize scrollView;
    @synthesize theAudio = _theAudio;
    @synthesize playBtn;
    
    -(IBAction) selectLick:(id)sender {
    	titleForButton = [sender titleForState: UIControlStateNormal];
    	imagePath = [[NSString alloc] initWithFormat:@"%@.png", titleForButton];
    	imageView = [[UIImageView alloc] initWithImage:[UIImage imageNamed:imagePath]];
    	[imagePath release];
        [scrollView setCanCancelContentTouches:NO];
        scrollView.clipsToBounds = YES; 
        scrollView.indicatorStyle = UIScrollViewIndicatorStyleWhite;
    	[[scrollView subviews] makeObjectsPerformSelector:@selector(removeFromSuperview)];
    	[scrollView addSubview:imageView];
        [scrollView setContentSize:CGSizeMake(imageView.frame.size.width, imageView.frame.size.height)];
    	[imageView release];
        [scrollView setScrollEnabled:YES];
    	[self.theAudio stop];
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    	musicPath = [[NSBundle mainBundle] pathForResource:titleForButton ofType:@"mp3"];
    	fileURL = [[NSURL alloc] initFileURLWithPath: musicPath];
    	self.theAudio = [[AVAudioPlayer alloc] initWithContentsOfURL: fileURL error: nil];
    	[fileURL release];
    	[self.theAudio setDelegate: self];
    	[self.theAudio prepareToPlay];
    }
    
    -(IBAction) playLick: (id) sender {
    	if (self.theAudio.isPlaying) {
    		[self.theAudio pause];
    		[sender setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    	}
    	else {
    		[self.theAudio play];
    		[sender setTitle:@"Pause" forState:(UIControlState)UIControlStateNormal];
    	}
    }
    
    - (void) audioPlayerDidFinishPlaying:(AVAudioPlayer *)theAudio successfully:(BOOL)flag 
    { 
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    }
    /*
    // Implement viewDidLoad to do additional setup after loading the view, typically from a nib.
    - (void)viewDidLoad {
    	[super viewDidLoad];
    }
    */
    
    - (void)flipsideViewControllerDidFinish:(FlipsideViewController *)controller {
        
    	[self dismissModalViewControllerAnimated:YES];
    }
    
    - (IBAction)showInfo:(id)sender {    
    	
    	FlipsideViewController *controller = [[FlipsideViewController alloc] initWithNibName:@"FlipsideView" bundle:nil];
    	controller.delegate = self;
    	
    	controller.modalTransitionStyle = UIModalTransitionStyleFlipHorizontal;
    	[self presentModalViewController:controller animated:YES];
    	
    	[controller release];
    }
    
    - (void)didReceiveMemoryWarning {
    	// Releases the view if it doesn't have a superview.
        [super didReceiveMemoryWarning];
    	
    	// Release any cached data, images, etc. that aren't in use.
    }
    
    - (void)viewDidUnload {
    	// Release any retained subviews of the main view.
    	// e.g. self.myOutlet = nil;
    }
    
    // Override to allow orientations other than the default portrait orientation.
    - (BOOL)shouldAutorotateToInterfaceOrientation:(UIInterfaceOrientation)interfaceOrientation {
    	// Return YES for supported orientations.
    	return (interfaceOrientation == UIInterfaceOrientationLandscapeLeft) || (interfaceOrientation == UIInterfaceOrientationLandscapeRight);
    }
    
    - (void)dealloc {
        [super dealloc];
    	[scrollView release];
    	[_theAudio release];
    	
    }
    
    @end
    
     
  11. dejo Moderator

    dejo

    Staff Member

    Joined:
    Sep 2, 2004
    Location:
    The Centennial State
    #11
    dealloc on the super should always be called last in the dealloc.


    If what I'm saying doesn't make sense, I suggest stepping back from the real coding and going and (re)learning the fundamentals of Objective-C.
     
  12. hotkarl thread starter macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #12
    Dig! I'm working on Objective-C, I'm just trying to write stuff to make it all stick and sometimes I get curious and try to take it a bit further than I know.

    Thanks for your help!
     
  13. hotkarl thread starter macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #13
    Ok, I got it, but it still leaks a little. Only on the first button press. Is there anything I can do about that?

    Code:
    -(IBAction) selectLick:(id)sender {
    	[self.theAudio release];
    	titleForButton = [sender titleForState: UIControlStateNormal];
    	imagePath = [[NSString alloc] initWithFormat:@"%@.png", titleForButton];
    	imageView = [[UIImageView alloc] initWithImage:[UIImage imageNamed:imagePath]];
    	[imagePath release];
        [scrollView setCanCancelContentTouches:NO];
        scrollView.clipsToBounds = YES; 
        scrollView.indicatorStyle = UIScrollViewIndicatorStyleWhite;
    	[[scrollView subviews] makeObjectsPerformSelector:@selector(removeFromSuperview)];
    	[scrollView addSubview:imageView];
        [scrollView setContentSize:CGSizeMake(imageView.frame.size.width, imageView.frame.size.height)];
    	[imageView release];
        [scrollView setScrollEnabled:YES];
    	[self.theAudio stop];
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    	musicPath = [[NSBundle mainBundle] pathForResource:titleForButton ofType:@"mp3"];
    	fileURL = [[NSURL alloc] initFileURLWithPath: musicPath];
    	self.theAudio = [[AVAudioPlayer alloc] initWithContentsOfURL: fileURL error: nil];
    	[fileURL release];
    	[self.theAudio prepareToPlay];
    }
     

    Attached Files:

  14. hotkarl thread starter macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #14
    And what about this part where I listen for the audio to finish?

    Code:
    - (void) audioPlayerDidFinishPlaying:(AVAudioPlayer *)theAudio successfully:(BOOL)flag 
    { 
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    }
    
    I've tried a few things there, but none seem to work.
     
  15. dejo Moderator

    dejo

    Staff Member

    Joined:
    Sep 2, 2004
    Location:
    The Centennial State
    #15
    playBtn is a property too, yeah?

    And you're sure the delegate method is being called?
     
  16. hotkarl thread starter macrumors newbie

    hotkarl

    Joined:
    Nov 19, 2010
    Location:
    Northern Utah
    #16
    playBtn is a button that makes the audio play. It was working before I started using the self.theAudio, but now I can't figure out what to use there. I'm not sure how to refer to self.theAudio in that area. (self.audio gives me an error)

    Code:
    -(IBAction) selectLick:(id)sender {
    	[self.theAudio release]; 
    	titleForButton = [sender titleForState: UIControlStateNormal]; //
    	imagePath = [[NSString alloc] initWithFormat:@"%@.png", titleForButton];
    	imageView = [[UIImageView alloc] initWithImage:[UIImage imageNamed:imagePath]];
    	[imagePath release];
        [scrollView setContentOffset:CGPointMake(0,0) animated:NO];
    	[[scrollView subviews] makeObjectsPerformSelector:@selector(removeFromSuperview)];
    	[scrollView addSubview:imageView];
        [scrollView setContentSize:CGSizeMake(imageView.frame.size.width, imageView.frame.size.height)];
    	[imageView release];
        [scrollView setScrollEnabled:YES];
    	[self.theAudio stop];
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    	musicPath = [[NSBundle mainBundle] pathForResource:titleForButton ofType:@"mp3"];
    	fileURL = [[NSURL alloc] initFileURLWithPath: musicPath];
    	self.theAudio = [[AVAudioPlayer alloc] initWithContentsOfURL: fileURL error: nil];
    	[fileURL release];
    	[self.theAudio prepareToPlay];
    }
    
    -(IBAction) playLick: (id) sender {
    	playBtn.showsTouchWhenHighlighted = YES;
    	if (self.theAudio.isPlaying) {
    		[self.theAudio pause];
    		[sender setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    	}
    	else {
    		[self.theAudio play];
    		[sender setTitle:@"Pause" forState:(UIControlState)UIControlStateNormal];
    	}
    }
    
    [COLOR="Red"]- (void) audioPlayerDidFinishPlaying:(AVAudioPlayer *)theAudio successfully:(BOOL)flag[COLOR="Lime"] //I'm not sure how to refer to the self.theAudio that I've been using above...[/COLOR]
    { 
    	[playBtn setTitle:@"Listen" forState:(UIControlState)UIControlStateNormal];
    }[/COLOR]
     
  17. dejo Moderator

    dejo

    Staff Member

    Joined:
    Sep 2, 2004
    Location:
    The Centennial State
    #17
    That's not really what I was asking. If theAudio is a property and I recommended you use the proper accessors, don't you think you should do the same for playBtn, which is also a property?

    What it? What there? You should try to be more specific if you expect us to provide assistance. That way we don't have to guess or ask for more details. I'm gonna suggest you check out this blog entry on Getting Answers.

    Of course. You never defined an audio property.
     
  18. Sykte macrumors regular

    Joined:
    Aug 26, 2010
    #18

    At first I thought this was really whacky, now I use it all the time.
     

Share This Page