struct memory allocation (leaks and crash, profiling on iPhone)

Question

I have a struct defined as:

struct VImage
{
    uint8_t *imageData;
    int dimensions[3];
    ...
}

I was profiling my code to look for leaks and other issues. Consider the following code:

for (int i = 0; i < 100; i++)
{
    // after we have the image, we use the cheap rgb function to get rgb data
    struct VImage *vImage = [SomeClass someFunction:someArgument];
    free(vImage);
}

This crashes my code. And I am wondering why that is the case. The condition i < 10 however doesn't crash the app. Profiling on instruments doesn't show that there is any leak; however the allocations spike and I am using a lot of live memory; so I am wondering if freeing the struct in this way is right OR if free is an async function that will free the memory at someone, but not just when I would ask it to.

EDIT: So I have been asked to provide details for this someFunction method and I will. I think there is no memory leak in the method but nonetheless, I will put it here:

+ (struct VImage *)grayScaleImageDataFromImage:(UIImage *)image
{
    int imageWidth = image.size.width, imageHeight = image.size.height;

    uint32_t *rgbImageData = (uint32_t *) malloc(imageWidth * imageHeight * sizeof(uint32_t));
    memset(rgbImageData, 0, imageWidth * imageHeight * sizeof(uint32_t));

    CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();

    // create context with RGBA pixels
    CGContextRef context = CGBitmapContextCreate(rgbImageData, imageWidth, imageHeight, 8, imageWidth * sizeof(uint32_t), colorSpace, kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipLast);
    CGContextSetInterpolationQuality(context, kCGInterpolationHigh);
    CGContextSetShouldAntialias(context, NO);

    // paint to fill pixels array
    CGContextDrawImage(context, CGRectMake(0, 0, imageWidth, imageHeight), [image CGImage]);
    CGContextRelease(context);
    CGColorSpaceRelease(colorSpace);

    // now convert to grayscale using CCIR 601 <=> Y' = 0.299 R' + 0.587 G' + 0.114 B'.
    uint8_t *grayScaleImageData = (uint8_t *) malloc(imageWidth * imageHeight);
    for(int y = 0; y < imageHeight; y++)
    {
        for(int x = 0; x < imageWidth; x++)
        {
            uint32_t rgbPixel = rgbImageData[y*imageWidth+x];
            grayScaleImageData[y*imageWidth + x] = 0.299*((rgbPixel>>24)&255) + 0.587*((rgbPixel>>16)&255) + 0.114*((rgbPixel>>8)&255);
        }
    }

    struct VImage *vImage = createGrayScaleDescription(grayScaleImageData, imageHeight, imageWidth);

    free(rgbImageData);
    return vImage;
}

struct VImage *createGrayScaleDescription(uint8_t *grayScaleImage, int height, int width)
{
    struct VImage *vImage = malloc(sizeof(struct VImage));
    vImage->grayScaleImage = grayScaleImage;
    vImage->numberOfDimensions = 2;
    vImage->dimensions[0] = height, vImage->dimensions[1] = width, vImage->dimensions[2] = 1;
    vImage->isGrayscale = true;
    return vImage;
}

So all I am doing is creating a grayscale image using rgb data by doing some computation. However, I don't think there's any problem here.

Answer 1

For one thing, you are leaking the memory pointed to by grayScaleImageData:

In grayScaleImageDataFromImage you do

uint8_t *grayScaleImageData = (uint8_t *) malloc(imageWidth * imageHeight);

which is fine, then in createGrayScaleDescription you set vImage->grayScaleImage to point to that memory, which is also fine.

But then in your 1 to 100 loop, you free vImage without freeing the memory pointed to by vImage->grayScaleImage first. That is a memory leak.

I don't think that should be crashing your application, but it is something you need to fix. Nothing else jumps out at me, but I'll take a closer look.

EDIT: On closer inspection, it looks like you are overrunning the buffer you allocated for grayScaleImageData.

You malloc it with:

uint8_t *grayScaleImageData = (uint8_t *) malloc(imageWidth * imageHeight);

but then you fill that buffer with:

uint32_t rgbPixel = rgbImageData[y*imageWidth+x];
grayScaleImageData[y*imageWidth + x] = 0.299*((rgbPixel>>24)&255) + 0.587*((rgbPixel>>16)&255) + 0.114*((rgbPixel>>8)&255);

You've allocated (imageWidth * imageHeight) 8-bit uints, but then you fill it with 32-bit uints.

Perhaps that is what is causing you to crash? Maybe try changing that malloc to

uint32_t *grayScaleImageData = (uint32_t *) malloc(imageWidth * imageHeight * sizeof (uint32_t));

as you did above for rgbImageData? That, or cast the result of your bit shifting calculations to a uint8_t before setting it into grayScaleImageData:

uint32_t rgbPixel = rgbImageData[y*imageWidth+x];
grayScaleImageData[y*imageWidth + x] = (uint8_t)(0.299*((rgbPixel>>24)&255) + 0.587*((rgbPixel>>16)&255) + 0.114*((rgbPixel>>8)&255));