2 files changed, 12 insertions, 50 deletions

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..c67ec39d4 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,
 };
 
@@ -36,9 +35,10 @@ pub struct CheckOptions {
 /// The spawned thread is shut down when this struct is dropped.
 #[derive(Debug)]
 pub struct CheckWatcher {
+    // XXX: drop order is significant
+    cmd_send: Sender<CheckCommand>,
+    handle: Option<jod_thread::JoinHandle<()>>,
     pub task_recv: Receiver<CheckTask>,
-    cmd_send: Option<Sender<CheckCommand>>,
-    handle: Option<JoinHandle<()>>,
 }
 
 impl CheckWatcher {
@@ -47,39 +47,16 @@ impl CheckWatcher {
 
         let (task_send, task_recv) = unbounded::<CheckTask>();
         let (cmd_send, cmd_recv) = unbounded::<CheckCommand>();
-        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);
         });
-        CheckWatcher { task_recv, cmd_send: Some(cmd_send), handle: Some(handle) }
-    }
-
-    /// Returns a CheckWatcher that doesn't actually do anything
-    pub fn dummy() -> CheckWatcher {
-        CheckWatcher { task_recv: never(), cmd_send: None, handle: None }
+        CheckWatcher { task_recv, cmd_send, handle: Some(handle) }
     }
 
     /// Schedule a re-start of the cargo check worker.
     pub fn update(&self) {
-        if let Some(cmd_send) = &self.cmd_send {
-            cmd_send.send(CheckCommand::Update).unwrap();
-        }
-    }
-}
-
-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();
-        }
+        self.cmd_send.send(CheckCommand::Update).unwrap();
     }
 }
 
@@ -237,8 +214,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<JoinHandle<()>>,
+    // XXX: drop order is significant
     message_recv: Receiver<CheckEvent>,
+    _handle: Option<jod_thread::JoinHandle<()>>,
 }
 
 enum CheckEvent {
@@ -333,7 +311,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 +330,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 +361,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 }
     }
 }