aboutsummaryrefslogtreecommitdiff
path: root/crates
diff options
context:
space:
mode:
authorAleksey Kladov <[email protected]>2020-06-28 22:42:44 +0100
committerAleksey Kladov <[email protected]>2020-06-28 22:42:44 +0100
commit5cdd8d442ef5a573f4af07e68dce7720ca603aba (patch)
tree3e5a759068adf8607cc142144d3ec7d665d7e5f6 /crates
parent32e85a1a89877dc1314ea950bd4cba43d9ad9627 (diff)
Cleanup cargo process handling in flycheck
Diffstat (limited to 'crates')
-rw-r--r--crates/flycheck/src/lib.rs121
-rw-r--r--crates/rust-analyzer/src/main_loop.rs6
2 files changed, 66 insertions, 61 deletions
diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs
index ab1d71b98..1023d3040 100644
--- a/crates/flycheck/src/lib.rs
+++ b/crates/flycheck/src/lib.rs
@@ -85,7 +85,7 @@ pub enum Message {
85pub enum Progress { 85pub enum Progress {
86 DidStart, 86 DidStart,
87 DidCheckCrate(String), 87 DidCheckCrate(String),
88 DidFinish, 88 DidFinish(io::Result<()>),
89 DidCancel, 89 DidCancel,
90} 90}
91 91
@@ -100,7 +100,7 @@ struct FlycheckActor {
100 /// doesn't provide a way to read sub-process output without blocking, so we 100 /// doesn't provide a way to read sub-process output without blocking, so we
101 /// have to wrap sub-processes output handling in a thread and pass messages 101 /// have to wrap sub-processes output handling in a thread and pass messages
102 /// back over a channel. 102 /// back over a channel.
103 check_process: Option<CargoHandle>, 103 cargo_handle: Option<CargoHandle>,
104} 104}
105 105
106enum Event { 106enum Event {
@@ -114,10 +114,10 @@ impl FlycheckActor {
114 config: FlycheckConfig, 114 config: FlycheckConfig,
115 workspace_root: PathBuf, 115 workspace_root: PathBuf,
116 ) -> FlycheckActor { 116 ) -> FlycheckActor {
117 FlycheckActor { sender, config, workspace_root, check_process: None } 117 FlycheckActor { sender, config, workspace_root, cargo_handle: None }
118 } 118 }
119 fn next_event(&self, inbox: &Receiver<Restart>) -> Option<Event> { 119 fn next_event(&self, inbox: &Receiver<Restart>) -> Option<Event> {
120 let check_chan = self.check_process.as_ref().map(|cargo| &cargo.receiver); 120 let check_chan = self.cargo_handle.as_ref().map(|cargo| &cargo.receiver);
121 select! { 121 select! {
122 recv(inbox) -> msg => msg.ok().map(Event::Restart), 122 recv(inbox) -> msg => msg.ok().map(Event::Restart),
123 recv(check_chan.unwrap_or(&never())) -> msg => Some(Event::CheckEvent(msg.ok())), 123 recv(check_chan.unwrap_or(&never())) -> msg => Some(Event::CheckEvent(msg.ok())),
@@ -128,15 +128,22 @@ impl FlycheckActor {
128 match event { 128 match event {
129 Event::Restart(Restart) => { 129 Event::Restart(Restart) => {
130 while let Ok(Restart) = inbox.recv_timeout(Duration::from_millis(50)) {} 130 while let Ok(Restart) = inbox.recv_timeout(Duration::from_millis(50)) {}
131
131 self.cancel_check_process(); 132 self.cancel_check_process();
132 self.check_process = Some(self.start_check_process()); 133
133 self.send(Message::Progress(Progress::DidStart)); 134 let mut command = self.check_command();
135 command.stdout(Stdio::piped()).stderr(Stdio::null()).stdin(Stdio::null());
136 if let Ok(child) = command.spawn().map(JodChild) {
137 self.cargo_handle = Some(CargoHandle::spawn(child));
138 self.send(Message::Progress(Progress::DidStart));
139 }
134 } 140 }
135 Event::CheckEvent(None) => { 141 Event::CheckEvent(None) => {
136 // Watcher finished, replace it with a never channel to 142 // Watcher finished, replace it with a never channel to
137 // avoid busy-waiting. 143 // avoid busy-waiting.
138 assert!(self.check_process.take().is_some()); 144 let cargo_handle = self.cargo_handle.take().unwrap();
139 self.send(Message::Progress(Progress::DidFinish)); 145 let res = cargo_handle.join();
146 self.send(Message::Progress(Progress::DidFinish(res)));
140 } 147 }
141 Event::CheckEvent(Some(message)) => match message { 148 Event::CheckEvent(Some(message)) => match message {
142 cargo_metadata::Message::CompilerArtifact(msg) => { 149 cargo_metadata::Message::CompilerArtifact(msg) => {
@@ -161,11 +168,11 @@ impl FlycheckActor {
161 self.cancel_check_process(); 168 self.cancel_check_process();
162 } 169 }
163 fn cancel_check_process(&mut self) { 170 fn cancel_check_process(&mut self) {
164 if self.check_process.take().is_some() { 171 if self.cargo_handle.take().is_some() {
165 self.send(Message::Progress(Progress::DidCancel)); 172 self.send(Message::Progress(Progress::DidCancel));
166 } 173 }
167 } 174 }
168 fn start_check_process(&self) -> CargoHandle { 175 fn check_command(&self) -> Command {
169 let mut cmd = match &self.config { 176 let mut cmd = match &self.config {
170 FlycheckConfig::CargoCommand { 177 FlycheckConfig::CargoCommand {
171 command, 178 command,
@@ -197,8 +204,7 @@ impl FlycheckActor {
197 } 204 }
198 }; 205 };
199 cmd.current_dir(&self.workspace_root); 206 cmd.current_dir(&self.workspace_root);
200 207 cmd
201 CargoHandle::spawn(cmd)
202 } 208 }
203 209
204 fn send(&self, check_task: Message) { 210 fn send(&self, check_task: Message) {
@@ -207,49 +213,62 @@ impl FlycheckActor {
207} 213}
208 214
209struct CargoHandle { 215struct CargoHandle {
210 receiver: Receiver<cargo_metadata::Message>, 216 child: JodChild,
211 #[allow(unused)] 217 #[allow(unused)]
212 thread: jod_thread::JoinHandle, 218 thread: jod_thread::JoinHandle<io::Result<bool>>,
219 receiver: Receiver<cargo_metadata::Message>,
213} 220}
214 221
215impl CargoHandle { 222impl CargoHandle {
216 fn spawn(command: Command) -> CargoHandle { 223 fn spawn(mut child: JodChild) -> CargoHandle {
224 let child_stdout = child.stdout.take().unwrap();
217 let (sender, receiver) = unbounded(); 225 let (sender, receiver) = unbounded();
218 let actor = CargoActor::new(command, sender); 226 let actor = CargoActor::new(child_stdout, sender);
219 let thread = jod_thread::spawn(move || { 227 let thread = jod_thread::spawn(move || actor.run());
220 let _ = actor.run(); 228 CargoHandle { child, thread, receiver }
221 }); 229 }
222 CargoHandle { receiver, thread } 230 fn join(mut self) -> io::Result<()> {
231 // It is okay to ignore the result, as it only errors if the process is already dead
232 let _ = self.child.kill();
233 let exit_status = self.child.wait()?;
234 let read_at_least_one_message = self.thread.join()?;
235 if !exit_status.success() && !read_at_least_one_message {
236 // FIXME: Read the stderr to display the reason, see `read2()` reference in PR comment:
237 // https://github.com/rust-analyzer/rust-analyzer/pull/3632#discussion_r395605298
238 return Err(io::Error::new(
239 io::ErrorKind::Other,
240 format!(
241 "Cargo watcher failed,the command produced no valid metadata (exit code: {:?})",
242 exit_status
243 ),
244 ));
245 }
246 Ok(())
223 } 247 }
224} 248}
225 249
226struct CargoActor { 250struct CargoActor {
227 command: Command, 251 child_stdout: process::ChildStdout,
228 sender: Sender<cargo_metadata::Message>, 252 sender: Sender<cargo_metadata::Message>,
229} 253}
230 254
231impl CargoActor { 255impl CargoActor {
232 fn new(command: Command, sender: Sender<cargo_metadata::Message>) -> CargoActor { 256 fn new(
233 CargoActor { command, sender } 257 child_stdout: process::ChildStdout,
258 sender: Sender<cargo_metadata::Message>,
259 ) -> CargoActor {
260 CargoActor { child_stdout, sender }
234 } 261 }
235 fn run(mut self) -> io::Result<()> { 262 fn run(self) -> io::Result<bool> {
236 let child = self
237 .command
238 .stdout(Stdio::piped())
239 .stderr(Stdio::null())
240 .stdin(Stdio::null())
241 .spawn()?;
242 let mut child = ChildKiller(child);
243
244 // We manually read a line at a time, instead of using serde's 263 // We manually read a line at a time, instead of using serde's
245 // stream deserializers, because the deserializer cannot recover 264 // stream deserializers, because the deserializer cannot recover
246 // from an error, resulting in it getting stuck, because we try to 265 // from an error, resulting in it getting stuck, because we try to
247 // be resillient against failures. 266 // be resilient against failures.
248 // 267 //
249 // Because cargo only outputs one JSON object per line, we can 268 // Because cargo only outputs one JSON object per line, we can
250 // simply skip a line if it doesn't parse, which just ignores any 269 // simply skip a line if it doesn't parse, which just ignores any
251 // erroneus output. 270 // erroneus output.
252 let stdout = BufReader::new(child.stdout.take().unwrap()); 271 let stdout = BufReader::new(self.child_stdout);
253 let mut read_at_least_one_message = false; 272 let mut read_at_least_one_message = false;
254 for message in cargo_metadata::Message::parse_stream(stdout) { 273 for message in cargo_metadata::Message::parse_stream(stdout) {
255 let message = match message { 274 let message = match message {
@@ -264,50 +283,32 @@ impl CargoActor {
264 283
265 // Skip certain kinds of messages to only spend time on what's useful 284 // Skip certain kinds of messages to only spend time on what's useful
266 match &message { 285 match &message {
267 cargo_metadata::Message::CompilerArtifact(artifact) if artifact.fresh => continue, 286 cargo_metadata::Message::CompilerArtifact(artifact) if artifact.fresh => (),
268 cargo_metadata::Message::BuildScriptExecuted(_) 287 cargo_metadata::Message::BuildScriptExecuted(_)
269 | cargo_metadata::Message::Unknown => continue, 288 | cargo_metadata::Message::Unknown => (),
270 _ => { 289 _ => self.sender.send(message).unwrap(),
271 // if the send channel was closed, we want to shutdown
272 if self.sender.send(message).is_err() {
273 break;
274 }
275 }
276 } 290 }
277 } 291 }
278 292 Ok(read_at_least_one_message)
279 // It is okay to ignore the result, as it only errors if the process is already dead
280 let _ = child.kill();
281
282 let exit_status = child.wait()?;
283 if !exit_status.success() && !read_at_least_one_message {
284 // FIXME: Read the stderr to display the reason, see `read2()` reference in PR comment:
285 // https://github.com/rust-analyzer/rust-analyzer/pull/3632#discussion_r395605298
286
287 // FIXME: make the `message_send` to be `Sender<Result<CheckEvent, CargoError>>`
288 // to display user-caused misconfiguration errors instead of just logging them here
289 log::error!("Cargo watcher failed,the command produced no valid metadata (exit code: {:?}): {:?}", exit_status, self.command);
290 }
291 Ok(())
292 } 293 }
293} 294}
294 295
295struct ChildKiller(process::Child); 296struct JodChild(process::Child);
296 297
297impl ops::Deref for ChildKiller { 298impl ops::Deref for JodChild {
298 type Target = process::Child; 299 type Target = process::Child;
299 fn deref(&self) -> &process::Child { 300 fn deref(&self) -> &process::Child {
300 &self.0 301 &self.0
301 } 302 }
302} 303}
303 304
304impl ops::DerefMut for ChildKiller { 305impl ops::DerefMut for JodChild {
305 fn deref_mut(&mut self) -> &mut process::Child { 306 fn deref_mut(&mut self) -> &mut process::Child {
306 &mut self.0 307 &mut self.0
307 } 308 }
308} 309}
309 310
310impl Drop for ChildKiller { 311impl Drop for JodChild {
311 fn drop(&mut self) { 312 fn drop(&mut self) {
312 let _ = self.0.kill(); 313 let _ = self.0.kill();
313 } 314 }
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index e5194fe41..9fd16ef3b 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -216,7 +216,11 @@ impl GlobalState {
216 flycheck::Progress::DidCheckCrate(target) => { 216 flycheck::Progress::DidCheckCrate(target) => {
217 (Progress::Report, Some(target)) 217 (Progress::Report, Some(target))
218 } 218 }
219 flycheck::Progress::DidFinish | flycheck::Progress::DidCancel => { 219 flycheck::Progress::DidCancel => (Progress::End, None),
220 flycheck::Progress::DidFinish(result) => {
221 if let Err(err) = result {
222 log::error!("cargo check failed: {}", err)
223 }
220 (Progress::End, None) 224 (Progress::End, None)
221 } 225 }
222 }; 226 };