From f7df0b56a76360419b31d8030c5c250bd54d8b6d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 28 Mar 2020 11:31:10 +0100 Subject: Use automatic thread joining for cargo-watch --- crates/ra_cargo_watch/src/lib.rs | 48 +++++++--------------------------------- 1 file changed, 8 insertions(+), 40 deletions(-) (limited to 'crates/ra_cargo_watch/src/lib.rs') diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index 7c525c430..1ced7712a 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -12,7 +12,6 @@ use std::{ io::{BufRead, BufReader}, path::{Path, PathBuf}, process::{Command, Stdio}, - thread::JoinHandle, time::Instant, }; @@ -37,8 +36,9 @@ pub struct CheckOptions { #[derive(Debug)] pub struct CheckWatcher { pub task_recv: Receiver, + // XXX: drop order is significant cmd_send: Option>, - handle: Option>, + handle: Option>, } impl CheckWatcher { @@ -47,7 +47,7 @@ impl CheckWatcher { let (task_send, task_recv) = unbounded::(); let (cmd_send, cmd_recv) = unbounded::(); - let handle = std::thread::spawn(move || { + let handle = jod_thread::spawn(move || { let mut check = CheckWatcherThread::new(options, workspace_root); check.run(&task_send, &cmd_recv); }); @@ -67,22 +67,6 @@ impl CheckWatcher { } } -impl std::ops::Drop for CheckWatcher { - fn drop(&mut self) { - if let Some(handle) = self.handle.take() { - // Take the sender out of the option - let cmd_send = self.cmd_send.take(); - - // Dropping the sender finishes the thread loop - drop(cmd_send); - - // Join the thread, it should finish shortly. We don't really care - // whether it panicked, so it is safe to ignore the result - let _ = handle.join(); - } - } -} - #[derive(Debug)] pub enum CheckTask { /// Request a clearing of all cached diagnostics from the check watcher @@ -237,8 +221,9 @@ pub struct DiagnosticWithFixes { /// The correct way to dispose of the thread is to drop it, on which the /// sub-process will be killed, and the thread will be joined. struct WatchThread { - handle: Option>, + // XXX: drop order is significant message_recv: Receiver, + _handle: Option>, } enum CheckEvent { @@ -333,7 +318,7 @@ pub fn run_cargo( impl WatchThread { fn dummy() -> WatchThread { - WatchThread { handle: None, message_recv: never() } + WatchThread { message_recv: never(), _handle: None } } fn new(options: &CheckOptions, workspace_root: &Path) -> WatchThread { @@ -352,7 +337,7 @@ impl WatchThread { let (message_send, message_recv) = unbounded(); let workspace_root = workspace_root.to_owned(); let handle = if options.enable { - Some(std::thread::spawn(move || { + Some(jod_thread::spawn(move || { // If we trigger an error here, we will do so in the loop instead, // which will break out of the loop, and continue the shutdown let _ = message_send.send(CheckEvent::Begin); @@ -383,23 +368,6 @@ impl WatchThread { } else { None }; - WatchThread { handle, message_recv } - } -} - -impl std::ops::Drop for WatchThread { - fn drop(&mut self) { - if let Some(handle) = self.handle.take() { - // Replace our reciever with dummy one, so we can drop and close the - // one actually communicating with the thread - let recv = std::mem::replace(&mut self.message_recv, never()); - - // Dropping the original reciever initiates thread sub-process shutdown - drop(recv); - - // Join the thread, it should finish shortly. We don't really care - // whether it panicked, so it is safe to ignore the result - let _ = handle.join(); - } + WatchThread { message_recv, _handle: handle } } } -- cgit v1.2.3