Skip to content

fix: Incorrect flychecks with multiple workspaces#21709

Open
Wilfred wants to merge 1 commit intorust-lang:masterfrom
Wilfred:race_condition_flycheck_workspaces
Open

fix: Incorrect flychecks with multiple workspaces#21709
Wilfred wants to merge 1 commit intorust-lang:masterfrom
Wilfred:race_condition_flycheck_workspaces

Conversation

@Wilfred
Copy link
Contributor

@Wilfred Wilfred commented Feb 25, 2026

When rust-analyzer receives textDocument/didChangeWatchedFiles, we want to trigger a flycheck in all workspaces if we can't find which workspace owns those files.

However, since we used .any() instead of .all(), this behaviour was always triggered when the user had more than one workspace.

For cargo-based projects, this just makes rust-analyzer slower. For projects with a custom check command using $saved_file or {saved_file}, this introduced a race condition that sometimes prevented diagnostics.

When we see the following flycheck events in this order:

// Created by textDocument/didSave.
RequestStateChange(Restart { ... saved_file: Some("foo.rs") })
// Created by textDocument/didChangeWatchedFiles
RequestStateChange(Restart { ... saved_file: None })

Then the flycheck debounce takes the last event, we invoke flycheck with saved_file: None, and no flycheck occurs (because we require a value to substitute in $saved_file).

Previously the debounce took the first event (until #21666), but that just meant a race condition when events arrive in the opposite order.

Instead, use .all() and add an integration test to exercise this behaviour.

AI disclosure: Once I understood the problem, I used Opus 4.6 to do the first draft of this integration test.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 25, 2026
@rustbot

This comment has been minimized.

@Wilfred Wilfred force-pushed the race_condition_flycheck_workspaces branch from 9638c64 to 0960ec2 Compare February 25, 2026 20:51
@rustbot

This comment has been minimized.

@Wilfred
Copy link
Contributor Author

Wilfred commented Feb 25, 2026

Interesting, a macOS test is hanging due to no diagnostics. That sounds like #21571.

When rust-analyzer receives textDocument/didChangeWatchedFiles, we
want to trigger a flycheck in all workspaces if we can't find which
workspace owns those files.

However, since we used .any() instead of .all(), this behaviour was
always triggered when the user had more than one workspace.

For cargo-based projects, this just makes rust-analyzer slower. For
projects with a custom check command using $saved_file or
{saved_file}, this introduced a race condition that sometimes
prevented diagnostics.

When we see the following flycheck events in this order:

    // Created by textDocument/didSave.
    RequestStateChange(Restart { ... saved_file: Some("foo.rs") })
    // Created by textDocument/didChangeWatchedFiles
    RequestStateChange(Restart { ... saved_file: None })

Then the flycheck debounce takes the last event, we invoke flycheck
with saved_file: None, and no flycheck occurs (because we require a
value to substitute in $saved_file).

Previously the debounce took the first event (until rust-lang#21666), but that
just meant a race condition when events arrive in the opposite order.

Instead, use .all() and add an integration test to exercise this
behaviour.

AI disclosure: Once I understood the problem, I used Opus 4.6 to do
the first draft of this integration test.
@Wilfred Wilfred force-pushed the race_condition_flycheck_workspaces branch from 0960ec2 to 08e9bcd Compare March 2, 2026 15:18
@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2026

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2026

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@Wilfred
Copy link
Contributor Author

Wilfred commented Mar 2, 2026

I can't reproduce the macOS failure on my mac laptop, I presume this is just a performance with GitHub's macOS CI. I've tweaked the timeouts, but as mentioned above I think a fix similar to #21571 is going to help, so I'll look at that too.

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

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants