Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Fix leak of image data in CGImageGetDataProvider#1710

Merged
aballway merged 3 commits into
microsoft:developfrom
aballway:develop
Jan 18, 2017
Merged

Fix leak of image data in CGImageGetDataProvider#1710
aballway merged 3 commits into
microsoft:developfrom
aballway:develop

Conversation

@aballway

@aballway aballway commented Jan 17, 2017

Copy link
Copy Markdown
Contributor

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

Comment thread Frameworks/CoreGraphics/CGImage.mm Outdated
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@msft-Jeyaram msft-Jeyaram Jan 17, 2017

Copy link
Copy Markdown

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
Copy Markdown
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.

(unsigned char*)[imageByteData bytes],
&inputImage);
[imageByteData release];
CGDataProviderRelease(provider);

Copy link
Copy Markdown

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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants