Skip to content

[sourcekitd] Fix range shifting vs semantic update timing issues #33063

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
merged 2 commits into from
Jul 23, 2020

Conversation

benlangmuir
Copy link
Contributor

  • If a semantic update finishes fast enough, the token snapshot may be identical to the edit snapshot, but because of getBufferForSnapshot consolidating edits into a new buffer, we were not detecting that case properly, and it could cause an assertion failure (or potentially incorrect range shifting in a release build). This would have reproduced very rarely in practice, but I can reproduce it by putting sleep(2) calls right before we read the semantic info in open and edit requests.
  • Fix sourcekit-test and unit tests for the (rare) case where an open or edit already has updated semantic info.
  • Fix editing test previously disabled so that semantic tokens will be the same regardleess of whether they come from range-shifting or from a full semantic update.

…date

The goal of the test is to test the behaviour when the edit is
range-shifted, but in the (rare) case where the document update happens
before the edit finishes, we need the ranges to be the same. In
particular, using separate statements ensures that the tokens not
touched by the edit are not affected by the edit.

Re-enable the test disabled on ASan, since this seems to be the
underlying issue.

rdar://65934938
If a semantic update finishes fast enough, the token snapshot may be
identical to the edit snapshot, but because of getBufferForSnapshot
consolidating edits into a new buffer, we were not detecting that case
properly, and it could cause an assertion failure (or potentially
incorrect range shifting in a release build). This would have reproduced
very rarely in practice, but I can reproduce it by putting `sleep(2)`
calls right before we read the semantic info in open and edit requests.

Incidentally, fix sourcekit-test and unit tests for the (rare) case
where an open or edit already has updated semantic info.
@benlangmuir benlangmuir requested review from akyrtzi and nathawes July 23, 2020 00:53
@benlangmuir
Copy link
Contributor Author

@swift-ci please asan test

@benlangmuir
Copy link
Contributor Author

@swift-ci please test

@benlangmuir
Copy link
Contributor Author

Windows failure is unrelated, and seems to have been fixed already.

@benlangmuir benlangmuir merged commit 589b919 into swiftlang:master Jul 23, 2020
@benlangmuir benlangmuir deleted the racing-edit branch July 23, 2020 15:58
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