8 files changed, 63 insertions, 86 deletions

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 5050c558c..fb7afe9fd 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -11,6 +11,7 @@ env:
   CARGO_INCREMENTAL: 0
   CARGO_NET_RETRY: 10
   RUN_SLOW_TESTS: 1
+  RUST_BACKTRACE: short
   RUSTFLAGS: -D warnings
   RUSTUP_MAX_RETRIES: 10
 
diff --git a/Cargo.lock b/Cargo.lock
index 8d81c4839..a3887ce99 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"
@@ -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..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 }
     }
 }
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<bool> = RefCell::new(false));
 
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 7825b0077..c899ff677 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -14,7 +14,7 @@ use std::{
     time::{Duration, Instant},
 };
 
-use crossbeam_channel::{select, unbounded, RecvError, Sender};
+use crossbeam_channel::{never, select, unbounded, RecvError, Sender};
 use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response};
 use lsp_types::{
     ClientCapabilities, NumberOrString, WorkDoneProgress, WorkDoneProgressBegin,
@@ -208,6 +208,9 @@ pub fn main_loop(
         )
     };
 
+    loop_state.roots_total = world_state.vfs.read().n_roots();
+    loop_state.roots_scanned = 0;
+
     let pool = ThreadPool::default();
     let (task_sender, task_receiver) = unbounded::<Task>();
     let (libdata_sender, libdata_receiver) = unbounded::<LibraryData>();
@@ -229,7 +232,7 @@ pub fn main_loop(
                     Err(RecvError) => return Err("vfs died".into()),
                 },
                 recv(libdata_receiver) -> data => Event::Lib(data.unwrap()),
