-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix for SR-6849 and SR-6962 leak in NSData and NSMutableData #1455
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
Conversation
@@ -69,11 +69,10 @@ private final class _NSDataDeallocator { | |||
|
|||
private let __kCFMutable: CFOptionFlags = 0x01 | |||
private let __kCFGrowable: CFOptionFlags = 0x02 | |||
private let __kCFMutableVarietyMask: CFOptionFlags = 0x03 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__kCFMutableVarietyMask is not used anymore.
private let __kCFBytesInline: CFOptionFlags = 0x04 | ||
private let __kCFUseAllocator: CFOptionFlags = 0x08 | ||
private let __kCFDontDeallocate: CFOptionFlags = 0x10 | ||
private let __kCFAllocatesCollectable: CFOptionFlags = 0x20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__kCFAllocatesCollectable is not used anymore.
|
||
private let __kCFBytesInline: CFOptionFlags = 2 | ||
private let __kCFUseAllocator: CFOptionFlags = 3 | ||
private let __kCFDontDeallocate: CFOptionFlags = 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed for bitmasks to integers on CoreFoundation's CFData.c
@swift-ci please test |
Thank you @mamabusi. I have a feeling this is one of those changes that looks simple but took forever to track down. |
Approved for 4.1 branch as well. |
@swift-ci please test and merge |
Test case:
Valgrind memcheck tool output before the fix (as reported in SR-6849):
Valgrind memcheck tool output after the fix:
Summary:
This is a regression from the df3ec55 commit for the High Sierra changes.
Previously, PR: #380 had a fix for NSData leak where
__CFDataGetInfoBit
for__kCFDontDeallocate
(then equivalent of__CFDataDontDeallocate
before PR for High Sierra changes) was set on an ‘if’ condition in_CFDataInit
method and it was in turn checked in the__CFDataDeallocate
method ofCFData
during deallocation.However, with the HighSierra PR 1338 changes now, the value for
__CFDataDontDeallocate
is hardcoded to'true'
__CFDataSetDontDeallocate(memory, true);
With this, data is never deallocated and hence the leak.