Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix data race in problem view tree #13841

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Conversation

msujew
Copy link
Member

@msujew msujew commented Jun 24, 2024

What it does

Closes #13750
Closes #13835

Whenever we encounter a new diagnostics collection for a given file (i.e. the diagnostics have been updated/replaced), we now simply replace the still waiting collection in the map. This ensures that the problem view tree stays consistent and outdated diagnostics get correctly replaced.

How to test

Run the test steps from #13835 and assert that everything works as expected.

Review checklist

Reminder for reviewers

@tsmaeder
Copy link
Contributor

@msujew When I open the directory with the rust test code ("Open Folder"), I get an error in the browser log (Electron)

logger-protocol.ts:117 2024-06-25T09:11:07.994Z root ERROR TypeError: Cannot set property root of #<ProblemTree> which has only a getter
    at new MarkerTree (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/packages_markers_lib_browser_problem_problem-widget_js.js:104:19)
    at new ProblemTree (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/packages_markers_lib_browser_problem_problem-widget_js.js:442:9)
    at createInstanceWithInjections (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256717:20)
    at _createInstance (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256707:22)
    at resolveInstance (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256802:18)
    at _getResolvedFromBinding (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256920:85)
    at file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256938:22
    at _resolveInScope (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256932:14)
    at _resolveBinding (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256937:12)
    at file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256899:20

@tsmaeder
Copy link
Contributor

Playwright tests have the same problem.

@msujew
Copy link
Member Author

msujew commented Jun 25, 2024

@tsmaeder Yep, sorry. Wanted to be too smart about type checking, which led to surprising runtime errors. Weird that TypeScript didn't catch that actually.

@tsmaeder
Copy link
Contributor

@msujew I get an error dialog when starting up. Is this relevent?

Unfortunately we don't ship binaries for your platform yet. You need to manually clone the rust-analyzer repository and run cargo xtask install --server to build the language server from sources. If you feel that your platform should be supported, please create an issue about that [here](https://github.com/rust-lang/rust-analyzer/issues) and we will consider it.

@msujew
Copy link
Member Author

msujew commented Jun 25, 2024

@tsmaeder This should be fixed with #13825, which isn't included in this PR yet. Let me quickly rebase this. Alternatively you can just manually download the correct version for your OS from open-vsx.

@msujew msujew force-pushed the msujew/fix-problem-view-data-race branch from d8f8655 to be55d2a Compare June 25, 2024 13:15
@tsmaeder
Copy link
Contributor

@msujew the rust language server still complains about some toolchain not being setup correctly:

2024-06-25T13:31:29.769174Z ERROR rust_analyzer::main_loop: FetchWorkspaceError:
rust-analyzer failed to load workspace: Failed to load the project at C:\Users\thomas\Downloads\test1\test1\Cargo.toml: Failed to query rust toolchain version at C:\Users\thomas\Downloads\test1\test1, is your toolchain setup correctly?: "cargo" "--version" failed: program not found

Is there some additional setup required?

@msujew
Copy link
Member Author

msujew commented Jun 25, 2024

@tsmaeder The rust analyzer requires the cargo CLI (the rust build tools) to work correctly. See https://doc.rust-lang.org/cargo/getting-started/installation.html with instructions on how to install on Windows.

@msujew msujew merged commit 377de26 into master Jun 25, 2024
14 checks passed
@msujew msujew deleted the msujew/fix-problem-view-data-race branch June 25, 2024 15:08
@github-actions github-actions bot added this to the 1.51.0 milestone Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
problems issues related to the problems widget
Projects
Archived in project
2 participants