Read script carets without blocking the UI thread by xperiandri · Pull Request #20523 · dotnet/fsharp

xperiandri · 2026-09-11T10:18:47Z

Fixes #20522

Visual Studio could hang for good: the project options reactor asked the UI thread for a script's caret (to know whether an #r "nuget: …" line was still being typed) while the UI thread was itself synchronously waiting on the reactor, resolving a breakpoint in the same file. The UI thread now publishes the caret itself, through the text buffer the reactor already reads; the reactor never waits on the UI thread again, and only scripts look for a caret at all.

Moving focus away from a script now submits its pending #r "nuget: …" line, where before only moving the caret to another line did — the caret is unknown once nothing has focus.

github-actions · 2026-09-11T10:18:58Z

✅ Release notes checked

✅ Found changes and release notes in following paths:

Change path Release notes path Description

`vsintegration/src` docs/release-notes/.VisualStudio/18.vNext.md

T-Gro

🤖 🕵️ LGTM

T-Gro

🤖🕵️ Please shorten the description using this guidance. Focus on the problem and why the change is needed, in simplified technical English. Leave the implementation inventory to the Files tab and retain necessary caveats.

T-Gro

🤖🕵️

xperiandri

Addressing the inline review feedback in the updated branch.

The project options reactor looked up the caret itself through ServiceProvider.GlobalProvider, the RDT, IVsTextView and an IVsTextViewEvents connection point, all of which need the UI thread. When the UI thread synchronously waited on project options (breakpoint validation when a document frame is shown), the reactor waited for the UI thread and the UI thread for the reactor. An IWpfTextViewCreationListener now publishes the caret of the focused editor into the text buffer's properties, and the reactor only reads it. Only scripts look for it: FCS uses the caret only to skip the `#r "nuget: ..."` line being typed. Fixes dotnet#20522 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Only the shared FocusedCaret.Position field needed to become `option` for the atomic reference write; the reactor's own local binding was converted along with it for no reason. Route it back through ValueOption and land on `option` once, at the two points that need it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues could leave stale caret or project-option state.

Review effort: Lite
Findings: 1 · 1

Open (2)

Cache misses caret updates when the editor view appears later · New
Add tests for caret updates, focus transitions, and no-editor state · New

What changed in this PR

Updates F# project-option handling to publish editor caret state without blocking the UI thread, preventing Visual Studio hangs.

Changes:

Adds focused-caret tracking through text-buffer state.
Uses caret state for script option resolution.
Removes obsolete COM caret access and documents the fix.

File	Summary
`vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs`	Consumes published caret state; cache subscription and stale source-text issues remain.
`vsintegration/src/FSharp.Editor/LanguageService/FocusedCaret.fs`	Tracks caret and focus changes; direct view closure may leave stale state, and coverage is missing.
`vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj`	Includes the focused-caret tracker.
`vsintegration/src/FSharp.Editor/Common/Extensions.fs`	Removes obsolete COM helpers.
`docs/release-notes/.VisualStudio/18.vNext.md`	Documents the Visual Studio hang fix.

xperiandri · 2026-09-26T11:57:49Z

+                            focusedCaret
+                            |> ValueOption.map _.LineChanged.Subscribe(updateProjectOptions)
+                            |> ValueOption.toOption


The lifecycle is real but it cannot leave a reference pending, because the suppression and the subscription have the same precondition. FCS skips a #r "nuget: …" line only when it is given a caret, and a caret can only come from the same FocusedCaret object whose LineChanged the entry subscribes to. When TryGet answers ValueNone — no view yet — no caret is passed, so that computation resolves every reference rather than holding one back, and there is nothing for a later notification to re-resolve. Once the user does type in the view that appears, the text version changes, the entry is dropped by fileStamp <> oldFileStamp, and the recomputation finds the caret and subscribes.

What the same lifecycle does break is the other direction, and that is fixed in 1826a7e2a6: the caret lives on the buffer, which outlives the view, so a view closed while it held focus left its line published. Options computed after that were given a caret on a line no editor was on, and the #r there stayed unresolved until some other view moved a caret. Closed now clears the caret when the view being closed is the one holding focus — a view that does not hold focus published nothing to clear, and a sibling view's caret is left alone.

xperiandri · 2026-09-26T11:58:00Z

+    member _.Update(newPosition: Position option) =
+        let hasLineChanged =
+            (position |> Option.map _.Line) <> (newPosition |> Option.map _.Line)
+
+        position <- newPosition
+
+        if hasLineChanged then
+            lineChanged.Trigger()


Added in 1826a7e2a6: FocusedCaretTests covers what the reactor reads and when it is told to read again — a move to another line raises the change and publishes the position, a move along the same line publishes the column without raising it, focus leaving raises it and clears the caret, focus already gone raises nothing, and a text no view is open on has no caret at all. Five tests, all passing.