-                recv(world_state.check_watcher.task_recv) -> task => match task {
+                recv(world_state.check_watcher.as_ref().map_or(&never(), |it| &it.task_recv)) -> task => match task {
                     Ok(task) => Event::CheckWatcher(task),
                     Err(RecvError) => return Err("check watcher died".into()),
                 }
@@ -333,7 +336,10 @@ struct LoopState {
     in_flight_libraries: usize,
     pending_libraries: Vec<(SourceRootId, Vec<(FileId, RelativePathBuf, Arc<String>)>)>,
     workspace_loaded: bool,
-    roots_scanned_progress: Option<usize>,
+
+    roots_progress_reported: Option<usize>,
+    roots_scanned: usize,
+    roots_total: usize,
 }
 
 impl LoopState {
@@ -377,6 +383,7 @@ fn loop_turn(
             world_state.add_lib(lib);
             world_state.maybe_collect_garbage();
             loop_state.in_flight_libraries -= 1;
+            loop_state.roots_scanned += 1;
         }
         Event::CheckWatcher(task) => on_check_task(task, world_state, task_sender)?,
         Event::Msg(msg) => match msg {
@@ -408,7 +415,7 @@ fn loop_turn(
     };
 
     let mut state_changed = false;
-    if let Some(changes) = world_state.process_changes() {
+    if let Some(changes) = world_state.process_changes(&mut loop_state.roots_scanned) {
         state_changed = true;
         loop_state.pending_libraries.extend(changes);
     }
@@ -427,21 +434,27 @@ fn loop_turn(
         });
     }
 
+    let show_progress = !loop_state.workspace_loaded
+        && world_state.feature_flags.get("notifications.workspace-loaded");
+
     if !loop_state.workspace_loaded
-        && world_state.roots_to_scan == 0
+        && loop_state.roots_scanned == loop_state.roots_total
         && loop_state.pending_libraries.is_empty()
         && loop_state.in_flight_libraries == 0
     {
         loop_state.workspace_loaded = true;
-        world_state.check_watcher.update();
+        if let Some(check_watcher) = &world_state.check_watcher {
+            check_watcher.update();
+        }
         pool.execute({
             let subs = loop_state.subscriptions.subscriptions();
             let snap = world_state.snapshot();
             move || snap.analysis().prime_caches(subs).unwrap_or_else(|_: Canceled| ())
         });
-        send_startup_progress(&connection.sender, loop_state, world_state);
-    } else if !loop_state.workspace_loaded {
-        send_startup_progress(&connection.sender, loop_state, world_state);
+    }
+
+    if show_progress {
+        send_startup_progress(&connection.sender, loop_state);
     }
 
     if state_changed {
@@ -604,7 +617,9 @@ fn on_notification(
     };
     let not = match notification_cast::<req::DidSaveTextDocument>(not) {
         Ok(_params) => {
-            state.check_watcher.update();
+            if let Some(check_watcher) = &state.check_watcher {
+                check_watcher.update();
+            }
             return Ok(());
         }
         Err(not) => not,
@@ -706,21 +721,13 @@ fn on_diagnostic_task(task: DiagnosticTask, msg_sender: &Sender<Message>, state:
     }
 }
 
-fn send_startup_progress(
-    sender: &Sender<Message>,
-    loop_state: &mut LoopState,
-    world_state: &WorldState,
-) {
-    if !world_state.feature_flags.get("notifications.workspace-loaded") {
-        return;
-    }
-
-    let total: usize = world_state.workspaces.iter().map(|it| it.n_packages()).sum();
-    let prev_progress = loop_state.roots_scanned_progress;
-    let progress = total - world_state.roots_to_scan;
-    loop_state.roots_scanned_progress = Some(progress);
+fn send_startup_progress(sender: &Sender<Message>, loop_state: &mut LoopState) {
+    let total: usize = loop_state.roots_total;
+    let prev = loop_state.roots_progress_reported;
+    let progress = loop_state.roots_scanned;
+    loop_state.roots_progress_reported = Some(progress);
 
-    match (prev_progress, loop_state.workspace_loaded) {
+    match (prev, loop_state.workspace_loaded) {
         (None, false) => {
             let work_done_progress_create = request_new::<req::WorkDoneProgressCreate>(
                 loop_state.next_request_id(),
diff --git a/crates/rust-analyzer/src/world.rs b/crates/rust-analyzer/src/world.rs
index 64a7b907e..ca045f93c 100644
--- a/crates/rust-analyzer/src/world.rs
+++ b/crates/rust-analyzer/src/world.rs
@@ -51,15 +51,13 @@ pub struct Options {
 pub struct WorldState {
     pub options: Options,
     pub feature_flags: Arc<FeatureFlags>,
-    //FIXME: this belongs to `LoopState` rather than to `WorldState`
-    pub roots_to_scan: usize,
     pub roots: Vec<PathBuf>,
     pub workspaces: Arc<Vec<ProjectWorkspace>>,
     pub analysis_host: AnalysisHost,
     pub vfs: Arc<RwLock<Vfs>>,
     pub task_receiver: Receiver<VfsTask>,
     pub latest_requests: Arc<RwLock<LatestRequests>>,
-    pub check_watcher: CheckWatcher,
+    pub check_watcher: Option<CheckWatcher>,
     pub diagnostics: DiagnosticCollection,
 }
 
@@ -123,7 +121,7 @@ impl WorldState {
         let (task_sender, task_receiver) = unbounded();
         let task_sender = Box::new(move |t| task_sender.send(t).unwrap());
         let (mut vfs, vfs_roots) = Vfs::new(roots, task_sender, watch);
-        let roots_to_scan = vfs_roots.len();
+
         for r in vfs_roots {
             let vfs_root_path = vfs.root2path(r);
             let is_local = folder_roots.iter().any(|it| vfs_root_path.starts_with(it));
@@ -178,11 +176,11 @@ impl WorldState {
             })
             .map(|cargo| {
                 let cargo_project_root = cargo.workspace_root().to_path_buf();
-                CheckWatcher::new(&options.cargo_watch, cargo_project_root)
+                Some(CheckWatcher::new(&options.cargo_watch, cargo_project_root))
             })
             .unwrap_or_else(|| {
                 log::warn!("Cargo check watching only supported for cargo workspaces, disabling");
-                CheckWatcher::dummy()
+                None
             });
 
         let mut analysis_host = AnalysisHost::new(lru_capacity);
@@ -190,7 +188,6 @@ impl WorldState {
         WorldState {
             options,
             feature_flags: Arc::new(feature_flags),
-            roots_to_scan,
             roots: folder_roots,
             workspaces: Arc::new(workspaces),
             analysis_host,
@@ -206,6 +203,7 @@ impl WorldState {
     /// FIXME: better API here
     pub fn process_changes(
         &mut self,
+        roots_scanned: &mut usize,
     ) -> Option<Vec<(SourceRootId, Vec<(FileId, RelativePathBuf, Arc<String>)>)>> {
         let changes = self.vfs.write().commit_changes();
         if changes.is_empty() {
@@ -219,7 +217,7 @@ impl WorldState {
                     let root_path = self.vfs.read().root2path(root);
                     let is_local = self.roots.iter().any(|r| root_path.starts_with(r));
                     if is_local {
-                        self.roots_to_scan -= 1;
+                        *roots_scanned += 1;
                         for (file, path, text) in files {
                             change.add_file(SourceRootId(root.0), FileId(file.0), path, text);
                         }
@@ -247,7 +245,6 @@ impl WorldState {
     }
 
     pub fn add_lib(&mut self, data: LibraryData) {
-        self.roots_to_scan -= 1;
         let mut change = AnalysisChange::new();
         change.add_library(data);
         self.analysis_host.apply_change(change);
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<u64>,
     messages: RefCell<Vec<Message>>,
-    dir: TempDir,
     _thread: jod_thread::JoinHandle<()>,
     client: Connection,
+    /// XXX: remove the tempdir last
+    dir: TempDir,
 }
 
 impl Server {