NSURLConnection sendAsynchronousRequest never free up the memory

Question

I am working on an iOS app which dispatch quite a number of tasks to my serial queue. The task is to download images from my web server, save it to disk, and later displayed on UIImageView. However, [NSURLConnection sendAsynchrousRequest] will keep eating up more and more memory until iOS kill my process.

The downloader method looks like this:

// dispatch_queue_t is created once by: m_pRequestQueue = dispatch_queue_create( "mynamespace.app", DISPATCH_QUEUE_SERIAL);

- (void) downloadImageInBackgroundWithURL:(NSString*) szUrl {
__block typeof(self) bSelf = self;
__block typeof(m_pUrlRequestQueue) bpUrlRequestQueue = m_pRequestQueue;

dispatch_async( m_pRequestQueue, ^{ 
    NSAutoreleasePool *pAutoreleasePool = [[NSAutoreleasePool alloc] init];
    NSURLRequest *pRequest = [NSURLRequest requestWithURL:[NSURL URLWithString:szUrl]
                                              cachePolicy:NSURLRequestReloadIgnoringCacheData
                                          timeoutInterval:URL_REQUEST_TIMEOUT];

    [NSURLConnection sendAsynchronousRequest:pRequest queue:bpUrlRequestQueue completionHandler:^(NSURLResponse *pResponse, NSData *pData, NSError *pError) {
        NSAutoreleasePool *pPool = [[NSAutoreleasePool alloc] init];
        if ( pError != nil ) {
        } else {                
            // convert image to png format
            UIImage *pImg = [UIImage imageWithData:pData];
            NSData *pDataPng = UIImagePNGRepresentation(pImg);
            bool bSaved = [[NSFileManager defaultManager] createFileAtPath:szCacheFile contents:pDataPng attributes:nil];                    
        }

            __block typeof(pDataPng) bpDataPng = pDataPng;
            __block typeof(pError) bpError = pError;
            dispatch_sync( dispatch_get_main_queue(), ^ {
                NSAutoreleasePool *autoreleasepool = [[NSAutoreleasePool alloc] init];
                UIImage *pImage = [[UIImage alloc] initWithData:bpDataPng];

                // display the image

                [pImage release];
//                    NSLog( @"image retain count: %d", [pImage retainCount] ); // 0, bad access

                [autoreleasepool drain];
            });
        }
        [pPool drain];
    }]; // end sendAsynchronousRequest

    [pAutoreleasePool drain];

}); // end dispatch_async
} // end downloadImageInBackgroundWithURL

I am quite sure it is something inside [NSURLConnection sendAsynchronousRequest] as the profiler is showing that the function is the one eating up all the memory...

However, I am also not very sure about the dispatch_*** and block things, I've always used C and C++ code with pthread before, but after reading from Apple's documentation on migrating away from thread, I decided to give GCD a try, objective-c is so troublesome and I'm not sure how to release the NSData *pData and NSURLResponse *pResponse as it crash whenever I do it.

Please advice... really need help to learn and appreciate objective-c...

ADDITIONAL EDIT:

Thanks to @robhayward, I put the pImg and pDataPng outside as __block variable, use his RHCacheImageView way of downloading data ( NSData initWithContentOfURL )

Thanks as well to @JorisKluivers, the first UIImage can actually be reused to display as UIImageView recognized both jpg and png format, just my later processing requires png format and I am reading from the disk later just when required

Answer 1

I would firstly put it down to the image and data objects that you are creating:

UIImage *pImg = [UIImage imageWithData:pData];
NSData *pDataPng = UIImagePNGRepresentation(pImg);

Which might be hanging around too long, perhaps put them outside the block, as they are probably being created/released on different threads:

__block UIImage *pImg = nil;
__block NSData *pDataPng = nil;
[NSURLConnection sendAsynchronousRequest..

(Also consider using ARC if you can)

I have some code on Github that does a similar job without this issue, feel free to check it out:

https://github.com/robinhayward/RHCache/blob/master/RHCache/RHCache/Helpers/UIImageView/RHCacheImageView.m

Answer 2

First of all try simplifying your code. Things I did:

• Remove the outer dispatch_async. This is not needed, your sendAsynchronousRequest is async already. This also removes the need another __block variable on the queue.
• You create an image named pImg from the received pData, then convert that back to NSData of type png, and later create another image pImage from that again. Instead of converting over and over, just reuse the first image. You could even write the original pData to disk (unless you really want the png format on disk).

I didn't compile the code below myself, so it might contain a few mistakes. But it is a simpler version that might help solve the leak.

- (void) downloadImageInBackgroundWithURL:(NSString*)szUrl
{
    __block typeof(self) bSelf = self;

    NSAutoreleasePool *pAutoreleasePool = [[NSAutoreleasePool alloc] init];
    NSURLRequest *pRequest = [NSURLRequest requestWithURL:[NSURL URLWithString:szUrl]
                                          cachePolicy:NSURLRequestReloadIgnoringCacheData
                                      timeoutInterval:URL_REQUEST_TIMEOUT];

    [NSURLConnection sendAsynchronousRequest:pRequest queue:m_pRequestQueue completionHandler:^(NSURLResponse *pResponse, NSData *pData, NSError *pError) {
        NSAutoreleasePool *pPool = [[NSAutoreleasePool alloc] init];
        if (pError) {
            // TODO: handle error
            return;
        }

        // convert image to png format
        __block UIImage *pImg = [UIImage imageWithData:pData];

        // possibly just write pData to disk
        NSData *pDataPng = UIImagePNGRepresentation(pImg);
        bool bSaved = [[NSFileManager defaultManager] createFileAtPath:szCacheFile contents:pDataPng attributes:nil];                    

        dispatch_sync( dispatch_get_main_queue(), ^ {
            // display the image in var pImg
        });
    }];
    [pAutoreleasePool drain];
}