Auto merge of #17526 - Veykril:proc-server-errors, r=Veykril

internal: Improve error message when the proc-macro server unexpectedly exits

Author: bors

crates/proc-macro-api/src/lib.rs

@@ -130,6 +130,10 @@ impl ProcMacroServer {
             Err(message) => Err(ServerError { message, io: None }),
         }
     }
+
+    pub fn exited(&self) -> Option<&ServerError> {
+        self.process.exited()
+    }
 }
 
 impl ProcMacro {

crates/proc-macro-api/src/process.rs

@@ -2,8 +2,9 @@
 
 use std::{
     io::{self, BufRead, BufReader, Read, Write},
+    panic::AssertUnwindSafe,
     process::{Child, ChildStdin, ChildStdout, Command, Stdio},
-    sync::{Arc, Mutex},
+    sync::{Arc, Mutex, OnceLock},
 };
 
 use paths::AbsPath;
@@ -22,15 +23,15 @@ pub(crate) struct ProcMacroProcessSrv {
     state: Mutex<ProcessSrvState>,
     version: u32,
     mode: SpanMode,
+    /// Populated when the server exits.
+    exited: OnceLock<AssertUnwindSafe<ServerError>>,
 }
 
 #[derive(Debug)]
 struct ProcessSrvState {
     process: Process,
     stdin: ChildStdin,
     stdout: BufReader<ChildStdout>,
-    /// Populated when the server exits.
-    server_exited: Option<ServerError>,
 }
 
 impl ProcMacroProcessSrv {
@@ -44,9 +45,10 @@ impl ProcMacroProcessSrv {
             let (stdin, stdout) = process.stdio().expect("couldn't access child stdio");
 
             io::Result::Ok(ProcMacroProcessSrv {
-                state: Mutex::new(ProcessSrvState { process, stdin, stdout, server_exited: None }),
+                state: Mutex::new(ProcessSrvState { process, stdin, stdout }),
                 version: 0,
                 mode: SpanMode::Id,
+                exited: OnceLock::new(),
             })
         };
         let mut srv = create_srv(true)?;
@@ -77,6 +79,10 @@ impl ProcMacroProcessSrv {
         }
     }
 
+    pub(crate) fn exited(&self) -> Option<&ServerError> {
+        self.exited.get().map(|it| &it.0)
+    }
+
     pub(crate) fn version(&self) -> u32 {
         self.version
     }
@@ -118,36 +124,52 @@ impl ProcMacroProcessSrv {
     }
 
     pub(crate) fn send_task(&self, req: Request) -> Result<Response, ServerError> {
-        let state = &mut *self.state.lock().unwrap();
-        if let Some(server_error) = &state.server_exited {
-            return Err(server_error.clone());
+        if let Some(server_error) = self.exited.get() {
+            return Err(server_error.0.clone());
         }
 
+        let state = &mut *self.state.lock().unwrap();
         let mut buf = String::new();
-        send_request(&mut state.stdin, &mut state.stdout, req, &mut buf).map_err(|e| {
-            if e.io.as_ref().map(|it| it.kind()) == Some(io::ErrorKind::BrokenPipe) {
-                match state.process.child.try_wait() {
-                    Ok(None) => e,
-                    Ok(Some(status)) => {
-                        let mut msg = String::new();
-                        if !status.success() {
-                            if let Some(stderr) = state.process.child.stderr.as_mut() {
-                                _ = stderr.read_to_string(&mut msg);
+        send_request(&mut state.stdin, &mut state.stdout, req, &mut buf)
+            .and_then(|res| {
+                res.ok_or_else(|| {
+                    let message = "proc-macro server did not respond with data".to_owned();
+                    ServerError {
+                        io: Some(Arc::new(io::Error::new(
+                            io::ErrorKind::BrokenPipe,
+                            message.clone(),
+                        ))),
+                        message,
+                    }
+                })
+            })
+            .map_err(|e| {
+                if e.io.as_ref().map(|it| it.kind()) == Some(io::ErrorKind::BrokenPipe) {
+                    match state.process.child.try_wait() {
+                        Ok(None) | Err(_) => e,
+                        Ok(Some(status)) => {
+                            let mut msg = String::new();
+                            if !status.success() {
+                                if let Some(stderr) = state.process.child.stderr.as_mut() {
+                                    _ = stderr.read_to_string(&mut msg);
+                                }
                             }
+                            let server_error = ServerError {
+                                message: format!(
+                                    "proc-macro server exited with {status}{}{msg}",
+                                    if msg.is_empty() { "" } else { ": " }
+                                ),
+                                io: None,
+                            };
+                            // `AssertUnwindSafe` is fine here, we already correct initialized
+                            // server_error at this point.
+                            self.exited.get_or_init(|| AssertUnwindSafe(server_error)).0.clone()
                         }
-                        let server_error = ServerError {
-                            message: format!("server exited with {status}: {msg}"),
-                            io: None,
-                        };
-                        state.server_exited = Some(server_error.clone());
-                        server_error
                     }
-                    Err(_) => e,
+                } else {
+                    e
                 }
-            } else {
-                e
-            }
-        })
+            })
     }
 }
 
@@ -201,7 +223,7 @@ fn send_request(
     mut reader: &mut impl BufRead,
     req: Request,
     buf: &mut String,
-) -> Result<Response, ServerError> {
+) -> Result<Option<Response>, ServerError> {
     req.write(write_json, &mut writer).map_err(|err| ServerError {
         message: "failed to write request".into(),
         io: Some(Arc::new(err)),
@@ -210,5 +232,5 @@ fn send_request(
         message: "failed to read response".into(),
         io: Some(Arc::new(err)),
     })?;
-    res.ok_or_else(|| ServerError { message: "server exited".into(), io: None })
+    Ok(res)
 }

crates/proc-macro-srv/src/lib.rs

@@ -204,6 +204,8 @@ fn expand_ra_span(
     let macro_body = macro_body.to_subtree_resolved(CURRENT_API_VERSION, &span_data_table);
     let attributes =
         attributes.map(|it| it.to_subtree_resolved(CURRENT_API_VERSION, &span_data_table));
+    // Note, we spawn a new thread here so that thread locals allocation don't accumulate (this
+    // includes the proc-macro symbol interner)
     let result = thread::scope(|s| {
         let thread = thread::Builder::new()
             .stack_size(EXPANDER_STACK_SIZE)

crates/rust-analyzer/src/reload.rs

@@ -184,14 +184,28 @@ impl GlobalState {
                     message.push_str(err);
                     message.push_str("\n\n");
                 };
-                if let Some(Err(err)) = proc_macro_client {
-                    status.health |= lsp_ext::Health::Warning;
-                    format_to!(
-                        message,
-                        "Failed spawning proc-macro server for workspace `{}`: {err}",
-                        ws.manifest_or_root()
-                    );
-                    message.push_str("\n\n");
+                match proc_macro_client {
+                    Some(Err(err)) => {
+                        status.health |= lsp_ext::Health::Warning;
+                        format_to!(
+                            message,
+                            "Failed spawning proc-macro server for workspace `{}`: {err}",
+                            ws.manifest_or_root()
+                        );
+                        message.push_str("\n\n");
+                    }
+                    Some(Ok(client)) => {
+                        if let Some(err) = client.exited() {
+                            status.health |= lsp_ext::Health::Warning;
+                            format_to!(
+                                message,
+                                "proc-macro server for workspace `{}` exited: {err}",
+                                ws.manifest_or_root()
+                            );
+                            message.push_str("\n\n");
+                        }
+                    }
+                    _ => (),
                 }
             }
         }