views:

620

answers:

3

I have a function that takes some bitmap data and returns a UIImage * from it. It looks something like so:

UIImage * makeAnImage() 
{
    unsigned char * pixels = malloc(...);
    // ...
    CGDataProviderRef provider = CGDataProviderCreateWithData(NULL, pixels, pixelBufferSize, NULL);
    CGImageRef imageRef = CGImageCreate(..., provider, ...);
    UIImage * image =  [[UIImage alloc] initWithCGImage:imageRef];
    return [image autorelease];
}

Can anyone explain exactly who owns what memory here? I want to clean up properly, but I'm not sure how to do it safely. Docs are fuzzy on these. If I free pixels at the end of this function after creating the UIImage, and then use the UIImage, I crash. If I Release the provider or the imageRef after creating the UIImage, I don't see a crash, but they're apparently passing the pixels all the way through, so I'm skittish about releasing these intermediate states.

(I know per CF docs that I should need to call release on both of the latter because they come from Create functions, but can I do that before the UIImage is used?) Presumably I can use the provider's dealloc callback to cleanup the pixels buffer, but what else?

Thanks!

A: 

Aye, this code makes me queasy. As an old rule-to-live by, I try not to mix and match C and C++, and C/Objective-C in the same function/method/selector.

How about breaking this up into two methods. Have change this makeAnImage into makeAnImageRef and pull up the UIImage creation into another Obj-C selector.

David Sowsy
Although I understand the reaction :) doing so still doesn't help my memory management issue. I still have the problem that passing raw bytes into the start of the pipeline leaves me with a UIImage that somehow still needs those bytes. So this code is all more related than it may seem.
quixoto
+3  A: 

The thumb rule here is "-release* it if you don't need it".

Because you no longer need provider and imageRef afterwards, you should -release all of them, i.e.

UIImage * image =  [[UIImage alloc] initWithCGImage:imageRef];
CGDataProviderRelease(provider);
CGImageRelease(imageRef);
return [image autorelease];

pixel is not managed by ref-counting, so you need to tell the CG API to free them for you when necessary. Do this:

void releasePixels(void *info, const void *data, size_t size) {
   free((void*)data);
}
....

CGDataProviderRef provider = CGDataProviderCreateWithData(NULL, pixels, pixelBufferSize, releasePixels);

By the way, you can use +imageWithCGImage: instead of [[[* alloc] initWithCGImage:] autorelease]. Even better, there is +imageWithData: so you don't need to mess with the CG and malloc stuff.

(*: Except when the retainCount is already supposedly zero from the beginning.)

KennyTM
Thanks Kenny. That's a nicely succinct description; I think I was thrown a little by the unpredictability of the raw heap buffer, but as always, trust in the rules and ye shall be rewarded. Cheers.
quixoto
Just to add some clarification, the key words in Core functions are "Create" and "New". If the function contains either of these words, you must release the memory returned.Most Core data types are CFType compatible. Which means you can use the Objective-C retain/release/autorelease calls on them if its easier. i.e. [(id)imageRef release]; or CFRelease(imageRef);
Matt Gallagher
Remember to check if `imageRef` is `NULL` if you use `CFRelease`.
KennyTM
+2  A: 
unsigned char * pixels = malloc(...);

You own the pixels buffer because you mallocked it.

CGDataProviderRef provider = CGDataProviderCreateWithData(NULL, pixels, pixelBufferSize, NULL);

Core Graphics follows the Core Foundation rules. You own the data provider because you Created it.

You didn't provide a release callback, so you still own the pixels buffer. If you had provided a release callback, the CGDataProvider object would take ownership of the buffer here. (Generally a good idea.)

CGImageRef imageRef = CGImageCreate(..., provider, ...);

You own the CGImage object because you Created it.

UIImage * image =  [[UIImage alloc] initWithCGImage:imageRef];

You own the UIImage object because you allocked it.

You also still own the CGImage object. If the UIImage object wants to own the CGImage object, it will either retain it or make its own copy.

return [image autorelease];

You give up your ownership of the image.

So your code leaks the pixels (you didn't transfer ownership to the data provider and you didn't release them yourself), the data provider (you didn't release it), and the CGImage (you didn't release it). A fixed version would transfer ownership of the pixels to the data provider, and would release both the data provider and the CGImage by the time the UIImage is ready. Or, just use imageWithData:, as KennyTM suggested.

Peter Hosey
quixoto