Skip to content

[SyntaxParse] Prevent memory leak in Syntax parsing #27132

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

Merged

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Sep 12, 2019

ParsedRawSyntaxNode holds opaque pointer void * pointer for "recorded" node. The parser doesn't know how the memory is managed. When any ParsedRawSyntaxNode holding "recorded" node is discarded, that leaks because the parser doesn't know how to release it.

In this patch, to prevent that kind of leaking, make ParsedRawSyntaxNode move-only, nullify the original node when moved, assert the it does not hold recorded node in the destructor.

So that we can easily detect 'ParsedSyntaxNode' leaking. When it's
moved, the original node become "null" node. In the destructor of
'ParsedSyntaxNode', assert the node is not "recorded" node.
Add SyntaxParseActions::discardRecordedNode() as a workaround for memory
leak during syntax parsing migration.
@rintaro
Copy link
Member Author

rintaro commented Sep 12, 2019

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Sep 12, 2019

@swift-ci Please Sourcekit Stress test

@rintaro
Copy link
Member Author

rintaro commented Sep 13, 2019

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Sep 13, 2019

@swift-ci Please smoke test macOS

@rintaro rintaro merged commit e675c49 into swiftlang:master Sep 13, 2019
@compnerd
Copy link
Member

@rintaro this may have broken the Windows builds:

C:\agent\_work\1\s\swift\lib\Parse\ParseType.cpp(411): error C2280: 'swift::ParsedTypeSyntax::ParsedTypeSyntax(swift::ParsedTypeSyntax &)': attempting to reference a deleted function
C:/agent/_work/1/s/swift/include/swift/Parse/ParsedSyntaxNodes.h.gyb(82): note: compiler has generated 'swift::ParsedTypeSyntax::ParsedTypeSyntax' here
C:/agent/_work/1/s/swift/include/swift/Parse/ParsedSyntaxNodes.h.gyb(82): note: 'swift::ParsedTypeSyntax::ParsedTypeSyntax(swift::ParsedTypeSyntax &)': function was implicitly deleted because a base class invokes a deleted or inaccessible function 'swift::ParsedSyntax::ParsedSyntax(swift::ParsedSyntax &)'
C:\agent\_work\1\s\swift\include\swift/Parse/ParsedSyntax.h(68): note: 'swift::ParsedSyntax::ParsedSyntax(swift::ParsedSyntax &)': function was implicitly deleted because a data member invokes a deleted or inaccessible function 'swift::ParsedRawSyntaxNode::ParsedRawSyntaxNode(swift::ParsedRawSyntaxNode &)'
C:\agent\_work\1\s\swift\include\swift/Parse/ParsedRawSyntaxNode.h(100): note: 'swift::ParsedRawSyntaxNode::ParsedRawSyntaxNode(swift::ParsedRawSyntaxNode &)': function was explicitly deleted

https://dev.azure.com/compnerd/windows-swift/_build/results?buildId=8674&view=logs&j=2d2b3007-3c5c-5840-9bb0-2b1ea49925f3&t=2915bb0c-b896-5031-2bc6-f61128a41015&l=947

compnerd added a commit to compnerd/apple-swift that referenced this pull request Sep 14, 2019
The type deduction here will copy the returned ParsedTypeSyntax which
is no longer copy constructible. Use a reference to hold the result
instead which should fix the build on Windows.  The returned value of
the `popToken` returns a `ParsedTokenSyntax` which is no longer copy
constructible.  Explicitly move the value.
compnerd added a commit that referenced this pull request Sep 14, 2019
CodaFi added a commit to CodaFi/swift that referenced this pull request Sep 16, 2019
…arsedrawsyntax-moveonly"

This reverts commit e675c49, reversing
changes made to 73315ba.
Catfish-Man added a commit that referenced this pull request Sep 16, 2019
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.

2 participants