Skip to content

[stable/20221013] Proper fix for race in ClangdServer shutdown #5938

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
Jan 9, 2023

Conversation

bnbarham
Copy link

@bnbarham bnbarham commented Jan 9, 2023

Cherry-picks the upstream fix b494f67 and reverts the temporary one.

bnbarham and others added 2 commits January 9, 2023 09:30
In principle it's OK for stdlib-indexing tasks to run after the TUScheduler is
destroyed, as mostly they just update the dynamic index. We do drain the
stdlib-indexing queue before destroying the index.

However the task captures references to the PreambleCallbacks object, which is
owned by the TUScheduler. Once this is destroyed (explicitly, early in
~ClangdServer) an outstanding stdlib-indexing task may use-after-free.

The fix here is to avoid capturing references to the PreambleCallbacks.
Alternatives would be to have TUScheduler (exclusively) not own its callbacks
so they could live longer, or explicitly stopping the TUScheduler instead of
early-destroying it. These both seem more invasive.

See https://reviews.llvm.org/D115232 for some more context.

Differential Revision: https://reviews.llvm.org/D140486
@bnbarham
Copy link
Author

bnbarham commented Jan 9, 2023

@swift-ci please test

@bnbarham bnbarham merged commit b5a59f6 into swiftlang:stable/20221013 Jan 9, 2023
@bnbarham bnbarham deleted the replace-clangd-fix branch January 9, 2023 23:41
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