What's wrong with this dealloc in ARC?

Question

I am working on an app that does image processing and displays the resulting image. Im using UIScrollView to let user scroll all images, because the image is not a standard jpg or png, it takes time to load. I use GCD to load asynchronously, when finished dispatch to main queue to display. the snippet is as follows:

- (void)loadImage:(NSString *)name
{
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        UIImage *image = [Reader loadImage:name];
        dispatch_sync(dispatch_get_main_queue(), ^{
            [self displayImage:image];
        });
    });
}

the loadImage method of Reader is like this:

+ (UIImage *)loadImage:(NSString *)name
{
   UInt8 *data = NULL;
   NSString *mfjPath = [TMP stringByAppendingPathComponent:name];
   NSData *mfjData = [NSData dataWithContentsOfFile:mfjPath];
   if(mfjData){
        data = malloc(sizeof(UInt8)*mfjData.length);
        [mfjData getBytes:data];
   }
   if(data){
        ResultHolder *result = [sDecoder decodeData:data withOffset:0];// static id<IDecoder> sDecoder; in Reader.m before @implementation Reader.
        return [result bitmap];
   }
    retun nil;
}

IDCoder is a protocol which is

@protocol IDecoder <NSObject>
- (ResultHolder *)decodeData:(UInt8 *) withOffset:(int)offset;
@end

ResultHolder is a class to load simple image and combine complicated image. which is as follows:

ResultHolder.h

typedef struct color24{
    UInt8 R;
    UInt8 G;
    UInt8 B;
} Color24;

@interface ResultHolder : NSObject
{
    unsigned long mWidth;
    unsigned long mHeight;
    UInt8 *mData;
    CGImageRef mBitmap;

    BOOL isMonoColor;
    Color24 mMonoColor;
}

+ (ResultHolder *)resultHolderWithCGImage:(CGImageRef)image;
+ (ResultHolder *)resultHolderWithData:(UInt8 *)data Width:(unsigned long)width andHeight:(unsigned long)height;
+ (ResultHolder *)resultHolderWithMonoColor:(Color24)monoColor withWidth:(unsigned long)width andHeight:(unsigned long)height;

- (ResultHolder *)initWithData:(UInt8 *)data Width:(unsigned long)width andHeight:(unsigned long) height;
- (ResultHolder *)initWithCGImage:(CGImageRef)image;
- (ResultHolder *)initWithMonoColor:(Color24)monoColor withWidth:(unsigned long)width andHeight:(unsigned long)height;

- (BOOL)isSuccess;
- (UIImage *)bitmap;
- (void)combineFixResultHolder:(ResultHolder *)child Rect:(CGRect)bounds Width:(unsigned long)width andHeight:(unsigned long)height;
- (void)combineResultHolder:(ResultHolder *)child Bounds:(CGRect)bounds Width:(unsigned long)width andHeight:(unsigned long)height;
@end

ResultHolder.m

@implementation ResultHolder

@synthesize width = mWidth;
@synthesize height = mHeight;
@synthesize isMonoColor;
@synthesize monoColor = mMonoColor;

- (ResultHolder *)initWithData:(UInt8 *)data Width:(unsigned long)width andHeight:(unsigned long)height
{

    if (self = [super init]) {        
        mWidth = width;
        mHeight = height;
        mData = malloc(mWidth*mHeight*sizeof(Color24));
        memcpy(mData, data, mWidth*mHeight*sizeof(Color24));

        mBitmap = NULL;
    }

    return self;
}

- (ResultHolder *)initWithCGImage:(CGImageRef)image
{
    if (self = [super init]) {
        mBitmap = CGImageRetain(image);
        mWidth = CGImageGetWidth(image);
        mHeight = CGImageGetHeight(image);
    }
    return self;
}

- (ResultHolder *)initWithMonoColor:(Color24)monoColor withWidth:(unsigned long)width andHeight:(unsigned long)height
{
    if (self = [super init]) {
        mMonoColor = monoColor;

        isMonoColor = YES;
        mWidth = width;
        mHeight = height;
        mBitmap = NULL;
        mData = NULL;
    }

    return self;
}

+ (ResultHolder *)resultHolderWithCGImage:(CGImageRef)image
{
    ResultHolder *resultHolder = [[ResultHolder alloc] initWithCGImage:image];
    return resultHolder;
}

