From 0df88669c84d20246187ff86f2e5b3008a5d60ab Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:04:18 +0200 Subject: [PATCH 1/3] 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 11ad73627b3a8b80a5573a49563a72a29083d40d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:05:38 +0200 Subject: [PATCH 2/3] 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 022d14fa7357deef2d8eff16ee700fa167c8d37b Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 7 Sep 2026 20:46:56 +0200 Subject: [PATCH 3/3] 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 )