Skip to content

[LSP] Race in LSP document tracker when accessing tracked documents from non-mutating handlers #57276

@dibarbet

Description

@dibarbet

The normal document change tracker is not threadsafe - https://sourceroslyn.io/#Microsoft.CodeAnalysis.LanguageServer.Protocol/Handler/RequestExecutionQueue.DocumentChangeTracker.cs,67 and expects calls to it to be serial as it is updating document state.

The Nonmutating document change tracker is meant to be called in parallel as it is passed to LSP methods that are handled concurrently. It throws if state mutation is attempted. However, it accesses the mutating document change tracker (e.g. https://sourceroslyn.io/#Microsoft.CodeAnalysis.LanguageServer.Protocol/Handler/RequestExecutionQueue.DocumentChangeTracker.cs,44) which is a race and unsafe. This method is called in document pull diagnostics for example - https://sourceroslyn.io/#Microsoft.CodeAnalysis.LanguageServer.Protocol/Handler/Diagnostics/DocumentPullDiagnosticHandler.cs,71 (which runs concurrently with other requests).

The set of tracked documents at the time the request was de-queued must be passed immutably to the RequestContext. Since requests are de-queued serially, it is safe to access the mutating document change tracker. This also enforces that the request uses the tracked documents corresponding to the request's order in the queue.

Metadata

Metadata

Assignees

Labels

Area-IDELSPissues related to the roslyn language server protocol implementationuntriagedIssues and PRs which have not yet been triaged by a lead

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions