Skip to content

Keep Peek Definition off the main thread - #20503

Open
xperiandri wants to merge 5 commits into
dotnet:mainfrom
xperiandri:fix/peek-metadata-deadlock
Open

Keep Peek Definition off the main thread#20503
xperiandri wants to merge 5 commits into
dotnet:mainfrom
xperiandri:fix/peek-metadata-deadlock

Conversation

@xperiandri

Copy link
Copy Markdown
Contributor

Peek Definition on an F# symbol whose definition lives in metadata deadlocks Visual Studio — the whole
IDE, not just the editor. The easiest way to see it is to Peek twice: the first Peek from real source
usually resolves in source and is fine, and the second one, invoked from inside the metadata window
the first produced, hangs, because from there every symbol is external.

It presents as whatever the user happened to be doing at the time. It was reported to me as "rename
hangs"; the stack said Peek.

What the stack shows

Breaking into a hung instance, the UI thread reads, innermost last:

WindowFrame.Exec
└ PeekDefinitionCommandHandler.TriggerPeekDefinitionAtCaret
  └ PeekBroker.TriggerPeekSessionImpl → PeekSession.QueryPeekableItemSources
    └ PeekableItemSource.AugmentPeekSession
      └ VSUIThreadOperationExecutor.Execute
        └ JoinableTask.CompleteOnCurrentThread → WaitSynchronously → Task.Wait
          └ NoMessagePumpSyncContext.Wait

AugmentPeekSession runs on the main thread and holds it in JoinableTaskFactory.Run for the whole
augmentation, and that wait does not pump messages — which is why the whole IDE freezes. Meanwhile a
background thread sits in ThreadHelper.InvokeOnUIThreadServiceProvider.QueryService, asking for
the thread that is waiting on it, and no thread is in FSharp.Compiler.Service.dll at all: the work is
not slow, it is stopped. Resuming does not break the cycle.

F# is reached from there through FSharpNavigableItemsService, the INavigableItemsService for F#
(F# documents have no semantic model, so Peek takes that path), which calls
IFSharpFindDefinitionService. Producing a definition that only exists as generated metadata then
asks for the main thread twice: TryGetExternalDeclarationAsync switches to it to open the document,
and creating the workspace project context underneath is itself a JoinableTaskFactory.Run.

The change

FSharpNavigation.FindDefinitionsAsync was serving both entry points — Go To Definition through
IFSharpGoToDefinitionService and Peek through IFSharpFindDefinitionService — so this splits it.
Go To Definition keeps the metadata case, since it owns the wait it makes. Peek gets
FindDefinitionsWithoutMetadataAsync, which stops at definitions that already have a document and
therefore needs the main thread nowhere.

Peek into metadata shows nothing until that is restored properly. That is a smaller regression than
freezing the IDE, and Go To Definition still opens the generated signature.

Why not just make it work

Roslyn's own metadata Peek asks for the main thread nowhere: DefinitionPeekableItem generates the
file in IPeekResultSource.FindResults, which the broker calls on a background thread ("we must block
the thread since the API doesn't support proper asynchrony"), and hands Peek a file path through
PeekHelpers.CreateDocumentPeekResult rather than opening a document.

Matching that from F# needs the project context to be creatable off the main thread, which is
dotnet/roslyn#85219, and a way to reach the generated document without OpenDocumentViaProject.
Neither can land here first.

Base

Stacked on #20482, which is the other main-thread fix in this area; the diff will shrink to its own
commit once that merges.

🤖 Generated with Claude Code

xperiandri and others added 4 commits September 7, 2026 20:04
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>
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>
Peek reaches the F# language service through `INavigableItemsService` while its
broker holds the main thread in `JoinableTaskFactory.Run`, and that wait does
not pump messages. Producing a definition that only exists as generated
metadata needs the main thread — to create the workspace project context and to
open the document — so asking for it from there deadlocks Visual Studio, not
just the editor. It presents as whatever the user happened to be doing, and is
reliably reproduced by opening Peek from inside the metadata window a first
Peek produced, where every symbol is external.

Split the search rather than dropping the metadata case: Go To Definition keeps
it, since it owns the wait it makes, and Peek gets the variant that stops at
definitions which already have a document.

Peek into metadata therefore shows nothing for now. Roslyn's own Peek avoids
both waits by generating the file in `IPeekResultSource.FindResults`, which the
broker calls on a background thread, and handing it a path instead of opening a
document. Matching that needs the project context to be creatable off the main
thread, which is dotnet/roslyn#85219.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 10, 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 Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager label Sep 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: Visual Studio navigation code runs in the IDE.

Generated by PR Tooling Safety Check · gpt56 2.1M ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant