Memory management in viewDidUnload - should I nil both array and objects inside?

Question

I have a tableview to load news from internet. I and trying to nil all properties in viewDidUnload.

- (void)viewDidUnload
{
    self.newsArray = nil;
    self.newsTableView = nil;
    self.indicatorView = nil;
//  self.iconDownLoader = nil;
    self.downloadArray = nil;

    [super viewDidUnload];
}

Every time the app crash in viewDidUnload. If I comment self.iconDownLoader = nil;, it will be fine. So can any one tell me why does this happen? Thanks you.

---------------------NewsViewController.m--------------------------

//
//  NewsViewController.m
//
//  Created by on 18/01/12.
//  Copyright (c) 2012 __MyCompanyName__. All rights reserved.
//

#import "NewsViewController.h"
#import "ASIHTTPRequest.h"
#import "SBJson.h"
#import "NewsModel.h"
#import "NewsDetailViewController.h"

#define kCustomRowCount 6
#define IconPlaceHolder @"Spinner"

@implementation NewsViewController

@synthesize appDelegate, newsTableViewCell, newsTableView, indicatorView;
@synthesize iconDownLoader, newsArray, downloadArray;

- (id)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil
{
    self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
    if (self) {

        // setup appDelegate
        self.appDelegate = (SydneyAppDelegate *)[[UIApplication sharedApplication] delegate];

        // initial arrays
        self.newsArray = [[NSMutableArray alloc] init];
        self.downloadArray = [[NSMutableArray alloc] init];
    }
    return self;
}

- (void)didReceiveMemoryWarning
{
    [super didReceiveMemoryWarning];
}

#pragma mark - View lifecycle

- (void)viewDidLoad
{
    [super viewDidLoad];

    if(self.appDelegate.reachable) {
        [self getNews];
    }
    else
    {
        UIAlertView *alert = [[UIAlertView alloc] initWithTitle:@"No Connection" message:@"No Internet connection. Please try again later." delegate:self cancelButtonTitle:@"OK" otherButtonTitles:nil];
        [alert show];   
    }
}

- (void)viewDidUnload
{
    self.newsArray = nil;
    self.newsTableView = nil;
    self.indicatorView = nil;
//  self.iconDownLoader = nil;
    self.downloadArray = nil;

    [super viewDidUnload];
}

#pragma mark - ASIHTTPRequest

- (void) getNews
{
    NSURL *url = [NSURL URLWithString:@"http://ferrarimaseratisydney.com/api/getPublicNews.html"];
    ASIHTTPRequest *request = [ASIHTTPRequest requestWithURL:url];
    [request setDelegate:self];
    [request startAsynchronous];
}

- (void) requestFinished:(ASIHTTPRequest *)request
{
    NSString *responseString = [request responseString];
    NSArray *json = [responseString JSONValue];

    for (id aNewsInJson in json)
    {
        NewsModel *aNews = [[NewsModel alloc] initWithJson:aNewsInJson];
        [self.newsArray addObject:aNews];
    }

    [self.indicatorView removeFromSuperview];

    [self.newsTableView reloadData];
}

- (void) requestFailed:(ASIHTTPRequest *)request
{
    NSError *error;
    error = [request error];
}

#pragma mark - Table view delegate

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
{   
    [tableView deselectRowAtIndexPath:indexPath animated:YES];

    // Navigation logic may go here. Create and push another view controller.
    NewsDetailViewController *newsDetailViewController = [[NewsDetailViewController alloc] init];

    // transform news array
    newsDetailViewController.news = [self.newsArray objectAtIndex:indexPath.row];

    // Pass the selected object to the new view controller.
    [self.navigationController pushViewController:newsDetailViewController animated:YES];
}

#pragma mark - Table view data source

- (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView
{
    return 1;
}

- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section
{
    return [self.newsArray count];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *CellIdentifier = @"NewsCellIdentifier";

    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier];
    if (cell == nil) {
        [[NSBundle mainBundle] loadNibNamed:@"NewsTableViewCell" owner:self options:nil];
        cell = self.newsTableViewCell;
        self.newsTableViewCell = nil;
    }

    // read from newsModel
    NewsModel *news = [self.newsArray objectAtIndex:indexPath.row];

    UILabel *label;
    label = (UILabel *)[cell viewWithTag:10];
    label.text = [NSString stringWithString:news.title];
    label = nil;
    label = (UILabel *)[cell viewWithTag:11];
    label.text = [NSString stringWithString:news.description];
    UIImageView *imageView = (UIImageView *)[cell viewWithTag:12];
    imageView.image = news.image;

    if (news.image == nil)
    {
        imageView.image = [UIImage imageNamed:IconPlaceHolder];

        self.iconDownLoader = [[IconDownLoader alloc] init];
        self.iconDownLoader.url = news.imageUrl;
        self.iconDownLoader.delegate = self;
        self.iconDownLoader.indexPath = indexPath;
        if (self.appDelegate.ip4 == YES)
        {
            self.iconDownLoader.width = 300;
            self.iconDownLoader.height = 150;
        }
        else
        {
            self.iconDownLoader.width = 150;
            self.iconDownLoader.height = 75;
        }
        [self.downloadArray addObject:self.iconDownLoader];
        [self.iconDownLoader start];
    }
    return cell;
}

