From 42ce4792c71aae54d7a995c4ece735b45a5625e9 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:04:18 +0200 Subject: [PATCH 1/5] Wait for Go To Definition through the threaded-wait dialog 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 --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../FSharp.Editor/Common/CancellableTasks.fs | 4 +- .../src/FSharp.Editor/Common/RoslynHelpers.fs | 4 +- .../Navigation/GoToDefinition.fs | 51 ++++++++++++------- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index ba03f663967..dda5ee42a45 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -5,6 +5,7 @@ ### Fixed +* Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) diff --git a/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs b/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs index 7520395a084..6ca71cf9010 100644 --- a/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs +++ b/vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs @@ -777,7 +777,9 @@ module CancellableTasks = } // try not to yield if on bg thread already - let tcs = new TaskCompletionSource<_>(TaskCreationOptions.None) + let tcs = + new TaskCompletionSource<_>(TaskCreationOptions.RunContinuationsAsynchronously) + let barrier = VolatileBarrier() let reg = diff --git a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs index 2679740fbb3..d7439b18b57 100644 --- a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs +++ b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs @@ -135,7 +135,9 @@ module internal RoslynHelpers = return! computation } - let tcs = new TaskCompletionSource<_>(TaskCreationOptions.None) + let tcs = + new TaskCompletionSource<_>(TaskCreationOptions.RunContinuationsAsynchronously) + let barrier = VolatileBarrier() let reg = diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 6bc86ae57a3..3d13dab50fb 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -794,32 +794,45 @@ type internal FSharpNavigation(metadataAsSource: FSharpMetadataAsSourceService, } member _.TryGoToDefinition(position, cancellationToken) = - // Once we migrate to Roslyn-exposed MAAS and sourcelink (https://github.com/dotnet/fsharp/issues/13951), this can be a "normal" task - // Wrap this in a try/with as if the user clicks "Cancel" on the thread dialog, we'll be cancelled. - // Task.Wait throws an exception if the task is cancelled, so be sure to catch it. + // Once we migrate to Roslyn-exposed MAAS and sourcelink (https://github.com/dotnet/fsharp/issues/13951), this can be a "normal" task. + // The IFSharpGoToDefinitionService contract is synchronous, so the main thread has to wait here: the threaded-wait dialog + // keeps it pumping and cancellable, where a bare Task.Wait froze it until the VS watchdog auto-cancelled. try use _ = TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.GoToDefinition, [||]) let gtd = GoToDefinition(metadataAsSource) - let gtdTask = gtd.FindDefinitionAsync (initialDoc, position) cancellationToken + let navigated = ref false - gtdTask.Wait() - - if gtdTask.Status = TaskStatus.RanToCompletion && gtdTask.Result.IsSome then - match gtdTask.Result with - | ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), _) -> - gtd.NavigateToItem(navItem, cancellationToken) |> ignore - true - | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), _) -> - gtd.NavigateToExternalDeclaration(targetSymbolUse, metadataReferences, cancellationToken) - |> ignore + ThreadHelper.JoinableTaskFactory.Run( + SR.NavigatingTo(), + (fun _progress dialogCancellationToken -> + task { + use linked = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, dialogCancellationToken) + + return! + cancellableTask { + match! gtd.FindDefinitionAsync(initialDoc, position) with + | ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), _) -> + gtd.NavigateToItem(navItem, linked.Token) |> ignore + navigated.Value <- true + | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), _) -> + gtd.NavigateToExternalDeclaration(targetSymbolUse, metadataReferences, linked.Token) + |> ignore + + navigated.Value <- true + | _ -> () + } + |> CancellableTask.start linked.Token + }), + TimeSpan.FromSeconds 1 + ) - true - | _ -> false - else - false - with exc -> + navigated.Value + with + | :? OperationCanceledException -> false + | exc -> TelemetryReporter.ReportFault(TelemetryEvents.GoToDefinition, FaultSeverity.General, exc) false From 15b25a91176bd9e00859e9dc8342b726db96de97 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:05:38 +0200 Subject: [PATCH 2/5] Add the release note link for PR #20482 Co-Authored-By: Claude Fable 5.1 --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index dda5ee42a45..c9c58bb8a66 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -5,7 +5,7 @@ ### Fixed -* Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. +* Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) From b46ceaab750350b25625a135c91d44d2b5089dbc Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:46:56 +0200 Subject: [PATCH 3/5] Do not nest cancellableTask inside task in TryGoToDefinition 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 --- .../Navigation/GoToDefinition.fs | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 3d13dab50fb..813d9bc271a 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -807,25 +807,24 @@ type internal FSharpNavigation(metadataAsSource: FSharpMetadataAsSourceService, ThreadHelper.JoinableTaskFactory.Run( SR.NavigatingTo(), (fun _progress dialogCancellationToken -> - task { - use linked = - CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, dialogCancellationToken) - - return! - cancellableTask { - match! gtd.FindDefinitionAsync(initialDoc, position) with - | ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), _) -> - gtd.NavigateToItem(navItem, linked.Token) |> ignore - navigated.Value <- true - | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), _) -> - gtd.NavigateToExternalDeclaration(targetSymbolUse, metadataReferences, linked.Token) - |> ignore - - navigated.Value <- true - | _ -> () - } - |> CancellableTask.start linked.Token - }), + let linked = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, dialogCancellationToken) + + cancellableTask { + use _ = linked + + match! gtd.FindDefinitionAsync(initialDoc, position) with + | ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), _) -> + gtd.NavigateToItem(navItem, linked.Token) |> ignore + navigated.Value <- true + | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), _) -> + gtd.NavigateToExternalDeclaration(targetSymbolUse, metadataReferences, linked.Token) + |> ignore + + navigated.Value <- true + | _ -> () + } + |> CancellableTask.start linked.Token), TimeSpan.FromSeconds 1 ) From 51115b2cd3cd25e311b9acb197a6a901fb025276 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 02:20:34 +0200 Subject: [PATCH 4/5] Keep Peek Definition off the main thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../Navigation/FindDefinitionService.fs | 2 +- .../src/FSharp.Editor/Navigation/GoToDefinition.fs | 13 +++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index c9c58bb8a66..3ab58555a71 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -6,6 +6,7 @@ ### Fixed * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) +* Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20494](https://github.com/dotnet/fsharp/pull/20494)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) diff --git a/vsintegration/src/FSharp.Editor/Navigation/FindDefinitionService.fs b/vsintegration/src/FSharp.Editor/Navigation/FindDefinitionService.fs index bc408c01741..8cfc40887d6 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/FindDefinitionService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/FindDefinitionService.fs @@ -19,6 +19,6 @@ type internal FSharpFindDefinitionService [] (metadataAsSo member _.FindDefinitionsAsync(document: Document, position: int, cancellationToken: CancellationToken) = cancellableTask { let navigation = FSharpNavigation(metadataAsSource, document, rangeStartup) - return! navigation.FindDefinitionsAsync(position) + return! navigation.FindDefinitionsWithoutMetadataAsync(position) } |> CancellableTask.start cancellationToken diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 813d9bc271a..05da71ef97e 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -793,6 +793,19 @@ type internal FSharpNavigation(metadataAsSource: FSharpMetadataAsSourceService, | _ -> return ImmutableArray.empty } + /// The same search, minus the definitions that only exist once a metadata document has been generated: + /// generating one takes the main thread, and Peek's broker holds it in `JoinableTaskFactory.Run` without + /// pumping messages until this returns, so asking for it there deadlocks Visual Studio. + member _.FindDefinitionsWithoutMetadataAsync(position) = + cancellableTask { + let gtd = GoToDefinition(metadataAsSource) + let! result = gtd.FindDefinitionAtPosition(initialDoc, position) + + match result with + | ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), _) -> return ImmutableArray.create navItem + | _ -> return ImmutableArray.empty + } + member _.TryGoToDefinition(position, cancellationToken) = // Once we migrate to Roslyn-exposed MAAS and sourcelink (https://github.com/dotnet/fsharp/issues/13951), this can be a "normal" task. // The IFSharpGoToDefinitionService contract is synchronous, so the main thread has to wait here: the threaded-wait dialog From 0cb53b89d148ed825c37e75c4724773675decc4f Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 02:35:33 +0200 Subject: [PATCH 5/5] Point the release note at the actual PR number Co-Authored-By: Claude Opus 5 --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 3ab58555a71..f1741ae71d0 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -6,7 +6,7 @@ ### Fixed * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) -* Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20494](https://github.com/dotnet/fsharp/pull/20494)) +* Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))