Skip to content

[SourceKit] Make each Variant keep a strong reference to its SourceKitdResponse context #19689

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

nathawes
Copy link
Contributor

@nathawes nathawes commented Oct 3, 2018

sourcekitd_variant_t is only safe to use while the sourcekitd_response_t it was retrieved from is still alive, so keep a strong reference to SourceKitdResponse (the Swift wrapper of sourcekitd_response_t) in each Array, Dictionary and Variant (Swift wrappers of sourcekitd_variant_t).

@nathawes nathawes requested a review from nkcsgexi October 3, 2018 17:44
@nathawes
Copy link
Contributor Author

nathawes commented Oct 3, 2018

@swift-ci please smoke test

@@ -17,12 +17,14 @@ import sourcekitd
public class SourceKitdResponse: CustomStringConvertible {

public struct Dictionary: CustomStringConvertible, CustomReflectable {
private let context: SourceKitdResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! Could you add some comments why we need a strong reference to the response object?

…tdResponse context

sourcekitd_variant_t is only safe to use while the sourcekitd_response_t it was
retrieved from is still alive, so keep a strong reference to SourceKitdResponse
(the Swift wrapper of sourcekitd_response_t) in each Variant (the Swift wrapper
of sourcekitd_variant_t).
@nathawes nathawes force-pushed the keep-sourcekitd-response-alive-while-variant-lives branch from 3dcff94 to 88b1ae9 Compare October 3, 2018 18:07
@nathawes
Copy link
Contributor Author

nathawes commented Oct 3, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Oct 3, 2018

Build failed
Swift Test OS X Platform
Git Sha - 3dcff94539c91b7c8e826e365aa7c3de81645c8b

@nathawes
Copy link
Contributor Author

nathawes commented Oct 3, 2018

Thanks for reviewing @nkcsgexi! Added the comments.

@swift-ci
Copy link
Contributor

swift-ci commented Oct 3, 2018

Build failed
Swift Test Linux Platform
Git Sha - 3dcff94539c91b7c8e826e365aa7c3de81645c8b

@nathawes nathawes merged commit 700384a into swiftlang:master Oct 3, 2018
@nathawes nathawes deleted the keep-sourcekitd-response-alive-while-variant-lives branch October 3, 2018 23:11
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.

3 participants