-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[SR-3481] Fix for memory leak in NSURLComponents #897
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
[SR-3481] Fix for memory leak in NSURLComponents #897
Conversation
private let _components : CFURLComponentsRef! | ||
|
||
deinit { | ||
if let component = _components { | ||
__CFURLComponentsDeallocate(component) |
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.
Called CoreFoundation deallocate method to free the resources.
static void __CFURLComponentsDeallocate(CFTypeRef cf) { | ||
CFURLComponentsRef instance = (CFURLComponentsRef)cf; | ||
__CFGenericValidateType(cf, _CFURLComponentsGetTypeID()); | ||
CF_EXPORT void __CFURLComponentsDeallocate(CFURLComponentsRef instance) { |
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.
Used CF_EXPORT in-order to call from deinit
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.
Pleaes make this CF_SWIFT_EXPORT
Foundation/NSURL.swift
Outdated
@@ -948,8 +948,15 @@ open class NSURLQueryItem : NSObject, NSSecureCoding, NSCopying { | |||
} | |||
|
|||
open class NSURLComponents: NSObject, NSCopying { | |||
private let _cfinfo = _CFInfo(typeID: _CFURLComponentsGetTypeID()) |
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.
What's the purpose of this line?
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.
Realised it is no longer required. Will remove.
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.
Looks good with a couple small changes. Thanks!
static void __CFURLComponentsDeallocate(CFTypeRef cf) { | ||
CFURLComponentsRef instance = (CFURLComponentsRef)cf; | ||
__CFGenericValidateType(cf, _CFURLComponentsGetTypeID()); | ||
CF_EXPORT void __CFURLComponentsDeallocate(CFURLComponentsRef instance) { |
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.
Pleaes make this CF_SWIFT_EXPORT
@@ -30,6 +30,8 @@ typedef struct __CFURLComponents *CFURLComponentsRef; | |||
|
|||
CF_EXPORT CFTypeID _CFURLComponentsGetTypeID(void); | |||
|
|||
CF_EXPORT void __CFURLComponentsDeallocate(CFURLComponentsRef); |
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.
CF_SWIFT_EXPORT
here too.
@parkera Addressed the changes. |
@swift-ci please test and merge |
No description provided.