Skip to content

Read script carets without blocking the UI thread - #20523

Open
xperiandri wants to merge 6 commits into
dotnet:mainfrom
xperiandri:fix/script-caret-deadlock
Open

xperiandri wants to merge 6 commits into
dotnet:mainfrom
xperiandri:fix/script-caret-deadlock

Conversation

@xperiandri

@xperiandri xperiandri commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

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

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

✅ 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

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

This comment has been minimized.

@github-actions github-actions Bot added the ⚠️ Affects-Restore Tooling check: PR touches NuGet packages or feeds label Sep 11, 2026
@xperiandri
xperiandri force-pushed the fix/script-caret-deadlock branch from 89eaf2a to 550abf6 Compare September 11, 2026 14:44

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 🕵️ LGTM

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Sep 11, 2026
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 11, 2026
@T-Gro
T-Gro self-requested a review September 11, 2026 15:12

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖🕵️ 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.

@xperiandri
xperiandri force-pushed the fix/script-caret-deadlock branch from 550abf6 to 29779b0 Compare September 17, 2026 23:34
@xperiandri
xperiandri requested a review from T-Gro September 17, 2026 23:57

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖🕵️

Comment thread vsintegration/src/FSharp.Editor/LanguageService/FocusedCaret.fs Outdated
@xperiandri 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
xperiandri force-pushed the fix/script-caret-deadlock branch from 86f3ffa to 4c110d6 Compare September 18, 2026 19:55

@xperiandri xperiandri left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressing the inline review feedback in the updated branch.

Comment thread vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs Outdated
Comment thread vsintegration/src/FSharp.Editor/LanguageService/FocusedCaret.fs
Comment thread vsintegration/src/FSharp.Editor/LanguageService/FocusedCaret.fs
@T-Gro
T-Gro enabled auto-merge (squash) September 21, 2026 09:43
@T-Gro T-Gro added the vsintegration-only Changes only Visual Studio integration, plus optional docs, release notes, or tests label Sep 21, 2026
@github-actions github-actions Bot added the ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure label Sep 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Design-Time, Affects-Restore
Affects-Build-Infra: Non-test project compile inputs changed.
Affects-Design-Time: Project options and editor behavior changed.
Affects-Restore: NuGet reference handling changed.

Generated by PR Tooling Safety Check · gpt56 699.4K ·

auto-merge was automatically disabled September 21, 2026 11:22

Head branch was pushed to by a user without write access

@xperiandri
xperiandri force-pushed the fix/script-caret-deadlock branch 2 times, most recently from 530c52d to 75fa3cb Compare September 21, 2026 14:08
xperiandri and others added 6 commits September 21, 2026 20:00
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: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.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>
@xperiandri
xperiandri force-pushed the fix/script-caret-deadlock branch from 75fa3cb to 1cef1f4 Compare September 21, 2026 18:00
@xperiandri
xperiandri requested a review from T-Gro September 21, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager ⚠️ Affects-Restore Tooling check: PR touches NuGet packages or feeds AI-reviewed PR reviewed by AI review council vsintegration-only Changes only Visual Studio integration, plus optional docs, release notes, or tests

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

VS hangs: project options reactor reads the caret through UI-thread COM while the UI thread waits for the reactor

2 participants