Skip to content

Wait for Go To Definition through the threaded-wait dialog - #20482

Open
xperiandri wants to merge 3 commits into
dotnet:mainfrom
xperiandri:fix-goto-definition-main-thread-wait
Open

Wait for Go To Definition through the threaded-wait dialog#20482
xperiandri wants to merge 3 commits into
dotnet:mainfrom
xperiandri:fix-goto-definition-main-thread-wait

Conversation

@xperiandri

Copy link
Copy Markdown
Contributor

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.TryGoToDefinition is a synchronous bool contract that Roslyn calls on the UI thread, so the main thread has to wait for the checker. It did so with a bare Task.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 its lazy values, which is not cancellable — kept running on the pool. Each further F12 queued another waiter behind the same lazies (7 threads observed in ProjectSnapshot…GetVersion, 2 in SignatureVersion). 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, so TaggerMainThreadManager cancelled its work items and no colours arrived.

Changes.

  • GoToDefinition.fsTryGoToDefinition waits the way NavigateTo in the same file already does: ThreadHelper.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. A user cancel returns false instead of reporting a telemetry fault.
  • CancellableTasks.fs, RoslynHelpers.fs — the two TaskCompletionSource bridges 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.

No issue is filed for this; the diagnosis came from a live debugging session in the experimental instance.

Checklist

  • Test cases added — the change is a UI-thread wait strategy with no unit-testable surface; verified by deploying to the RoslynDev hive and repeating the F12 scenario.
  • Performance benchmarks added in case of performance changes — not applicable, no hot-path change.
  • Release notes entry updated: docs/release-notes/.VisualStudio/18.vNext.md (PR link added in a follow-up commit).

🤖 Generated with Claude Code

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>
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`vsintegration/src` docs/release-notes/.VisualStudio/18.vNext.md

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Sep 7, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant