Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 15, 2024

As a user makes an edit to a file, these requests are most likely no longer relevant. It also makes sure that a long-running sourcekitd request can't block the entire language server if the client does not cancel all requests. For example, consider the following sequence of requests:

  • textDocument/semanticTokens/full for document A
  • textDocument/didChange for document A
  • textDocument/formatting for document A

If the editor is not cancelling the semantic tokens request on edit (like VS Code does), then the didChange notification is blocked on the semantic tokens request finishing. Hence, we also can't run the textDocument/formatting request. Cancelling the semantic tokens on the edit fixes the issue.

rdar://133987424

…or closed

As a user makes an edit to a file, these requests are most likely no longer relevant. It also makes sure that a long-running sourcekitd request can't block the entire language server if the client does not cancel all requests. For example, consider the following sequence of requests:
 - `textDocument/semanticTokens/full` for document A
 - `textDocument/didChange` for document A
 - `textDocument/formatting` for document A

If the editor is not cancelling the semantic tokens request on edit (like VS Code does), then the `didChange` notification is blocked on the semantic tokens request finishing. Hence, we also can't run the `textDocument/formatting` request. Cancelling the semantic tokens on the edit fixes the issue.

rdar://133987424
@ahoppen
Copy link
Member Author

ahoppen commented Aug 15, 2024

@swift-ci Please test

Comment on lines +1283 to +1285
guard self.options.cancelTextDocumentRequestsOnEditAndCloseOrDefault else {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could just check in the outer method before queuing anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because options is isolated to the SourceKitLSPServer instance and the outer cancelTextDocumentRequests method is nonisolated.

@ahoppen ahoppen merged commit dd9c0af into swiftlang:main Aug 17, 2024
@ahoppen ahoppen deleted the implicit-request-cancellation branch August 17, 2024 01:18
ahoppen added a commit to ahoppen/sourcekit-lsp that referenced this pull request Jan 4, 2026
Instead, use `staleRequestSupport.retryOnContentModified` as an indicator which requests should return a `ContentModified` error when the document is modified. This fixes swiftlang#2136 by no longer implicitly cancelling inlayHints in Helix while still implicitly cancelling semantic tokens request, which was the motivation for swiftlang#1629.

Fixes swiftlang#2136
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