Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix leak of image data in CGImageGetDataProvider #1710

Merged
merged 3 commits into from
Jan 18, 2017

Conversation

aballway
Copy link
Contributor

@aballway aballway commented Jan 17, 2017

CGImageGetDataProvider was copying the CFData rather than accessing it directly, and would leak the data for consumers expecting a +0 reference.

Blocks #1692


This change is Reviewable

woc::unique_cf<CFDataRef> data{ CFDataCreateWithBytesNoCopy(nullptr, pPtr, length, kCFAllocatorNull) };
CGDataProviderRef dataProvider = CGDataProviderCreateWithCFData(data.get());

//TODO 1709:: Autorelease dataProvider so it won't leak for consumers expecting a non-owning reference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong issue number here. Why the TODO, why not add this to autorelease pool with this fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue number is correct. Adding to the autorelease pool was causing the failures I made #1709 for.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so this is a caller released object that was extra released.

Copy link
Contributor

@msft-Jeyaram msft-Jeyaram Jan 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the actual fix is postponed till the merge?, is it too much work to actually fix the ImageIO test rather than introducing intentional bugs to satisfy bad test code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already been fixed on CGD2D, If it was a complex change I didn't want to have it done twice.

@@ -1031,7 +1031,6 @@ void CGImageDestinationAddImage(CGImageDestinationRef idst, CGImageRef image, CF
(unsigned char*)[imageByteData bytes],
&inputImage);
[imageByteData release];
CGDataProviderRelease(provider);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CGDataProviderRelease [](start = 4, length = 21)

Thanks! :D

@aballway aballway merged commit deb99af into microsoft:develop Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants