2
votes

I've been using code from Raphael Cruzeiro's PDF Annotator, and have discovered a number of memory leaks (ARC is off, and will stay off to support older devices). After patching most of them up, I'm down to the last couple, and they have me stumped. So in a class called PDFDocument, he has properties for a CGPDFPageRef, CGPDFDocument, and a custom annotation class @synthesize'd. I had to pepper his dealloc method with releases and eliminate some dangling pointers, which works well except for one small problem: After about 3 complete retain-release cycles, it crashes at the @synthesize line for his annotation object... I've never seen a SIGABRT because of a deallocated object sent during @synthesize, so naturally have no idea how to fix it. If I remove the release code in dealloc, it leaks, but if I leave it in, it crashes. Here's the code for the PDFDocument class:

//.h

#import <Foundation/Foundation.h>

@class Annotation;

@interface PDFDocument : NSObject {
    Annotation *_annotation;
}

- (id)initWithDocument:(NSString *)documentPath;

- (NSInteger) pageCount;
- (void) loadPage:(NSInteger)number;
- (BOOL)save;

@property (nonatomic, retain) NSString *name;
@property (nonatomic, retain) NSString *hash;
@property (readwrite, nonatomic, assign) CGPDFDocumentRef document;
@property (readwrite, nonatomic, assign) CGPDFPageRef page;

@property (nonatomic, retain) NSString *version;

@property (nonatomic, assign) BOOL dirty;

@property (nonatomic, retain) Annotation *annotation;

@end

//.m
#import "PDFDocument.h"
#import "Annotation.h"
#import "HashExtensions.h"
#import "DocumentDeserializer.h"
#import "DocumentSerializer.h"


@implementation PDFDocument

@synthesize document;
@synthesize page;
@synthesize annotation = _annotation; //after 3rd cycle, it crashes here.
@synthesize name;
@synthesize hash;
@synthesize dirty;
@synthesize version;

- (id)initWithDocument:(NSString *)documentPath
{
    if((self = [super init]) != NULL) {

        self.name = [documentPath lastPathComponent];
        if ([self.name isEqualToString:@"Musette.pdf"] || [self.name isEqualToString:@"Minore.pdf"] || [self.name isEqualToString:@"Cantata.pdf"] || [self.name isEqualToString:@"Finalé.pdf"]) 
        {
        CFURLRef ref = CFBundleCopyResourceURL(CFBundleGetMainBundle(), (CFStringRef)self.name, NULL, NULL);
        self.document = CGPDFDocumentCreateWithURL(ref);
        self.page = CGPDFDocumentGetPage(document, 1);
        self.version = @"1.0";
        DocumentDeserializer *deserializer = [[[DocumentDeserializer alloc] init] autorelease];
        self.annotation = [deserializer readAnnotation:[[(NSURL*)ref absoluteString] stringByDeletingPathExtension]];

        CFRelease(ref);
        }

        else {  

            CFURLRef pdfURL = (CFURLRef)[[NSURL alloc] initFileURLWithPath:documentPath];
            self.document = CGPDFDocumentCreateWithURL(pdfURL);
            self.page = CGPDFDocumentGetPage(document, 1);
            self.version = @"1.0";
            DocumentDeserializer *deserializer = [[[DocumentDeserializer alloc] init] autorelease];
            self.annotation = [deserializer readAnnotation:[[(NSURL*)pdfURL absoluteString] stringByDeletingPathExtension]];

            CFRelease(pdfURL);
            CGPDFPageRelease(self.page);

        }
    }

    return self;
}

- (NSInteger)pageCount
{
    return CGPDFDocumentGetNumberOfPages(self.document);
}

- (void)loadPage:(NSInteger)number
{
    self.page = CGPDFDocumentGetPage(document, number);
}

- (BOOL)save
{
    DocumentSerializer *serializer = [[[DocumentSerializer alloc] init] autorelease];
    [serializer serialize:self];

    self.dirty = NO;
    return !self.dirty;
}

- (void)dealloc
{
    CGPDFDocumentRelease(self.document);
    if (self.annotation != nil && _annotation != nil) {
        [_annotation release];
        self.annotation = nil;
    } //my attempt to prevent the object from being over-released
    self.document = nil;
    self.name = nil;
    [super dealloc];
}

@end

Then I ran it through Instruments to find zombie objects, and sure enough, Instruments found a deallocated object being sent a message at the exact same @synthesize line!

Does anyone have any idea what's going on and how to fix it?

1
only first generation iPhone is incompatible with ARC, why wouldn't you use it?Andrey Zverev
Merely my own preferences... combined with the fact that the ARC refactoring tool is annoying me right now. Sigh... I'll convert right now.CodaFi
You say it crashed "during" synthesize. What that really is, is that it's crashing in either the - (Annotation*)annotation or - (void)setAnnotation:(Annotation*) synthesised methods. Probably the setter where the ivar has already been released.mattjgalloway

1 Answers

8
votes

This bit looks very wrong:

if (self.annotation != nil && _annotation != nil) {
    [_annotation release];
    self.annotation = nil;
}

Firstly, why are you checking self.annotation and _annotation for nil-ness. That's effectively doing the same check twice.

Secondly, you're using direct ivar access to release _annotation and then the setter for annotation will be releasing _annotation again and setting _annotation = nil. Effectively it's doing this:

if (self.annotation != nil && _annotation != nil) {
    [_annotation release];
    [_annotation release];
    _annotation = [nil retain];
}

Which as you can see, is going to over-release _annotation.

Also, seriously, just use ARC. ARC is (mainly) compile time and has nothing to do with the device or OS version it's running on. The only bit that's not supported on pre iOS 5 is auto nil-ed weak pointers. But that really shouldn't be a problem as that's totally new in Lion / iOS 5 anyway.