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

thread 'LspServer' panicked at 'called Option::unwrap() on a None value', crates/vfs/src/lib.rs:143:38 #13236

Closed
bjorn3 opened this issue Sep 15, 2022 · 6 comments · Fixed by #13237
Labels
C-bug Category: bug

Comments

@bjorn3
Copy link
Member

bjorn3 commented Sep 15, 2022

I got the following error several times in a row while working on a project which has a rust-lang/rust checkout in a subdirectory. The final message that vscode won't restart rust-analyzer happened while I had a file in src/test/ui open.

thread 'LspServer' panicked at 'called `Option::unwrap()` on a `None` value', crates/vfs/src/lib.rs:143:38
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:48:5
   3: rust_analyzer::global_state::GlobalState::process_changes
   4: rust_analyzer::main_loop::<impl rust_analyzer::global_state::GlobalState>::handle_event
   5: rust_analyzer::main_loop::<impl rust_analyzer::global_state::GlobalState>::run
   6: rust_analyzer::main_loop::main_loop
   7: rust_analyzer::run_server
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
[Info  - 1:26:23 PM] Connection to server got closed. Server will restart.

rust-analyzer version: rust-analyzer version: 0.3.1203-standalone

rustc version: rustc 1.65.0-nightly (750bd1a7f 2022-09-14)

relevant settings: None that I know of.

@bjorn3 bjorn3 added the C-bug Category: bug label Sep 15, 2022
@Veykril
Copy link
Member

Veykril commented Sep 15, 2022

The panic comes from fetching file contents of a file that is not in the vfs ... I assume you git checkout at least once before this started?

@bjorn3
Copy link
Member Author

bjorn3 commented Sep 15, 2022

Yes, although even after I restarted rust-analyzer it has been giving similar issues several times. Maybe the fact that the rust checkout is in the .gitignore of the parent repo is related?

@Veykril
Copy link
Member

Veykril commented Sep 15, 2022

I doubt that, so this panic is odd, I wonder if there is a race condition here because we apparently created a ChangeFile that is not a deletion but where the file id was already deleted. The panic happens here https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/global_state.rs#L210

@Veykril
Copy link
Member

Veykril commented Sep 15, 2022

Okay I can see one way how this can crash once (not repeatedly unless the same scenario is replicated). If in one process_changes call, the VFS received a modify and a delete for a given file, then when we process the modify we will crash on the linked line, because the changes happen immediately to the VFS and so process_changes observes the state only after the VFS has changed completely.

@bjorn3
Copy link
Member Author

bjorn3 commented Sep 15, 2022

To give a bit more info, the scripts/test_rustc_tests.sh script checks out a specific commit, reverts any changes to the working tree, applies some patches and deletes certain files and directories and after that runs the rustc test suite. Maybe it is a race condition between handling the reverting of all changes and subsequently applying patches and removing files? Or between separate inotify instances for different directories? By the way this is on Linux in case that is relevant.

@Veykril
Copy link
Member

Veykril commented Sep 15, 2022

Ye that pretty much describes a series of action that can cause the problem described in my previous comment, but I am still unsure how that would get the server stuck in an inconsistent state, unless panicking in that code can poison something there 🤔

Veykril added a commit to Veykril/rust-analyzer that referenced this issue Sep 15, 2022
When receiving multiple change events for a single file id where the
last change is a delete the server panics, as it tries to access the
file contents of a deleted file. This occurs due to the VFS changes and
the in memory file contents being updated immediately, while
`process_changes` processes the events afterwards in sequence which no
longer works as it will only observe the final file contents. By
folding these events together, we will no longer try to process these
intermediate changes, as they aren't relevant anyways.

Potentially fixes rust-lang#13236
Veykril added a commit to Veykril/rust-analyzer that referenced this issue Sep 27, 2022
When receiving multiple change events for a single file id where the
last change is a delete the server panics, as it tries to access the
file contents of a deleted file. This occurs due to the VFS changes and
the in memory file contents being updated immediately, while
`process_changes` processes the events afterwards in sequence which no
longer works as it will only observe the final file contents. By
folding these events together, we will no longer try to process these
intermediate changes, as they aren't relevant anyways.

Potentially fixes rust-lang#13236
bors added a commit that referenced this issue Sep 27, 2022
Amalgamate file changes for the same file ids in process_changes

When receiving multiple change events for a single file id where the last change is a delete the server panics, as it tries to access the file contents of a deleted file. This occurs due to the VFS changes and the in memory file contents being updated immediately, while `process_changes` processes the events afterwards in sequence which no longer works as it will only observe the final file contents. By folding these events together, we will no longer try to process these intermediate changes, as they aren't relevant anyways.

Potentially fixes #13236
@bors bors closed this as completed in c3a6c96 Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants