views:

205

answers:

6

Below is the @interface for an MREntitiesConverter object I use to strip all html tags from a string using an NSXMLParser.

@interface MREntitiesConverter : NSObject {
    NSMutableString* resultString;
    NSString* xmlStr;
    NSData *data;
    NSXMLParser* xmlParser;
}
@property (nonatomic, retain) NSMutableString* resultString;
- (NSString*)convertEntitiesInString:(NSString*)s;
@end

And this is the implementation:

@implementation MREntitiesConverter
@synthesize resultString;
- (id)init
{
    if([super init]) {
        self.resultString = [NSMutableString string];
    }
    return self;
}

- (NSString*)convertEntitiesInString:(NSString*)s {
    xmlStr = [NSString stringWithFormat:@"<data>%@</data>", s];
    data = [xmlStr dataUsingEncoding:NSUTF8StringEncoding allowLossyConversion:YES];
    xmlParser = [[NSXMLParser alloc] initWithData:data];

    [xmlParser setDelegate:self];
    [xmlParser parse];

    return [resultString autorelease];
}

- (void)dealloc {
    [data release];
    //I want to release xmlParser here but it crashes the app
    [super dealloc];
}

- (void)parser:(NSXMLParser *)parser foundCharacters:(NSString *)s {
    [self.resultString appendString:s];
}
@end

If I release xmlParser in the dealloc method I am crashing my app but without releasing I am quite obviously leaking memory.

I am new to Instruments and trying to get the hang of optimising this app. Any help you can offer on this particular issue will likely help me solve other memory issues in my app.

Yours in frustrated anticipation: ) Oisin

+2  A: 

I don't see xmlParser being used outside of convertEntitiesInString:. You could make xmlParser local to that method (not an instance variable) and release it when you're done with it in that method, before the return [resultString autorelease] line.

mipadi
thanks, I had far too many instance variables going on… this did not fix the crash but did make my code better
prendio2
+2  A: 

Are you sure [xmlParser release] is crashing the application in dealloc? I see you have [data release] in dealloc and I can't see a statement where you have used alloc to allocate memory to it.

Hetal Vora
yes you are right… I created a data instance to pass to xmlParser but I am not responsible for its memory so by calling dealloc on it *and* on xmlParser I was crashing the app. thanks.
prendio2
A: 

While the other two comments are correct (you're forgetting to release the parser in the method you alloc it, which is a memory leak), I bet it's crashing specifically because you set your MREntitiesConverter class as the delegate for the parser.

Shaggy Frog
MREntitiesConverter needs to be the delegate to process the string and create the final resultString
prendio2
Some classes have problems where their delegates are dealloced before they are told about it; UIWebView has suffered from this in previous versions of the SDK.
Shaggy Frog
+3  A: 

Both your class and NSXMLParser are releasing data, which causes your current crash. The only member should be resultString. You should initialize resultString in convertEntitiesInString: not init, so the same instance could be used more than once. You should return either self.resultString or [[resultString retain] autorelease] from convert, because what you currently do will cause a double release later if you release resultString in dealloc as you should. You should use resultString directly in parser:foundCharacters: instead of self.resultString which is a method call.

drawnonward
thanks for your suggestions… you've helped clarify some memory management for me. I will post my own "answer" with the latest version of the code
prendio2
A: 

I believe it is resultString, which cause crash. This is NSMutableString and you've not allocated any memory for this. Also release xmlParser in convertEntitiesInString before return [resultString autorelease];

iamiPhone
resultString is set as a @property, synthesized and as far as I understand memory is assigned when I write self.resultString = [NSMutableString string] — or have I misunderstood something?
prendio2
A: 

Based on some really helpful sugestions here, this is the latest version of my code, which does not appear to leak memory:

@interface MREntitiesConverter : NSObject {
    NSMutableString* resultString;
}
@property (nonatomic, retain) NSMutableString* resultString;
- (NSString*)convertEntitiesInString:(NSString*)s;
@end


@implementation MREntitiesConverter
@synthesize resultString;

- (NSString*)convertEntitiesInString:(NSString*)s {
    self.resultString = [NSMutableString string];

    NSString* xmlStr = [NSString stringWithFormat:@"<d>%@</d>", s];
    NSData *data = [xmlStr dataUsingEncoding:NSUTF8StringEncoding allowLossyConversion:YES];
    NSXMLParser* xmlParse = [[NSXMLParser alloc] initWithData:data];

    [xmlParse setDelegate:self];
    [xmlParse parse];
    [xmlParse release];

    return [self.resultString autorelease];
}

- (void)parser:(NSXMLParser *)parser foundCharacters:(NSString *)s {
    [resultString appendString:s];
}
@end

The only oddity is that if I trace the retainCount of resultString just prior to returning it I get a count of 2 where I would have expected it to be 1. Any idea why?

prendio2