NSURLConnection NSURLRequest Memory Leak

Discussion in 'iOS Programming' started by elronaldo, Sep 28, 2008.

  1. elronaldo macrumors newbie

    Joined:
    Sep 22, 2008
    #1
    I am subclassing NSURLConnection and experiencing a memory leak for the initWithRequest method for the NSURLRequest object. The init method is shown below:

    Code:
    - (id)initWithRequest:(NSURLRequest *)request fileName:(NSString *)newName fileType:(FileType)newType delegate:(id)delegate {
    	if (self = [super initWithRequest:request delegate:delegate]) {
    		self.targetURL = [[request URL] absoluteString];
    		self.totalFileSize = 0;
    		self.totalDownloaded = 0;
    		self.fileName = newName;
    		self.fileType = newType;
    		self.receivedData = [NSMutableData data];
    		
    	}
    	return self;
    }
    
    - (void)dealloc {
    	[self.receivedData release];
    	[self.fileName release];
    	[self.downloadPath release];
    	[self.targetURL release];
        [super dealloc];
    }
    
    Any ideas why this would be occurring?
     
  2. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #2
    How do you know you have a leak? Why are you subclassing NSURLConnection? Usually it's used by composition. Are you sending start to the connection?
     
  3. elronaldo thread starter macrumors newbie

    Joined:
    Sep 22, 2008
    #3
    The leak is showing in Instruments. I am subclassing the NSURLConnection to try determine the progress as the download is received. I have attached more code that I believe could be cause of the memory leaks. Downloader is the subclass of the NSURLConnection.

    Code:
    
    - (void)downloadImagesForMainHtml {
    		
    	self.filesCount = 0;
    	
    	NSMutableArray *stylesheetSrcs = [[NSMutableArray alloc] initWithArray:[self.htmlParser getAttribute:@"href" forElementsByTag:@"link"]];
    	if (stylesheetSrcs) {
    		for (int i=0; i<[stylesheetSrcs count]; i++) {
    			if ([stylesheetSrcs objectAtIndex:i] != nil) {
    				NSString *css = [[NSString alloc] initWithString:[stylesheetSrcs objectAtIndex:i]];
    				NSString *temp = [[NSString alloc] initWithFormat:@"%@%@", MAIN_URL, css];
    				
    				NSLog(@"Stylesheet Source %u: %@", i, temp);
    				NSURL *url = [[NSURL alloc] initWithString:temp];
    				[temp release];
    				
    				NSURLRequest *request = [[NSURLRequest alloc] initWithURL:url];
    				[url release];
    				Downloader *newDownloader = [[Downloader alloc] initWithRequest:request fileName:css fileType:kAnyStylesheetFile delegate:self];
    				[css release];
    				[request release];
    				
    				self.filesCount = self.filesCount + 1;
    			}
    		}
    	}
    	[stylesheetSrcs release];
    	
    	if (![self.fileManager fileExistsAtPath:self.imagesPath]) {
    		if (![self.fileManager createDirectoryAtPath:self.imagesPath attributes:nil]) {
    			NSLog(@"Could not create Downloads directory: %@", self.imagesPath);
    			return;
    		}
    	}
    	
    	NSMutableArray *imageSrcs = [[NSMutableArray alloc] initWithArray:[self.htmlParser getAttribute:@"src" forElementsByTag:@"img"]];
    	if (imageSrcs) {
    		for (int i=0; i<[imageSrcs count]; i++) {
    			NSString *src = [[NSString alloc] initWithString:[imageSrcs objectAtIndex:i]];
    			NSLog(@"Image Source %u: %@", i, src);
    			NSURL *url = [[NSURL alloc] initWithString:src];
    			
    			NSURLRequest *request = [[NSURLRequest alloc] initWithURL:url];
    			[url release];
    			Downloader *newDownloader = [[Downloader alloc] initWithRequest:request fileName:src fileType:kAnyImageFile delegate:self];
    			[src release];
    			[request release];
    			
    			self.filesCount = self.filesCount + 1;
    		}
    	}
    	[imageSrcs release];
    	self.filesTotal = self.filesTotal + [imageSrcs count];
    	
    	NSMutableArray *scriptSrcs = [[NSMutableArray alloc] initWithArray:[self.htmlParser getAttribute:@"src" forElementsByTag:@"script"]];
    	if (scriptSrcs) {
    		for (int i=0; i<[scriptSrcs count]; i++) {
    			if ([scriptSrcs objectAtIndex:i] != nil) {
    				NSString *js = [[NSString alloc] initWithString:[scriptSrcs objectAtIndex:i]];
    				NSString *temp = [[NSString alloc] initWithFormat:@"%@%@", MAIN_URL, js];
    				NSLog(@"JavaScript Source %u: %@", i, temp);	
    				NSURL *url = [[NSURL alloc] initWithString:temp];
    				[temp release];
    				
    				NSURLRequest *request = [[NSURLRequest alloc] initWithURL:url];
    				[url release];
    				Downloader *newDownloader = [[Downloader alloc] initWithRequest:request fileName:js fileType:kAnyScriptFile delegate:self];
    				[js release];
    				[request release];
    				
    				self.filesCount = self.filesCount + 1;
    			}
    		}
    	}
    	[scriptSrcs release];
    }
    
    
    
     
  4. elronaldo thread starter macrumors newbie

    Joined:
    Sep 22, 2008
    #4
    Any ideas where I could be going wrong here? Thanks!
     
  5. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #5
    Nothing jumps out at me. In your first post you seem to say that it is the NSURLRequest object that is leaking. Is that right?
     
  6. elronaldo thread starter macrumors newbie

    Joined:
    Sep 22, 2008
    #6
    Yeah, it is leaking on the initWithRequest in Instruments from this downloadImagesForMainHtml method. Seems to be the NSURLRequest but I don't understand why.
     
  7. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #7
    It seems that the only lines that reference the request are these two lines

    Code:
    	if (self = [super initWithRequest:request delegate:delegate]) {
    		self.targetURL = [[request URL] absoluteString];
    
    Do you know which one is guilty? The NSURLConnection probably copies the request that is passed to its initWithRequest method, although it might merely retain it. Your code doesn't store the request so I don't see how it would be leaking.

    I assume that your code works correctly otherwise. I assume that you aren't using the same Downloader object for more than one connection or calling the init method multiple times on the same object.

    You might want to build a simpler test of this class. Just create a simple method that downloads one url rather than the more complicated method you have where you're downloading a bunch of urls from several lists. See if the leak persists. It may be something in the way you're calling the init method.
     
  8. elronaldo thread starter macrumors newbie

    Joined:
    Sep 22, 2008
    #8
    Thanks! Could you explain the above further? I am allocating and initalising multiple Downloader objects within the for loop - could this be causing the leaks?

    I will also try a few other tests as per your recommendations. Thanks again.
     
  9. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #9
    In the code you posted it seemed that you were building one download object for each download operation. That's what you should do.

    It might seem possible to call the init method of a single object multiple times but Cocoa isn't designed for that. Although there isn't anything in the language that prevents it the Cocoa classes aren't designed to be inited more than one time. If you did that you might cause memory leaks or other weird behavior. That's why I asked.
     
  10. elronaldo thread starter macrumors newbie

    Joined:
    Sep 22, 2008
    #10
    Ah okay. Thanks! I don't think that is causing the errors, I am doing a bit of a re-design so I will see how I go with that.

    So generally with Cocoa, how would I re-initialize an object correctly. Would I release the object and simply re-allocate a new object? What is the recommended coding for this?
     
  11. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #11
    The Cocoa classes aren't designed to work correctly if you send an init message more than one time. What will happen is undefined.

    Many Cocoa classes can be thought of as mutable or immutable. NSString is immutable, NSMutableString is mutable. Instances of immutable classes can't be changed. Instances of mutable classes have various methods that allow them to be reset to a new value, but they still can't be re-initialized. NSURLConnection can only be used for one connection.
     
  12. elronaldo thread starter macrumors newbie

    Joined:
    Sep 22, 2008
    #12
    Only a few of the memory leaks are NSURLRequest. Most of the leaks are NSURLConnection initWithRequest:delegate > NSURLConnectionPrivate > createCFRequest

    Does that help with obtaining some more advice?
     
  13. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #13
    In the code that you showed you create the Downloader objects but you don't store them in an array or retain any reference to them. How do you release them? You set self as the delegate so they make callbacks to your controller object about the download progress but how do you release them when they're finished?
     
  14. elronaldo thread starter macrumors newbie

    Joined:
    Sep 22, 2008
    #14
    In the - (void)connectionDidFinishLoading: (NSURLConnection *)connection delegate I am calling [connection release] as per the documentation sample code.

    Thanks
     
  15. PhoneyDeveloper macrumors 68030

    PhoneyDeveloper

    Joined:
    Sep 2, 2008
    #15
    OK, that makes sense. Are you also releasing them in the error case? Do you ever cancel a download?

    I would try to try to track the Downloader objects that are being created and destroyed. Something like keep a counter of the current number of active objects and print it out when a Downloader is created and destroyed or build an array that holds the Downloader objects that are built and that removes them when they finish. The array should be empty at the end of your process.

    Are the Downloader objects being leaked also?
     

Share This Page