+ (ResultHolder *)resultHolderWithData:(UInt8 *)data Width:(unsigned long)width andHeight:(unsigned long)height
{
    ResultHolder *resultHolder = [[ResultHolder alloc] initWithData:data Width:width andHeight:height];
    return resultHolder;
}

+ (ResultHolder *)resultHolderWithMonoColor:(Color24)monoColor withWidth:(unsigned long)width andHeight:(unsigned long)height
{
    ResultHolder *resultHolder = [[ResultHolder alloc] initWithMonoColor:monoColor withWidth:width andHeight:height];
    return resultHolder;
}

- (BOOL)isSuccess
{
    if ([ReaderConfigures CodecDebug])
        NSLog(@"ResultHolder isSuccess");
    return (mData != NULL || isMonoColor || mBitmap != nil);
}

- (void)fillMonoColor
{

    if (isMonoColor) {
        if (mData) {
            free(mData);
        }
        mData = (UInt8 *)malloc(mWidth*mHeight*sizeof(Color24));

        for (int i = 0; i < mHeight; i++) {
            for (int j = 0; j < mWidth; j++) {
                memcpy(mData+(i*mWidth+j)*3, &mMonoColor, sizeof(Color24));
            }
        }
        isMonoColor = NO;
    }
}


- (void)extractBitmap  
{
    if (mBitmap) {

        CGDataProviderRef dataProvider = CGImageGetDataProvider(mBitmap);
        CFDataRef bitmapData = CGDataProviderCopyData(dataProvider);
        UInt8 * dataSource = (UInt8 *)CFDataGetBytePtr(bitmapData);

        size_t width = CGImageGetWidth(mBitmap);
        size_t height = CGImageGetHeight(mBitmap);
        if(mData)
            free(mData);
        mData = malloc(width*height*3);

        for (int i = 0; i < height; i++) {
            for (int j = 0; j < width; j++) {
                memcpy(mData+(i*width+j)*3, dataSource+(i*width+j)*4, sizeof(Color24));
            }
        }

        CFRelease(bitmapData);
        CGImageRelease(mBitmap);
        mBitmap = NULL;
    }
}


- (UInt8 *)getRawData
{

    if (mBitmap) {
        [self extractBitmap];
    }
    if (isMonoColor) {
        [self fillMonoColor];
    }

    return mData;
}

- (UIImage *)bitmap
{
    if (mBitmap) {
        UIImage *image = [[UIImage alloc] initWithCGImage:mBitmap];
        CGImageRelease(mBitmap);
        mBitmap = NULL;
        return image; 
    }
    if (isMonoColor) {
        [self fillMonoColor];
    }
    if (mData) {
        CGDataProviderRef dataProvider = CGDataProviderCreateWithData(NULL, mData, mWidth*mHeight*3, NULL);
        CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
        CGImageRef bitmap = CGImageCreate(mWidth, mHeight, 8, 24, mWidth*3, colorSpace, kCGBitmapByteOrderDefault, dataProvider, NULL, YES, kCGRenderingIntentDefault);
        CGColorSpaceRelease(colorSpace);
        CGDataProviderRelease(dataProvider);
        UIImage *image = [[UIImage alloc] initWithCGImage:bitmap];
        CGImageRelease(bitmap);

        return image;
    }

    return nil;
}

- (void)combineResultHolder:(ResultHolder *) child Bounds:(CGRect) bounds Width:(unsigned long)width andHeight:(unsigned long)height
{
    CGRect rect = CGRectMake(MAX(0, bounds.origin.x), MAX(0, bounds.origin.y),MIN(width - 1, bounds.origin.x + bounds.size.width), MIN(height - 1, bounds.origin.y + bounds.size.height));

    int w = MIN(rect.size.width + 1, child.width);
    int h = MIN(rect.size.height + 1, child.height);

    int dstPos = (height - 1 - (rect.origin.y + h - 1))*width;

    UInt8 *dataParent = [self getRawData];

    if (child.isMonoColor) {
        Color24 childMonoColor = child.monoColor;
        for (int i = 0; i < h; i++) {
            memcpy(dataParent+(dstPos+(int)rect.origin.x)*3, &childMonoColor, w*3);
            dstPos += width;
        }

    } else {
        UInt8 *dataChild = [child getRawData];
        if (dataChild != nil) {
            int srcPos = 0;
            for (int i = 0; i < h; i++) {
                memcpy(dataParent+dstPos*3+((int)rect.origin.x)*3, dataChild+srcPos*3, w*3);
                srcPos += child.width;
                dstPos += width;
            }
        }

    }

}

