Potential Leak of an Object that is released before reuse and dealloc'd on exit

Question

Using the xcode analyzer, I am getting a warning for a potential leak of an object. This warning is perplexing and I need some explanation why I am getting this error. Here is the code in which mediaSources hold the pointer to the object in question:

In the .h file, the a pointer to the MediaSources class is created and given a retain property:

@interface RootViewController : UIViewController <...>
{
    ...

    MediaSources                *mediaSources;

    ...
}

@property (nonatomic, retain)   MediaSources            *mediaSources;

In the .m file (rootViewController) is a method that can be called many times. Consequently, I release the object on each entry and alloc a new object. The MediaSources object performs background tasks, so I don't want to release it until I know it is complete. If I use autoRelease on the line that the class is alloc'd in, it crashes. :

-(void) getSelectedMediaSources
{
    [self setMediaSources: nil];    // release old stuff and nilify 
    [self setMediaSources: [[MediaSources alloc] init]]; 
    [self.mediaSources checkForMediaSourceUpdates];
}

Also in the .m file, mediaSources is also synthesized and is released in dealloc

@synthesize mediaSources;

...

- (void)dealloc {
    ...
    [mediaSources release];
    ...

    [super dealloc];
}

Please explain why I am getting this warning. I don't see how there could be a leak. Dealloc should release the last copy of this object.

IN RESPONSE TO REQUEST FOR CODE FROM checkForMediaSourceUpdates. This is going to get a bit complicated, but below is the essence:

(void) checkForMediaSourceUpdates
{    
    NSString *s = [NSString  stringWithFormat:@"http://www.mywebsite.com/mediaSources/%@/mediaSources.plist", countryCode];

    NSURL *url = [NSURL URLWithString:s];
    NSURLRequest *req = [NSURLRequest requestWithURL:url  cachePolicy:NSURLRequestReloadIgnoringLocalAndRemoteCacheData timeoutInterval:60.0];
    MyDownloader *d = [[MyDownloader alloc] initWithRequest:req];
    [self.connections addObject:d];
    [[NSNotificationCenter defaultCenter] addObserver:self  selector:@selector(checkForMediaSourceUpdatesDownloadFinished:) name:@"connectionFinished" object:d];
    [d.connection start];
    [d release];
}

-(void) checkForMediaSourceUpdatesDownloadFinished: (NSNotification *) n 
{
    MyDownloader *d = [n object];
    NSData *data = nil;
    if ([n userInfo]) {
        NSLog(@"In checkForMediaSourceUpdatesDownloadFinished: MyDownloader returned an error");
    }
    else {
        data = [d receivedData];

        // do something with the data
    }
}

The myDownloader class performs a download of a file as specified in the input NSURLRequest. After completing the download, this class will generate a NSNotification named "connectionFinished". The user of this class must register for this notification and handle all cleanup operations of this class. If the download fails, this class will generate a NSNotification, also named "connectionFinished", but with the addition of userInfo that indicates an error occured. Again, the user of this class must register for this notification and handle all cleanup operations of this class.

Answer 1

As already mentioned in other answers, you should pass an autoreleased instance to the setter like this:

[self setMediaSources: [[[MediaSources alloc] init] autorelease]]; 

The crash you're seeing as a result of doing this seems to stem from the fact that you are doing background processing in checkForMediaSourceUpdates. If you want to do background processing in this method, you should ensure that self remains valid for the entire duration of the background task. It is perfectly valid to manually call [self retain]; at the start and [self release]; at the end. (Note: GCD blocks automatically retain/release self object if you're referencing it from within the block).

EDIT: I hope you know the problem now, mAu called it. There is only one retain count (by RootViewController object) on MediaSources instance when a request is being executed. Before the request finishes, you call the method again, resulting in the MediaSources object being released. So when the request finishes, and NSNotification tries to send the callback message, app crashes since the observer instance has disappeared.

There are two ways to resolve this (use whichever is more appropriate for your case):

1. Either add [self retain]; in checkForMediaSourceUpdates and [self release]; at the end of checkForMediaSourceUpdatesDownloadFinished:
2. Or, add [[NSNotificationCenter defaultCenter] removeObserver: self] in -[MediaSources dealloc].

Btw, you should think about why do you need to allocate a new MediaSource on every call to getSelectedMediaSources? and why you're starting a new request every time when you potentially have a running request. There is something very wrong with your design.

Answer 2

[self setMediaSources: [[MediaSources alloc] init]]; 

You're retaining the new MediaSources object twice, once with the alloc and again with the setMediaSources. You only release it once.

Try:

[self setMediaSources: [[[MediaSources alloc] init] autorelease]]; 

Follow up from comments:

Yes, you can set the variable directly, but that doesn't solve your root problem. It sounds like your existing code works because of that extra retain and fails when you balance your retain/releases. You need to find out who is keeping a weak reference to the old mediaSources object and break that reference before you release it.

Answer 3

Per definition you pass an autoreleased object to a synthesized setter. The setter itself retains the object, thus the following line is wrong:

[self setMediaSources: [[MediaSources alloc] init]]; 

It should be:

[self setMediaSources: [[[MediaSources alloc] init] autorelease]]; 

Also you do not need to call your setter with nil before. When setting a different object via the setter, the old object gets released. The synthesized setter looks something like:

- (void) setMediaSources:(MediaSources *)mediaSources {
  if (_mediaSources != mediaSources) {
    [_mediaSources release];
    _mediaSources = [mediaSources retain];
  }
}

The question is: why do you need to allocate a new MediaSource on every call to getSelectedMediaSources? What crash are you expecting when deallocating the autoreleased MediaSource? Are your setting it as the delegate for another object or registering it to a NSNotificationCenter? If so, do not forget to nil out the delegate or remove it from the notification center.