making language service apis parallel safe#2604
Open
aasimkhan30 wants to merge 8 commits intomainfrom
Open
Conversation
added 2 commits
March 6, 2026 22:02
…d Safety - Updated CreateFileName method in ScripterCore to append a GUID for unique file names. - Refactored GetPeekDefinitionTempFolder in FileUtils to ensure thread safety when creating temporary folders. - Added unit tests for completion resolution to ensure dispatcher does not block during binding operations. - Introduced tests for unique file name generation and thread safety in peek definition folder creation. - Enhanced existing tests for autocomplete and language service functionalities.
added 3 commits
March 6, 2026 22:42
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent SQL Tools Service LanguageService stalls by enabling parallel dispatch for language-service request/event handlers and adding targeted synchronization for shared per-URI and peek-definition state.
Changes:
- Enable parallel processing for LanguageService request/event handlers (signature help, hover, completion, definition, syntax parse, rebuild intellisense, language flavor change, token refresh).
- Add per-URI serialization for rebuild-intellisense and language-flavor change to avoid concurrent cache/diagnostics mutations.
- Make peek-definition temp folder creation and scripted filename generation safe under parallel execution (lock + GUID-based uniqueness) and add unit tests for these behaviors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/PeekDefinitionTests.cs | Adds concurrency/uniqueness unit tests for peek-definition temp folder and script filename generation. |
| src/Microsoft.SqlTools.ServiceLayer/Utility/FileUtils.cs | Makes peek-definition temp folder creation thread-safe and uses GUID suffix for folder name. |
| src/Microsoft.SqlTools.ServiceLayer/Scripting/ScripterCore.cs | Ensures scripted peek-definition filenames are unique per request via GUID suffix. |
| src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs | Enables parallel dispatch, adds per-URI async locking for rebuild/flavor handlers, and introduces completion cancel-previous behavior. |
| src/Microsoft.SqlTools.ServiceLayer/LanguageServices/Completion/CompletionService.cs | Moves star-expansion into completion creation flow and adds verbose logging. |
Comments suppressed due to low confidence (1)
src/Microsoft.SqlTools.ServiceLayer/LanguageServices/Completion/CompletionService.cs:181
CreateCompletionsFromSqlParserassumesconnInfois non-null (e.g.,connInfo.IntellisenseMetrics.UpdateMetrics(...)below) butconnInfois passed through fromLanguageServiceand may be null in some flows. This can lead toNullReferenceExceptionwhile building completions, causing the request to stall/fail. Either require non-nullconnInfoat the API boundary or add null-safe handling (skip metrics + star-expansion, return defaults) when it is null.
// Expanding star expressions in query only when the script is connected to a database
// as the parser requires a connection to determine column names
if (connInfo != null)
{
CompletionItem[] starExpansionSuggestion = AutoCompleteHelper.ExpandSqlStarExpression(scriptDocumentInfo);
if (starExpansionSuggestion != null)
{
completionList = [.. starExpansionSuggestion, .. completionList];
}
}
result.CompleteResult(completionList);
//The bucket for number of milliseconds will take to send back auto complete list
connInfo.IntellisenseMetrics.UpdateMetrics(result.Duration, 1, (k2, v2) => v2 + 1);
return result;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.SqlTools.ServiceLayer/LanguageServices/Completion/CompletionService.cs
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Addressing: microsoft/vscode-mssql#21361
While the bug is still not identified, this is best effort solution to make sts not stall.
All Language Service request and event handlers are now registered with
isParallelProcessingSupported: true, so they no longer serialize on the single-threaded dispatcher queue. Previously, a slow handler (e.g. a completion request waiting on the binding queue) would stall every other dispatcher message behind it.Changes
1. Parallel dispatch for all handlers
Every
SetRequestHandler/SetEventHandlercall inInitializeServicenow passesisParallelProcessingSupported: true. Before this change, a single comment disabled parallelism for all of them:2. URI-serialized rebuild and language-flavor notifications
HandleRebuildIntelliSenseNotificationandHandleDidChangeLanguageFlavorNotificationboth mutate the per-URI intellisense cache (ScriptParseInfo,nonMssqlUriMap, diagnostics). To prevent race conditions where two concurrent rebuilds for the same document overwrite each other, both handlers now go throughRunSerializedByUriAsync, which holds a per-URIAsyncLock. Different URIs still run fully in parallel.3. Signature help and hover
HandleSignatureHelpRequestpreviously used a fire-and-forgetTask.Factory.StartNew+ContinueWithpattern to avoid blocking the dispatcher. With parallel dispatch enabled, the handler can simplyawaitthe work directly — the dispatcher is already unblocked.HandleHoverRequesthad a similar structure and was simplified the same way.4. Peek definition thread safety
HandleDefinitionRequestscripts objects to temp files. Two changes make this safe under parallel dispatch:FileUtilities.GetPeekDefinitionTempFolder()is now guarded by alockso the one-time temp folder creation is not raced by concurrent requests.ScripterCore.CreateFileNameappends a per-request GUID to every generated filename, so two concurrent peek-definition requests for the same object don't overwrite each other's script files.5. Syntax parse and token refresh
HandleSyntaxParseRequestis pure computation (parse input text, return errors) with no shared state mutation — trivially safe.HandleTokenRefreshedNotificationupdates connection/token state backed by concurrent collections.6. Completion request cancellation
Completion is registered as parallel-capable but uses a cancel-previous pattern rather than true parallelism. On each new
CompletionRequest:CancelPreviousCompletionRequest()atomically swaps in a newCancellationTokenSourceviaInterlocked.Exchange, cancelling and disposing the old one.GetCompletionItems, which checkscancellationToken.IsCancellationRequestedafter the expensiveParseAndBindstep and bails out early if superseded.CompletionResolveRequestis parallel-capable and only operates on the latest completion result cached incurrentCompletionParseInfo.7. Star-expansion completion improvements (AutoCompleteHelper)
Refactored star-expansion (
SELECT *) completion items with improved presentation and multiline formatting.Code Changes Checklist
dotnet test)Reviewers: Please read our reviewer guidelines