Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented May 13, 2025

A queued task might have been cancelled after the execution ask was started but before the task was yielded to executionTaskCreatedContinuation. In that case the result task will simply cancel the await on the executionTaskCreatedStream and hence not call valuePropagatingCancellation on the execution task. This means that the queued task cancellation wouldn't be propagated to the execution task. To address this, check if resultTaskCancelled was set and, if so, explicitly cancel the execution task here.

Fixes an issue I saw in CI during PR testing.

…kScheduler`

A queued task might have been cancelled after the execution ask was started but before the task was yielded to `executionTaskCreatedContinuation`. In that case the result task will simply cancel the await on the `executionTaskCreatedStream` and hence not call `valuePropagatingCancellation` on the execution task. This means that the queued task cancellation wouldn't be propagated to the execution task. To address this, check if `resultTaskCancelled` was  set and, if so, explicitly cancel the execution task here.

Fixes an issue I saw in CI during PR testing.
@ahoppen ahoppen requested review from bnbarham and hamishknight May 13, 2025 15:25
@ahoppen
Copy link
Member Author

ahoppen commented May 13, 2025

@swift-ci Please test

ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this pull request May 13, 2025
…down

This is what `shutDown()` is documented to do. I also remember having this check before, it might have gotten lost during a rebase when I was working on swiftlang#2081.

I noticed this while investigating swiftlang#2152: In this case `buildTarget/prepare` was cancelled because the SourceKit-LSP server was shut down but indexing of a file was still started after the shutdown now that preparation had finished (because it was cancelled).
@ahoppen ahoppen merged commit a78bc5e into swiftlang:main May 14, 2025
3 checks passed
@ahoppen ahoppen deleted the cancellation-missed branch May 14, 2025 17:21
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