views:

209

answers:

1

I seem to have a fundamental gap in my memory management understanding. The code below is located within a singleton that gets called multiple times within my app to parse data that is downloaded from the web. For each article I download, I allocate a mutable string, then do tons of parsing, then write the file to the file system for later display in a UIWebView.

But every time I enter this method, I allocate a new "articleString". And I never release this string. I think this is a leak, but if I add a release at the bottom of this method (after the file is written), my app crashes the next time this method is called. I don't understand why it crashes, since another NSMutableString is allocated next time it is called.

UPDATE: I do release articleString in the dealloc method. But it still seems that I should release at the end of this method, since I alloc every time I enter.

UPDATE: articleString is defined as follows in the header:

@property (nonatomic, retain) NSMutableString *articleString;

the parseArticle method below is a placeholder for a series of methods that manipulate articleString.

 self.articleString = [[NSMutableString alloc] initWithData:articleData encoding:NSUTF8StringEncoding];

 //Parse the article for display
 [self parseArticle];

 //Write the article string to a file for later display
 NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
 NSString *documentsDirectory = [paths objectAtIndex:0];
 NSString *path = [documentsDirectory stringByAppendingPathComponent:@"article.html"];
 NSLog(@"%@", articleString);
 [articleString writeToFile:path atomically:YES];
+1  A: 

I like to let properties handle this for me. If the articleString property is set to retain then this is simple.

self.articleString = [[[NSMutableString alloc] initWithData:articleData encoding:NSUTF8StringEncoding] autorelease];

[self doStuff];

Then

- (void)dealloc {
    self.articleString = nil;
    [super dealloc]
}

article string will get released and properly retain when you set a new one. And it will be cleaned up on dealloc.

Squeegy
Why do self.articleString = nil; instead of [articleString release] in the dealloc method?
Don Wilson
because setting a retained property to nil releases it. And it's a direct compliment to how it's assigned (with no explicit `retain`). And this crashes: `[articleString release]; [articleString someMethod]` while this works since calling methods on `nil` is cool: `self.articleString = nil; [articleString doStuff]`. And, along the same vein, if you already released it, this statement does nothing harmful. In general, while it does the same thing, it's overall far less error prone and easier to understand.
Squeegy
As a firm rule, you never release a property except in the dealloc method. The properties synthesized accessor methods are tracking the retain count for you. Any attempt to release a property directly will almost always cause a bug.
TechZen
Conversely, you should consider any attempt to send a message to a half-deallocated or half-initialized object a bug. This includes accessor messages. It might be a synthesized accessor today, but you could replace it with a custom accessor tomorrow, or in a subclass. Always retain/copy and assign directly to the ivar in `init` methods, and release the object directly in `dealloc`.
Peter Hosey