From 8bea5ace7e1d27a2d275549c28d334ac1fa2a63c Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 28 Mar 2020 11:24:42 +0100 Subject: :arrow_up: jod-thread --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d81c4839..bbeb00312 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -563,9 +563,9 @@ dependencies = [ [[package]] name = "jod-thread" -version = "0.1.0" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f52a11f73b88fab829a0e4d9e13ea5982c7ac457c72eb3541d82a4afdfce4ff" +checksum = "4022656272c3e564a7cdebcaaba6518d844b0d0c1836597196efb5bfeb98bb49" [[package]] name = "kernel32-sys" -- cgit v1.2.3 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 --- Cargo.lock | 1 + crates/ra_cargo_watch/Cargo.toml | 1 + crates/ra_cargo_watch/src/lib.rs | 48 +++++++--------------------------------- 3 files changed, 10 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bbeb00312..a3887ce99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -893,6 +893,7 @@ dependencies = [ "cargo_metadata", "crossbeam-channel", "insta", + "jod-thread", "log", "lsp-types", "serde_json", diff --git a/crates/ra_cargo_watch/Cargo.toml b/crates/ra_cargo_watch/Cargo.toml index 741345a21..300033a18 100644 --- a/crates/ra_cargo_watch/Cargo.toml +++ b/crates/ra_cargo_watch/Cargo.toml @@ -10,6 +10,7 @@ lsp-types = { version = "0.73.0", features = ["proposed"] } log = "0.4.8" cargo_metadata = "0.9.1" serde_json = "1.0.48" +jod-thread = "0.1.1" [dev-dependencies] insta = "0.15.0" 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 From 12297ab67533200748ee9f60da4bc86dee1133d9 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 28 Mar 2020 13:19:05 +0100 Subject: Fix race in the tests --- crates/ra_cargo_watch/src/lib.rs | 2 +- crates/ra_prof/src/lib.rs | 8 ++++++++ crates/rust-analyzer/tests/heavy_tests/support.rs | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index 1ced7712a..1cac954c3 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -35,10 +35,10 @@ pub struct CheckOptions { /// The spawned thread is shut down when this struct is dropped. #[derive(Debug)] pub struct CheckWatcher { - pub task_recv: Receiver, // XXX: drop order is significant cmd_send: Option>, handle: Option>, + pub task_recv: Receiver, } impl CheckWatcher { diff --git a/crates/ra_prof/src/lib.rs b/crates/ra_prof/src/lib.rs index 9e167db96..00ea3a9b0 100644 --- a/crates/ra_prof/src/lib.rs +++ b/crates/ra_prof/src/lib.rs @@ -339,6 +339,14 @@ pub fn print_backtrace() { let bt = backtrace::Backtrace::new(); eprintln!("{:?}", bt); } +#[cfg(not(feature = "backtrace"))] +pub fn print_backtrace() { + eprintln!( + r#"enable the backtrace feature: + ra_prof = {{ path = "../ra_prof", features = [ "backtrace"] }} +"# + ); +} thread_local!(static IN_SCOPE: RefCell = RefCell::new(false)); diff --git a/crates/rust-analyzer/tests/heavy_tests/support.rs b/crates/rust-analyzer/tests/heavy_tests/support.rs index 1d7062bdf..67f3c9332 100644 --- a/crates/rust-analyzer/tests/heavy_tests/support.rs +++ b/crates/rust-analyzer/tests/heavy_tests/support.rs @@ -83,9 +83,10 @@ pub fn project(fixture: &str) -> Server { pub struct Server { req_id: Cell, messages: RefCell>, - dir: TempDir, _thread: jod_thread::JoinHandle<()>, client: Connection, + /// XXX: remove the tempdir last + dir: TempDir, } impl Server { -- cgit v1.2.3