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

More rustfmt hang investigations #19080

Merged
merged 1 commit into from
Feb 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions crates/rust-analyzer/src/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ impl GlobalState {
|| !self.config.same_source_root_parent_map(&self.local_roots_parent_map)
{
let config_change = {
let _p = span!(Level::INFO, "GlobalState::process_changes/config_change").entered();
let user_config_path = (|| {
let mut p = Config::user_config_dir_path()?;
p.push("rust-analyzer.toml");
Expand Down Expand Up @@ -569,12 +570,12 @@ impl GlobalState {
if let Some((method, start)) = self.req_queue.incoming.complete(&response.id) {
if let Some(err) = &response.error {
if err.message.starts_with("server panicked") {
self.poke_rust_analyzer_developer(format!("{}, check the log", err.message))
self.poke_rust_analyzer_developer(format!("{}, check the log", err.message));
}
}

let duration = start.elapsed();
tracing::debug!("handled {} - ({}) in {:0.2?}", method, response.id, duration);
tracing::debug!(name: "message response", method, %response.id, duration = format_args!("{:0.2?}", duration));
self.send(response.into());
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/rust-analyzer/src/handlers/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ pub(crate) fn handle_did_open_text_document(
tracing::error!("duplicate DidOpenTextDocument: {}", path);
}

state.vfs.write().0.set_file_contents(path, Some(params.text_document.text.into_bytes()));
let contents = params.text_document.text.into_bytes();
state.vfs.write().0.set_file_contents(path, Some(contents));
if state.config.discover_workspace_config().is_some() {
tracing::debug!("queuing task");
let _ = state
Expand Down
21 changes: 12 additions & 9 deletions crates/rust-analyzer/src/handlers/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2318,18 +2318,21 @@ fn run_rustfmt(
}
};

tracing::debug!(?command, "created format command");
let output = {
let _p = tracing::info_span!("rustfmt", ?command).entered();

let mut rustfmt = command
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.context(format!("Failed to spawn {command:?}"))?;
let mut rustfmt = command
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.context(format!("Failed to spawn {command:?}"))?;

rustfmt.stdin.as_mut().unwrap().write_all(file.as_bytes())?;
rustfmt.stdin.as_mut().unwrap().write_all(file.as_bytes())?;

rustfmt.wait_with_output()?
};

let output = rustfmt.wait_with_output()?;
let captured_stdout = String::from_utf8(output.stdout)?;
let captured_stderr = String::from_utf8(output.stderr).unwrap_or_default();

Expand Down
3 changes: 1 addition & 2 deletions crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,8 +701,7 @@ impl GlobalState {

let (crate_graph, proc_macro_paths, ws_data) = {
// Create crate graph from all the workspaces
let vfs = &mut self.vfs.write().0;

let vfs = &self.vfs.read().0;
Copy link
Member Author

Choose a reason for hiding this comment

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

This could've caused some requests (including formatting) to wait on acquiring their read lock when we were recreating the crate graph. This was needed to be a write lock before, but we've since changed the loading behavior here, so this is no longer the case.

let load = |path: &AbsPath| {
let vfs_path = vfs::VfsPath::from(path.to_path_buf());
self.crate_graph_file_dependencies.insert(vfs_path.clone());
Expand Down