From be45e7a364c6f82d2629180717f3a1f921151e4b Mon Sep 17 00:00:00 2001 From: KushalMeghani1644 Date: Sun, 17 Aug 2025 19:06:43 +0530 Subject: [PATCH 1/2] Improve stdio_transport: error handling & channel buffering --- lib/lsp-server/src/stdio.rs | 54 ++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/lib/lsp-server/src/stdio.rs b/lib/lsp-server/src/stdio.rs index eccc89fd59c3..d651228ea5b4 100644 --- a/lib/lsp-server/src/stdio.rs +++ b/lib/lsp-server/src/stdio.rs @@ -11,37 +11,39 @@ use crate::Message; /// Creates an LSP connection via stdio. pub(crate) fn stdio_transport() -> (Sender, Receiver, IoThreads) { - let (drop_sender, drop_receiver) = bounded::(0); - let (writer_sender, writer_receiver) = bounded::(0); + let (drop_sender, drop_receiver) = bounded::(1); // Changed from 0 -> 1 for minimal buffering + let (writer_sender, writer_receiver) = bounded::(1); // Changed from 0 -> 1 + let writer = thread::Builder::new() .name("LspServerWriter".to_owned()) - .spawn(move || { + .spawn(move || -> io::Result<()> { let stdout = stdout(); let mut stdout = stdout.lock(); - writer_receiver.into_iter().try_for_each(|it| { + for it in writer_receiver { let result = it.write(&mut stdout); let _ = drop_sender.send(it); - result - }) + result?; // Propagate error instead of unwrap + } + Ok(()) }) - .unwrap(); + .expect("Failed to spawn writer thread"); // Replaced unwrap with expect for better context + let dropper = thread::Builder::new() .name("LspMessageDropper".to_owned()) .spawn(move || drop_receiver.into_iter().for_each(drop)) - .unwrap(); - let (reader_sender, reader_receiver) = bounded::(0); + .expect("Failed to spawn dropper thread"); + + let (reader_sender, reader_receiver) = bounded::(1); + let reader = thread::Builder::new() .name("LspServerReader".to_owned()) - .spawn(move || { + .spawn(move || -> io::Result<()> { let stdin = stdin(); let mut stdin = stdin.lock(); while let Some(msg) = Message::read(&mut stdin)? { let is_exit = matches!(&msg, Message::Notification(n) if n.is_exit()); - debug!("sending message {msg:#?}"); - if let Err(e) = reader_sender.send(msg) { - return Err(io::Error::other(e)); - } + reader_sender.send(msg).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; // Better error propagation if is_exit { break; @@ -49,7 +51,8 @@ pub(crate) fn stdio_transport() -> (Sender, Receiver, IoThread } Ok(()) }) - .unwrap(); + .expect("Failed to spawn reader thread"); + let threads = IoThreads { reader, writer, dropper }; (writer_sender, reader_receiver, threads) } @@ -71,21 +74,10 @@ pub struct IoThreads { impl IoThreads { pub fn join(self) -> io::Result<()> { - match self.reader.join() { - Ok(r) => r?, - Err(err) => std::panic::panic_any(err), - } - match self.dropper.join() { - Ok(_) => (), - Err(err) => { - std::panic::panic_any(err); - } - } - match self.writer.join() { - Ok(r) => r, - Err(err) => { - std::panic::panic_any(err); - } - } + self.reader + .join() + .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))??; + self.dropper.join().map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))?; + self.writer.join().map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))? } } From cb3d8b131b26625a86b8418692b5e2a38cc4112c Mon Sep 17 00:00:00 2001 From: KushalMeghani1644 Date: Mon, 18 Aug 2025 11:05:15 +0530 Subject: [PATCH 2/2] fix: correct syntax errors and error handling in LSP transport --- lib/lsp-server/src/stdio.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/lsp-server/src/stdio.rs b/lib/lsp-server/src/stdio.rs index d651228ea5b4..714c3509d1da 100644 --- a/lib/lsp-server/src/stdio.rs +++ b/lib/lsp-server/src/stdio.rs @@ -1,14 +1,11 @@ +use crate::Message; +use crossbeam_channel::{Receiver, Sender, bounded}; +use log::debug; use std::{ io::{self, stdin, stdout}, thread, }; -use log::debug; - -use crossbeam_channel::{Receiver, Sender, bounded}; - -use crate::Message; - /// Creates an LSP connection via stdio. pub(crate) fn stdio_transport() -> (Sender, Receiver, IoThreads) { let (drop_sender, drop_receiver) = bounded::(1); // Changed from 0 -> 1 for minimal buffering @@ -21,7 +18,7 @@ pub(crate) fn stdio_transport() -> (Sender, Receiver, IoThread let mut stdout = stdout.lock(); for it in writer_receiver { let result = it.write(&mut stdout); - let _ = drop_sender.send(it); + let _ = drop_sender.send(it); // Fixed: was `let * = drop*sender.send(it);` result?; // Propagate error instead of unwrap } Ok(()) @@ -34,7 +31,6 @@ pub(crate) fn stdio_transport() -> (Sender, Receiver, IoThread .expect("Failed to spawn dropper thread"); let (reader_sender, reader_receiver) = bounded::(1); - let reader = thread::Builder::new() .name("LspServerReader".to_owned()) .spawn(move || -> io::Result<()> { @@ -44,7 +40,6 @@ pub(crate) fn stdio_transport() -> (Sender, Receiver, IoThread let is_exit = matches!(&msg, Message::Notification(n) if n.is_exit()); debug!("sending message {msg:#?}"); reader_sender.send(msg).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; // Better error propagation - if is_exit { break; } @@ -77,7 +72,7 @@ impl IoThreads { self.reader .join() .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))??; - self.dropper.join().map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))?; - self.writer.join().map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))? + self.dropper.join().map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))?; // Fixed: was std::io::Error::Other + self.writer.join().map_err(|e| io::Error::new(io::ErrorKind::Other, format!("{e:?}")))? // Fixed: was std::io::Error::Other and missing ? } }