- (void)combineFixResultHolder:(ResultHolder *)child Rect:(CGRect)bounds Width:(unsigned long)width andHeight:(unsigned long)height
{
    CGRect rect = CGRectMake(bounds.origin.x, height-1-bounds.origin.y-bounds.size.height, bounds.origin.x+bounds.size.width, height-1-bounds.origin.y);

    [self combineResultHolder:child Bounds:rect Width:width andHeight:height];
}

- (void)dealloc
{
    if (mData) {
        free(mData);
        mData = NULL;
    }
    if (mBitmap) {
        CGImageRelease(mBitmap);
        mBitmap = NULL;
    }
}

@end

for simple image, for example JPEG image only, + (ResultHolder *)resultHolderWithCGImage:(CGImageRef)image; and - (UIImage *)bitmap; methods are called. for some complicated ones, ResultHolder will extract mBitmap to mData, and then combine with sub resultHolder's mData to get the image. these methods work well if I load image in my main thread, but if I use GCD or NSThread to load image in background it is easy to crash, especially when loading complicated ones in background. when the app crashes, the main thread state a CGSConvertBGR888toRGBA8888 method error, one of other threads is running the [ResultHolder dealloc] method, actually is free(mData). It seems there is a memory conflict between the loading thread and the main thread.

when the app crashes, the error is like this:

I have struggled for this bug for days, but still cannot find how to fix it. I do hope someone can help me. Any suggestions are appreciated.

UPDATE: I make a demo project ReaderDemo to simulate the situation. If you are interested, you can download to see the error. There are 15 images in this project, the 5,7,14 images will cause the crash when scrolling, they are a little complicated than others. but if you scroll through thumbnail scrollview then click, they all can be displayed.

Answer 1

You have a number of problems but lets start off with the first I found:

1. Improper test

   if (index > [mPageNames count]) {
   

   That needs to be >= or you crash.

2. you are calling dispatch_sync on the mainQueue, that does not seem to be a good decision (but maybe you have a really good one) - I changed it to async, seems to work OK

3. If you enable exceptions in this project it will really help you. Click the Break Points button in the Xcode toolbar. Then select the BreakPoints option left pane, second from the right. Tap the bottom left '+' icon and add an All Exceptions breakpoint. Now when you run the debugger stops where the problem occurrs.

4. I got a final crash that I'll let you fix:

   2012-09-26 08:55:12.378 ReaderDemo[787:11303] MFJAtIndex index out of bounds,index:15,bounds:15
   2012-09-26 08:55:12.379 ReaderDemo[787:11303] *** Assertion failure in -[ImageScrollView showLoadingForMFJ:], /Volumes/Data/Users/dhoerl/Downloads/ReaderDemo/ReaderDemo/ImageScrollView.m:247
   

This should get you on your way.

---

EDIT: Your problem relates to the management of the mData memory. You are trying to manage the lifecycle of it in your code, but this management is not sync'd with the CGImageDataProvider that is trying to use it. The crash is almost for sure (meaning I'm 99.99% convinced) a byproduct of the CGImageProvided created by CGDataProviderCreateWithData trying to access the data after your class has freed that memory in dealloc. I have had similar experiences with data providers.

The proper solution is to remove all free(data) calls, or at least most of them. Given the current structure of your code you will need to think about this carefully - you may want to replaced all the tests and malloc/frees with a flag. In the end, what you want to do is once the memory pointer is handed ovdr to CGDataProviderCreateWithData, you need to NULL out mData and let the data provider handle the removal.

The way to do this is to provide a function pointer to CGDataProviderCreateWithData in the past parameter:

CGDataProviderReleaseDataCallback
A callback function that releases data you supply to the function CGDataProviderCreateWithData.

typedef void (*CGDataProviderReleaseDataCallback) (
   void *info,
   const void *data,
   size_t size
);

All that function needs to do is just call free(data);. So whenever the data provider is done with the allocated memory, it will free it (and you don't need to worry about it).

Answer 2

If you want to free() or release your resources in any class in ARC enabled environment, you have to set proper flags for your class in 'Build Phases'. To do that, select your project file in XCode, select your target, go to 'Build Phases' section, find your class, and put -fno-objc-arc flag for that class. Or, maybe another reason, you are calling some CoreGraphics function that has to be called from main thread only in another thread.