Wait for Go To Definition through the threaded-wait dialog - #20482
Open
xperiandri wants to merge 3 commits into
Open
Wait for Go To Definition through the threaded-wait dialog#20482xperiandri wants to merge 3 commits into
xperiandri wants to merge 3 commits into
Conversation
IFSharpGoToDefinitionService.TryGoToDefinition is a synchronous contract Roslyn calls on the UI thread, so the main thread has to wait for the checker. It did so with a bare Task.Wait, which pumps nothing: the VS watchdog showed "Please wait for an editor command to finish" after two seconds and auto-cancelled, while the check itself, and the snapshot version walk under its lazies, kept running on the pool. Pressing F12 again queued another waiter behind the same lazies, and the dialog stealing focus pushed the main thread into a focus-lost handler that blocked on the JTF context lock, so tagger work was cancelled and semantic classification never arrived. Wait the way NavigateTo in the same file already does, through JoinableTaskFactory.Run with the threaded-wait dialog, which keeps the main thread pumping and gives the user a Cancel button. The Roslyn token and the dialog token are linked so either cancels the check. The two TaskCompletionSource bridges in CancellableTasks and RoslynHelpers were created with TaskCreationOptions.None, so TrySetResult ran every awaiting continuation inline on whichever thread finished the F# async - the heavy post-check work of a navigation landed on the pool thread that completed the check. RunContinuationsAsynchronously moves those continuations to the pool instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Under --optimize+ the outer task inlines the inner builder's Bind into its own resumable body, and the inner __resumableEntry then reaches IlxGen as a bare value: FS3401 on every Windows CI job, while Debug builds compiled the same code. Build the single cancellableTask the way NavigateTo does and hand it the linked CancellationTokenSource to dispose, so there is one builder and nothing to leak. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
Repeated F12 on a large solution (89 projects) left files without semantic colours and VS showing "Please wait for an editor command to finish… Exceeded execution timeout, attempting to auto cancel". Breakpoints on the classification recovery paths never fired — classification was not failing, it was never reached.
What was happening.
IFSharpGoToDefinitionService.TryGoToDefinitionis a synchronousboolcontract that Roslyn calls on the UI thread, so the main thread has to wait for the checker. It did so with a bareTask.Wait()(GoToDefinition.fs), which pumps nothing: after two seconds the VS watchdog showed the dialog and auto-cancelled, but the check — and the project-snapshot version walk under itslazyvalues, which is not cancellable — kept running on the pool. Each further F12 queued another waiter behind the same lazies (7 threads observed inProjectSnapshot…GetVersion, 2 inSignatureVersion). The dialog stealing focus then pushed the main thread into a focus-lost handler (WpfTextView.OnIsKeyboardFocusWithinChanged → UpdateCommandUIContext → QueryService → JTF.Run) that blocked on the JTF context lock together with 12 other threads, soTaggerMainThreadManagercancelled its work items and no colours arrived.Changes.
GoToDefinition.fs—TryGoToDefinitionwaits the wayNavigateToin the same file already does:ThreadHelper.JoinableTaskFactory.Runwith the threaded-wait dialog, which keeps the main thread pumping and gives the user a Cancel button. The Roslyn token and the dialog token are linked so either cancels the check. A user cancel returnsfalseinstead of reporting a telemetry fault.CancellableTasks.fs,RoslynHelpers.fs— the twoTaskCompletionSourcebridges were created withTaskCreationOptions.None, soTrySetResultran every awaiting continuation inline on whichever thread finished the F# async; the heavy post-check work of a navigation landed on the pool thread that completed the check.RunContinuationsAsynchronouslymoves those continuations to the pool.No issue is filed for this; the diagnosis came from a live debugging session in the experimental instance.
Checklist
docs/release-notes/.VisualStudio/18.vNext.md(PR link added in a follow-up commit).🤖 Generated with Claude Code