The tracker itself is not tested: it is an IWpfTextViewCreationListener whose whole body is subscriptions to ITextView events, and standing up a text view in these tests would test the editor rather than this code — the same line the repository draws around the other OLE and view plumbing. What it does with what it observes is the Update behaviour above, and the reactor side of it is the subscription in tryComputeOptionsBySingleScriptOrFile, which needs a real FSharpChecker script resolution to observe.

The caret belongs to the buffer, which outlives the view. A view closed while it held focus left its line published, and the reactor went on skipping the `#r "nuget: …"` on a line no editor was on. `FocusedCaretTests` covers what the reactor reads and when it is told to read again: a move to another line, a move along one, focus leaving, focus already gone, and a text no view is open on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-26T13:40:03Z

🔍 Tooling Safety Check — Affects-Design-Time, Affects-Restore
Affects-Design-Time: Changes editor or language-service behavior.
Affects-Restore: Changes package feeds, references, or restore configuration.

Generated by PR Tooling Safety Check · gpt56 1.9M · ◷

xperiandri requested a review from a team as a code owner September 11, 2026 10:18

github-project-automation Bot added this to F# Compiler and Tooling Sep 11, 2026

github-project-automation Bot moved this to New in F# Compiler and Tooling Sep 11, 2026

github-actions Bot added the ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager label Sep 11, 2026

This comment has been minimized.

Sign in to view

github-actions Bot added the ⚠️ Affects-Restore Tooling check: PR touches NuGet packages or feeds label Sep 11, 2026

xperiandri force-pushed the fix/script-caret-deadlock branch from 89eaf2a to 550abf6 Compare September 11, 2026 14:44

T-Gro approved these changes Sep 11, 2026

View reviewed changes

github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Sep 11, 2026

T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 11, 2026

T-Gro self-requested a review September 11, 2026 15:12

T-Gro requested changes Sep 15, 2026

View reviewed changes

xperiandri force-pushed the fix/script-caret-deadlock branch from 550abf6 to 29779b0 Compare September 17, 2026 23:34

xperiandri requested a review from T-Gro September 17, 2026 23:57

T-Gro requested changes Sep 18, 2026

View reviewed changes

Comment thread vsintegration/src/FSharp.Editor/LanguageService/FocusedCaret.fs Outdated

xperiandri changed the title ~~Read a script's caret without waiting for the UI thread~~ Read script carets without blocking the UI thread Sep 18, 2026

xperiandri force-pushed the fix/script-caret-deadlock branch from 86f3ffa to 4c110d6 Compare September 18, 2026 19:55

xperiandri commented Sep 19, 2026

View reviewed changes

xperiandri requested a review from T-Gro September 19, 2026 00:40

xperiandri force-pushed the fix/script-caret-deadlock branch from 4c110d6 to 40c0cb9 Compare September 20, 2026 16:02

xperiandri mentioned this pull request Sep 20, 2026

Note when a shared field needs option instead of voption #20594

Open

T-Gro approved these changes Sep 21, 2026

View reviewed changes

T-Gro enabled auto-merge (squash) September 21, 2026 09:43

T-Gro added the vsintegration-only Changes only Visual Studio integration, plus optional docs, release notes, or tests label Sep 21, 2026

github-actions Bot added the ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure label Sep 21, 2026

This comment has been minimized.

Sign in to view

auto-merge was automatically disabled September 21, 2026 11:22
Head branch was pushed to by a user without write access

xperiandri force-pushed the fix/script-caret-deadlock branch 2 times, most recently from 530c52d to 75fa3cb Compare September 21, 2026 14:08

xperiandri force-pushed the fix/script-caret-deadlock branch from 75fa3cb to 1cef1f4 Compare September 21, 2026 18:00

xperiandri requested a review from T-Gro September 21, 2026 21:03

This comment has been minimized.

Sign in to view

xperiandri force-pushed the fix/script-caret-deadlock branch from 1cef1f4 to 8b54a3c Compare September 23, 2026 18:38

This comment has been minimized.

Sign in to view

github-actions Bot added the ⚠️ Affects-Bootstrap Tooling check: PR touches compiler bootstrap chain label Sep 24, 2026

xperiandri and others added 6 commits September 26, 2026 03:00

Release notes for the script caret hang

5b15163

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Keep the caret in voption until GetProjectOptionsFromScript takes it

b6e602f

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Avoid torn focused-caret publication

57bc4ca

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Format project options manager

6e9472d

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings September 26, 2026 01:00

xperiandri force-pushed the fix/script-caret-deadlock branch from 8b54a3c to bdd422e Compare September 26, 2026 01:00

Copilot started reviewing on behalf of xperiandri September 26, 2026 01:00 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Sep 26, 2026

View reviewed changes

xperiandri mentioned this pull request Sep 26, 2026

Resolve a script's #r lines from its current text #20645

Open

Conversation

xperiandri commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Release notes checked

Uh oh!

This comment has been minimized.

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xperiandri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

xperiandri Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

xperiandri Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 26, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xperiandri commented Sep 11, 2026 •

edited

Loading

github-actions Bot commented Sep 11, 2026 •

edited

Loading