#pragma mark - IconDownLoaderDelegate

- (void)iconDownLoadFinsh:(NSData *)imageData row:(NSIndexPath *)indexPath {

    UITableViewCell *cell = [self.newsTableView cellForRowAtIndexPath:indexPath];
    UIImageView *imageView = (UIImageView *)[cell viewWithTag:12];
    if (imageData != 0)
    {
        imageView.image = [UIImage imageWithData:imageData];
    }
    else
    {
        imageView.image = [UIImage imageNamed:@"icon57"];
    }
    NewsModel *newsModel = [self.newsArray objectAtIndex:indexPath.row];
    newsModel.image = [UIImage imageWithData:imageData];
}

@end

-----------------------IconDownLoader.m-------------------

//
//  IconDownLoader.m
//
//  Created by on 24/11/11.
//  Copyright (c) 2011 __MyCompanyName__. All rights reserved.
//

#import "IconDownLoader.h"
#import "ASIHTTPRequest.h"

@implementation IconDownLoader

@synthesize delegate = _delegate;
@synthesize url = _url;
@synthesize indexPath = _indexPath;
@synthesize width = _width;
@synthesize height = _height;
@synthesize request = _request;

- (void)start {

    NSString *originalString = @"width=%s&height=%s";
    NSString *newString = [NSString stringWithFormat:@"width=%d&height=%d&type=jpg", self.width, self.height];

    NSString *resizedURL = [self.url stringByReplacingOccurrencesOfString:originalString withString:newString];

    NSURL *url = [NSURL URLWithString:[resizedURL stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding]];
    _request = [ASIHTTPRequest requestWithURL:url];
    if (_indexPath) {
        _request.userInfo = [NSDictionary dictionaryWithObject:_indexPath forKey:@"indexPath"];
    }
    [_request setDelegate:self];
    [_request startAsynchronous];
}

- (void)requestFinished:(ASIHTTPRequest *)request {

    NSInteger statusCode = request.responseStatusCode;
    switch (statusCode) {
        case 401: // Not Authorized: either you need to provide authentication credentials, or the credentials provided aren't valid.
            break;

        case 200: {
            NSData *responseData = [request responseData];
            if (!responseData) {
                UIAlertView *alertView;
                alertView = [[UIAlertView alloc] initWithTitle:@"Oops" message:[NSString stringWithFormat:@"Download failed in row %d", _indexPath.row] delegate:nil cancelButtonTitle:@"OK" otherButtonTitles:nil];
                return;
            }
            [_delegate iconDownLoadFinsh:responseData row:[request.userInfo objectForKey:@"indexPath"]];
        }
            break;

        default:{

        }
    }
}

- (void)dealloc {
    if (_request != nil) {
        [_request clearDelegatesAndCancel];
    }
}

@end

Answer 1

Generally in viewDidUnload you should only release and zero out all references to the nib objects you own.

That said, you can destroy model objects in viewDidUnload too if they consume a lot of memory. You should remember that viewDidUnload is a counterpart to viewDidLoad, so a good rule of thumb is to destroy only those objects in viewDidUnload which you create in viewDidLoad. You should also remember that viewDidUnload is not called when the view controller is released – only when its view is.

In your case I would not release newsArray and downloadArray just because you create them in init.... I would just send them removeAllObjects instead.

As for the crash, you create a new shared icon downloader every time a cell needs an image, which is a bit awkward. If you do need a shared downloader instance, you should not recreate it for each and every cell. It looks like you want to keep all the downloaders you created in an ivar array instead because each downloader is one-shot and is responsible for loading a single image.

Answer 2

Not enough information here to tell, but probably you have released iconDownloader directly in some other part of the code we cannot see without setting the reference to nil at that time.

Then in viewDidUnload you are trying to release an invalid reference.

For @synthesize, use:

@synthesize iconDownLoader = _iconDownloader;

then fix all of the compiler warnings to use self.icondownloder instead of iconDownloader, and eliminate all uses of "release" (assuming your property is marked as retain which it should be).

In fact, perhaps your whole problem is that the property is not a retain property so you make the iconDOwnloader and it's promptly released.