From d151b2a655057b9da45243d0f4e160b10a98ac37 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 14:05:08 +0300 Subject: remove useless Arc --- crates/ra_vfs/src/roots.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/ra_vfs/src/roots.rs b/crates/ra_vfs/src/roots.rs index 5e2776a35..9d3918502 100644 --- a/crates/ra_vfs/src/roots.rs +++ b/crates/ra_vfs/src/roots.rs @@ -1,6 +1,5 @@ use std::{ iter, - sync::Arc, path::{Path, PathBuf}, }; @@ -25,7 +24,7 @@ struct RootData { } pub(crate) struct Roots { - roots: Arena>, + roots: Arena, } impl Roots { @@ -38,9 +37,7 @@ impl Roots { let nested_roots = paths[..i].iter().filter_map(|it| rel_path(path, it)).collect::>(); - let config = Arc::new(RootData::new(path.clone(), nested_roots)); - - roots.alloc(config.clone()); + roots.alloc(RootData::new(path.clone(), nested_roots)); } Roots { roots } } -- cgit v1.2.3 From 4c154c289e3c18c2517ab8ce91e1d61f45f70388 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 14:13:13 +0300 Subject: remove arena from Roots we want to move ra_vfs to a new repo, so having fewer deps is useful. Arena is a thin layer of sugar on top of Vec anyway. --- crates/ra_vfs/src/lib.rs | 14 +++++++------- crates/ra_vfs/src/roots.rs | 20 +++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index 7f555a3c0..1c5402aed 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -25,7 +25,7 @@ use std::{ }; use crossbeam_channel::Receiver; -use ra_arena::{impl_arena_id, Arena, RawId, map::ArenaMap}; +use ra_arena::{impl_arena_id, Arena, RawId}; use relative_path::{RelativePath, RelativePathBuf}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -53,7 +53,7 @@ struct VfsFileData { pub struct Vfs { roots: Arc, files: Arena, - root2files: ArenaMap>, + root2files: FxHashMap>, pending_changes: Vec, worker: Worker, } @@ -72,7 +72,7 @@ impl Vfs { pub fn new(roots: Vec) -> (Vfs, Vec) { let roots = Arc::new(Roots::new(roots)); let worker = io::start(Arc::clone(&roots)); - let mut root2files = ArenaMap::default(); + let mut root2files = FxHashMap::default(); for root in roots.iter() { root2files.insert(root, Default::default()); @@ -164,7 +164,7 @@ impl Vfs { let mut cur_files = Vec::new(); // While we were scanning the root in the background, a file might have // been open in the editor, so we need to account for that. - let existing = self.root2files[root] + let existing = self.root2files[&root] .iter() .map(|&file| (self.files[file].path.clone(), file)) .collect::>(); @@ -241,7 +241,7 @@ impl Vfs { ) -> VfsFile { let data = VfsFileData { root, path, text, is_overlayed }; let file = self.files.alloc(data); - self.root2files.get_mut(root).unwrap().insert(file); + self.root2files.get_mut(&root).unwrap().insert(file); file } @@ -256,7 +256,7 @@ impl Vfs { self.files[file].text = Default::default(); self.files[file].path = Default::default(); let root = self.files[file].root; - let removed = self.root2files.get_mut(root).unwrap().remove(&file); + let removed = self.root2files.get_mut(&root).unwrap().remove(&file); assert!(removed); } @@ -267,7 +267,7 @@ impl Vfs { } fn find_file(&self, root: VfsRoot, path: &RelativePath) -> Option { - self.root2files[root].iter().map(|&it| it).find(|&file| self.files[file].path == path) + self.root2files[&root].iter().map(|&it| it).find(|&file| self.files[file].path == path) } } diff --git a/crates/ra_vfs/src/roots.rs b/crates/ra_vfs/src/roots.rs index 9d3918502..4503458ee 100644 --- a/crates/ra_vfs/src/roots.rs +++ b/crates/ra_vfs/src/roots.rs @@ -4,12 +4,10 @@ use std::{ }; use relative_path::{ RelativePath, RelativePathBuf}; -use ra_arena::{impl_arena_id, Arena, RawId}; /// VfsRoot identifies a watched directory on the file system. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct VfsRoot(pub RawId); -impl_arena_id!(VfsRoot); +pub struct VfsRoot(pub u32); /// Describes the contents of a single source root. /// @@ -24,12 +22,12 @@ struct RootData { } pub(crate) struct Roots { - roots: Arena, + roots: Vec, } impl Roots { pub(crate) fn new(mut paths: Vec) -> Roots { - let mut roots = Arena::default(); + let mut roots = Vec::new(); // A hack to make nesting work. paths.sort_by_key(|it| std::cmp::Reverse(it.as_os_str().len())); paths.dedup(); @@ -37,7 +35,7 @@ impl Roots { let nested_roots = paths[..i].iter().filter_map(|it| rel_path(path, it)).collect::>(); - roots.alloc(RootData::new(path.clone(), nested_roots)); + roots.push(RootData::new(path.clone(), nested_roots)); } Roots { roots } } @@ -51,20 +49,24 @@ impl Roots { self.roots.len() } pub(crate) fn iter<'a>(&'a self) -> impl Iterator + 'a { - self.roots.iter().map(|(id, _)| id) + (0..self.roots.len()).into_iter().map(|idx| VfsRoot(idx as u32)) } pub(crate) fn path(&self, root: VfsRoot) -> &Path { - self.roots[root].path.as_path() + self.root(root).path.as_path() } /// Checks if root contains a path and returns a root-relative path. pub(crate) fn contains(&self, root: VfsRoot, path: &Path) -> Option { - let data = &self.roots[root]; + let data = self.root(root); iter::once(&data.path) .chain(data.canonical_path.as_ref().into_iter()) .find_map(|base| rel_path(base, path)) .filter(|path| !data.excluded_dirs.contains(path)) .filter(|path| !data.is_excluded(path)) } + + fn root(&self, root: VfsRoot) -> &RootData { + &self.roots[root.0 as usize] + } } impl RootData { -- cgit v1.2.3 From 74288ae272029262beffb7696752a222e36fe49e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 14:20:54 +0300 Subject: remove depedency on ra_arena --- crates/ra_vfs/Cargo.toml | 1 - crates/ra_vfs/src/lib.rs | 40 +++++++++++++++++++++++----------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/crates/ra_vfs/Cargo.toml b/crates/ra_vfs/Cargo.toml index 2da5c499b..451daed1b 100644 --- a/crates/ra_vfs/Cargo.toml +++ b/crates/ra_vfs/Cargo.toml @@ -15,7 +15,6 @@ drop_bomb = "0.1.0" parking_lot = "0.7.0" thread_worker = { path = "../thread_worker" } -ra_arena = { path = "../ra_arena" } [dev-dependencies] tempfile = "3" diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index 1c5402aed..d07bc5694 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -25,7 +25,6 @@ use std::{ }; use crossbeam_channel::Receiver; -use ra_arena::{impl_arena_id, Arena, RawId}; use relative_path::{RelativePath, RelativePathBuf}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -40,8 +39,7 @@ pub use crate::{ }; #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct VfsFile(pub RawId); -impl_arena_id!(VfsFile); +pub struct VfsFile(pub u32); struct VfsFileData { root: VfsRoot, @@ -52,7 +50,7 @@ struct VfsFileData { pub struct Vfs { roots: Arc, - files: Arena, + files: Vec, root2files: FxHashMap>, pending_changes: Vec, worker: Worker, @@ -78,8 +76,7 @@ impl Vfs { root2files.insert(root, Default::default()); worker.sender().send(io::Task::AddRoot { root }).unwrap(); } - let res = - Vfs { roots, files: Arena::default(), root2files, worker, pending_changes: Vec::new() }; + let res = Vfs { roots, files: Vec::new(), root2files, worker, pending_changes: Vec::new() }; let vfs_roots = res.roots.iter().collect(); (res, vfs_roots) } @@ -96,8 +93,8 @@ impl Vfs { } pub fn file2path(&self, file: VfsFile) -> PathBuf { - let rel_path = &self.files[file].path; - let root_path = &self.roots.path(self.files[file].root); + let rel_path = &self.file(file).path; + let root_path = &self.roots.path(self.file(file).root); rel_path.to_path(root_path) } @@ -166,11 +163,11 @@ impl Vfs { // been open in the editor, so we need to account for that. let existing = self.root2files[&root] .iter() - .map(|&file| (self.files[file].path.clone(), file)) + .map(|&file| (self.file(file).path.clone(), file)) .collect::>(); for (path, text) in files { if let Some(&file) = existing.get(&path) { - let text = Arc::clone(&self.files[file].text); + let text = Arc::clone(&self.file(file).text); cur_files.push((file, path, text)); continue; } @@ -184,7 +181,7 @@ impl Vfs { } TaskResult::SingleFile { root, path, text } => { let existing_file = self.find_file(root, &path); - if existing_file.map(|file| self.files[file].is_overlayed) == Some(true) { + if existing_file.map(|file| self.file(file).is_overlayed) == Some(true) { return; } match (existing_file, text) { @@ -240,22 +237,23 @@ impl Vfs { is_overlayed: bool, ) -> VfsFile { let data = VfsFileData { root, path, text, is_overlayed }; - let file = self.files.alloc(data); + let file = VfsFile(self.files.len() as u32); + self.files.push(data); self.root2files.get_mut(&root).unwrap().insert(file); file } fn raw_change_file(&mut self, file: VfsFile, new_text: Arc, is_overlayed: bool) { - let mut file_data = &mut self.files[file]; + let mut file_data = &mut self.file_mut(file); file_data.text = new_text; file_data.is_overlayed = is_overlayed; } fn raw_remove_file(&mut self, file: VfsFile) { // FIXME: use arena with removal - self.files[file].text = Default::default(); - self.files[file].path = Default::default(); - let root = self.files[file].root; + self.file_mut(file).text = Default::default(); + self.file_mut(file).path = Default::default(); + let root = self.file(file).root; let removed = self.root2files.get_mut(&root).unwrap().remove(&file); assert!(removed); } @@ -267,7 +265,15 @@ impl Vfs { } fn find_file(&self, root: VfsRoot, path: &RelativePath) -> Option { - self.root2files[&root].iter().map(|&it| it).find(|&file| self.files[file].path == path) + self.root2files[&root].iter().map(|&it| it).find(|&file| self.file(file).path == path) + } + + fn file(&self, file: VfsFile) -> &VfsFileData { + &self.files[file.0 as usize] + } + + fn file_mut(&mut self, file: VfsFile) -> &mut VfsFileData { + &mut self.files[file.0 as usize] } } -- cgit v1.2.3 From 062aa9723583cd6dae17b3bbe604ec7ad6c0394b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 14:21:25 +0300 Subject: move public API to top of the file --- Cargo.lock | 1 - crates/ra_vfs/src/lib.rs | 16 ++++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 39bfcde41..25ad749ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1128,7 +1128,6 @@ dependencies = [ "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "notify 4.0.9 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", - "ra_arena 0.1.0", "relative-path 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.0.7 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index d07bc5694..8005c4ff8 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -66,6 +66,14 @@ impl fmt::Debug for Vfs { } } +#[derive(Debug, Clone)] +pub enum VfsChange { + AddRoot { root: VfsRoot, files: Vec<(VfsFile, RelativePathBuf, Arc)> }, + AddFile { root: VfsRoot, file: VfsFile, path: RelativePathBuf, text: Arc }, + RemoveFile { root: VfsRoot, file: VfsFile, path: RelativePathBuf }, + ChangeFile { file: VfsFile, text: Arc }, +} + impl Vfs { pub fn new(roots: Vec) -> (Vfs, Vec) { let roots = Arc::new(Roots::new(roots)); @@ -276,11 +284,3 @@ impl Vfs { &mut self.files[file.0 as usize] } } - -#[derive(Debug, Clone)] -pub enum VfsChange { - AddRoot { root: VfsRoot, files: Vec<(VfsFile, RelativePathBuf, Arc)> }, - AddFile { root: VfsRoot, file: VfsFile, path: RelativePathBuf, text: Arc }, - RemoveFile { root: VfsRoot, file: VfsFile, path: RelativePathBuf }, - ChangeFile { file: VfsFile, text: Arc }, -} -- cgit v1.2.3 From c5a65466e264b850607113cbd6621e531cd58329 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 14:29:54 +0300 Subject: hide TaskResult from the public API --- crates/ra_vfs/src/io.rs | 19 +++++++++++-------- crates/ra_vfs/src/lib.rs | 24 +++++++++++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs index 0cffc03f3..8eb148a38 100644 --- a/crates/ra_vfs/src/io.rs +++ b/crates/ra_vfs/src/io.rs @@ -9,7 +9,7 @@ use relative_path::RelativePathBuf; use walkdir::WalkDir; use notify::{DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher as _Watcher}; -use crate::{Roots, VfsRoot}; +use crate::{Roots, VfsRoot, VfsTask}; pub(crate) enum Task { AddRoot { root: VfsRoot }, @@ -18,7 +18,7 @@ pub(crate) enum Task { /// `TaskResult` transfers files read on the IO thread to the VFS on the main /// thread. #[derive(Debug)] -pub enum TaskResult { +pub(crate) enum TaskResult { /// Emitted when we've recursively scanned a source root during the initial /// load. BulkLoadRoot { root: VfsRoot, files: Vec<(RelativePathBuf, String)> }, @@ -46,7 +46,7 @@ enum ChangeKind { const WATCHER_DELAY: Duration = Duration::from_millis(250); -pub(crate) type Worker = thread_worker::Worker; +pub(crate) type Worker = thread_worker::Worker; pub(crate) fn start(roots: Arc) -> Worker { // This is a pretty elaborate setup of threads & channels! It is // explained by the following concerns: @@ -122,7 +122,7 @@ pub(crate) fn start(roots: Arc) -> Worker { fn watch_root( watcher: Option<&mut RecommendedWatcher>, - sender: &Sender, + sender: &Sender, roots: &Roots, root: VfsRoot, ) { @@ -136,7 +136,8 @@ fn watch_root( Some((path, text)) }) .collect(); - sender.send(TaskResult::BulkLoadRoot { root, files }).unwrap(); + let res = TaskResult::BulkLoadRoot { root, files }; + sender.send(VfsTask(res)).unwrap(); log::debug!("... loaded {}", root_path.display()); } @@ -173,7 +174,7 @@ fn convert_notify_event(event: DebouncedEvent, sender: &Sender<(PathBuf, ChangeK fn handle_change( watcher: Option<&mut RecommendedWatcher>, - sender: &Sender, + sender: &Sender, roots: &Roots, path: PathBuf, kind: ChangeKind, @@ -195,13 +196,15 @@ fn handle_change( .try_for_each(|rel_path| { let abs_path = rel_path.to_path(&roots.path(root)); let text = read_to_string(&abs_path); - sender.send(TaskResult::SingleFile { root, path: rel_path, text }) + let res = TaskResult::SingleFile { root, path: rel_path, text }; + sender.send(VfsTask(res)) }) .unwrap() } ChangeKind::Write | ChangeKind::Remove => { let text = read_to_string(&path); - sender.send(TaskResult::SingleFile { root, path: rel_path, text }).unwrap(); + let res = TaskResult::SingleFile { root, path: rel_path, text }; + sender.send(VfsTask(res)).unwrap(); } } } diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index 8005c4ff8..3cd11c9f6 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -33,10 +33,20 @@ use crate::{ roots::Roots, }; -pub use crate::{ - io::TaskResult as VfsTask, - roots::VfsRoot, -}; +pub use crate::roots::VfsRoot; + +/// Opaque wrapper around file-system event. +/// +/// Calling code is expected to just pass `VfsTask` to `handle_task` method. It +/// is exposed as a public API so that the caller can plug vfs events into the +/// main event loop and be notified when changes happen. +pub struct VfsTask(TaskResult); + +impl fmt::Debug for VfsTask { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("VfsTask { ... }") + } +} #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct VfsFile(pub u32); @@ -159,12 +169,12 @@ impl Vfs { mem::replace(&mut self.pending_changes, Vec::new()) } - pub fn task_receiver(&self) -> &Receiver { + pub fn task_receiver(&self) -> &Receiver { self.worker.receiver() } - pub fn handle_task(&mut self, task: io::TaskResult) { - match task { + pub fn handle_task(&mut self, task: VfsTask) { + match task.0 { TaskResult::BulkLoadRoot { root, files } => { let mut cur_files = Vec::new(); // While we were scanning the root in the background, a file might have -- cgit v1.2.3 From 9da37051911132279131530e958b0adc0a5a9aaa Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 14:39:18 +0300 Subject: drop unused extern crate --- Cargo.lock | 1 - crates/ra_vfs/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 25ad749ee..65f839fe8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1123,7 +1123,6 @@ name = "ra_vfs" version = "0.1.0" dependencies = [ "crossbeam-channel 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", - "drop_bomb 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "flexi_logger 0.10.5 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "notify 4.0.9 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/crates/ra_vfs/Cargo.toml b/crates/ra_vfs/Cargo.toml index 451daed1b..2fe3102ab 100644 --- a/crates/ra_vfs/Cargo.toml +++ b/crates/ra_vfs/Cargo.toml @@ -11,7 +11,6 @@ rustc-hash = "1.0" crossbeam-channel = "0.3.5" log = "0.4.6" notify = "4.0.9" -drop_bomb = "0.1.0" parking_lot = "0.7.0" thread_worker = { path = "../thread_worker" } -- cgit v1.2.3 From def7bc0ec55a0afb2cb577e2e80d95b33a1cf115 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 15:30:58 +0300 Subject: drop dependency on thread_worker --- Cargo.lock | 1 - crates/ra_vfs/Cargo.toml | 2 - crates/ra_vfs/src/io.rs | 159 +++++++++++++++++++++++++++++------------------ crates/ra_vfs/src/lib.rs | 4 +- 4 files changed, 99 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 65f839fe8..8f087749f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1130,7 +1130,6 @@ dependencies = [ "relative-path 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.0.7 (registry+https://github.com/rust-lang/crates.io-index)", - "thread_worker 0.1.0", "walkdir 2.2.7 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/crates/ra_vfs/Cargo.toml b/crates/ra_vfs/Cargo.toml index 2fe3102ab..fdaf31b9c 100644 --- a/crates/ra_vfs/Cargo.toml +++ b/crates/ra_vfs/Cargo.toml @@ -13,8 +13,6 @@ log = "0.4.6" notify = "4.0.9" parking_lot = "0.7.0" -thread_worker = { path = "../thread_worker" } - [dev-dependencies] tempfile = "3" flexi_logger = "0.10.0" diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs index 8eb148a38..b6a057697 100644 --- a/crates/ra_vfs/src/io.rs +++ b/crates/ra_vfs/src/io.rs @@ -3,8 +3,9 @@ use std::{ path::{Path, PathBuf}, sync::{mpsc, Arc}, time::Duration, + thread, }; -use crossbeam_channel::{Sender, unbounded, RecvError, select}; +use crossbeam_channel::{Sender, Receiver, unbounded, RecvError, select}; use relative_path::RelativePathBuf; use walkdir::WalkDir; use notify::{DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher as _Watcher}; @@ -46,7 +47,40 @@ enum ChangeKind { const WATCHER_DELAY: Duration = Duration::from_millis(250); -pub(crate) type Worker = thread_worker::Worker; +// Like thread::JoinHandle, but joins the thread on drop. +// +// This is useful because it guarantees the absence of run-away threads, even if +// code panics. This is important, because we might seem panics in the test and +// we might be used in an IDE context, where a failed component is just +// restarted. +// +// Because all threads are joined, care must be taken to avoid deadlocks. That +// typically means ensuring that channels are dropped before the threads. +struct ScopedThread(Option>); + +impl ScopedThread { + fn spawn(name: String, f: impl FnOnce() + Send + 'static) -> ScopedThread { + let handle = thread::Builder::new().name(name).spawn(f).unwrap(); + ScopedThread(Some(handle)) + } +} + +impl Drop for ScopedThread { + fn drop(&mut self) { + let res = self.0.take().unwrap().join(); + if !thread::panicking() { + res.unwrap(); + } + } +} + +pub(crate) struct Worker { + // XXX: it's important to drop `sender` before `_thread` to avoid deadlock. + pub(crate) sender: Sender, + _thread: ScopedThread, + pub(crate) receiver: Receiver, +} + pub(crate) fn start(roots: Arc) -> Worker { // This is a pretty elaborate setup of threads & channels! It is // explained by the following concerns: @@ -55,69 +89,70 @@ pub(crate) fn start(roots: Arc) -> Worker { // * we want to read all files from a single thread, to guarantee that // we always get fresher versions and never go back in time. // * we want to tear down everything neatly during shutdown. - Worker::spawn( - "vfs", - 128, - // This are the channels we use to communicate with outside world. - // If `input_receiver` is closed we need to tear ourselves down. - // `output_sender` should not be closed unless the parent died. - move |input_receiver, output_sender| { - // Make sure that the destruction order is - // - // * notify_sender - // * _thread - // * watcher_sender - // - // this is required to avoid deadlocks. - - // These are the corresponding crossbeam channels - let (watcher_sender, watcher_receiver) = unbounded(); - let _thread; - { - // These are `std` channels notify will send events to - let (notify_sender, notify_receiver) = mpsc::channel(); - - let mut watcher = notify::watcher(notify_sender, WATCHER_DELAY) - .map_err(|e| log::error!("failed to spawn notify {}", e)) - .ok(); - // Start a silly thread to transform between two channels - _thread = thread_worker::ScopedThread::spawn("notify-convertor", move || { - notify_receiver - .into_iter() - .for_each(|event| convert_notify_event(event, &watcher_sender)) - }); - - // Process requests from the called or notifications from - // watcher until the caller says stop. - loop { - select! { - // Received request from the caller. If this channel is - // closed, we should shutdown everything. - recv(input_receiver) -> t => match t { - Err(RecvError) => { - drop(input_receiver); - break - }, - Ok(Task::AddRoot { root }) => { - watch_root(watcher.as_mut(), &output_sender, &*roots, root); - } - }, - // Watcher send us changes. If **this** channel is - // closed, the watcher has died, which indicates a bug - // -- escalate! - recv(watcher_receiver) -> event => match event { - Err(RecvError) => panic!("watcher is dead"), - Ok((path, change)) => { - handle_change(watcher.as_mut(), &output_sender, &*roots, path, change); - } + let _thread; + // This are the channels we use to communicate with outside world. + // If `input_receiver` is closed we need to tear ourselves down. + // `output_sender` should not be closed unless the parent died. + let (input_sender, input_receiver) = unbounded(); + let (output_sender, output_receiver) = unbounded(); + + _thread = ScopedThread::spawn("vfs".to_string(), move || { + // Make sure that the destruction order is + // + // * notify_sender + // * _thread + // * watcher_sender + // + // this is required to avoid deadlocks. + + // These are the corresponding crossbeam channels + let (watcher_sender, watcher_receiver) = unbounded(); + let _notify_thread; + { + // These are `std` channels notify will send events to + let (notify_sender, notify_receiver) = mpsc::channel(); + + let mut watcher = notify::watcher(notify_sender, WATCHER_DELAY) + .map_err(|e| log::error!("failed to spawn notify {}", e)) + .ok(); + // Start a silly thread to transform between two channels + _notify_thread = ScopedThread::spawn("notify-convertor".to_string(), move || { + notify_receiver + .into_iter() + .for_each(|event| convert_notify_event(event, &watcher_sender)) + }); + + // Process requests from the called or notifications from + // watcher until the caller says stop. + loop { + select! { + // Received request from the caller. If this channel is + // closed, we should shutdown everything. + recv(input_receiver) -> t => match t { + Err(RecvError) => { + drop(input_receiver); + break }, - } + Ok(Task::AddRoot { root }) => { + watch_root(watcher.as_mut(), &output_sender, &*roots, root); + } + }, + // Watcher send us changes. If **this** channel is + // closed, the watcher has died, which indicates a bug + // -- escalate! + recv(watcher_receiver) -> event => match event { + Err(RecvError) => panic!("watcher is dead"), + Ok((path, change)) => { + handle_change(watcher.as_mut(), &output_sender, &*roots, path, change); + } + }, } } - // Drain pending events: we are not interested in them anyways! - watcher_receiver.into_iter().for_each(|_| ()); - }, - ) + } + // Drain pending events: we are not interested in them anyways! + watcher_receiver.into_iter().for_each(|_| ()); + }); + Worker { sender: input_sender, _thread, receiver: output_receiver } } fn watch_root( diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index 3cd11c9f6..808c138df 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -92,7 +92,7 @@ impl Vfs { for root in roots.iter() { root2files.insert(root, Default::default()); - worker.sender().send(io::Task::AddRoot { root }).unwrap(); + worker.sender.send(io::Task::AddRoot { root }).unwrap(); } let res = Vfs { roots, files: Vec::new(), root2files, worker, pending_changes: Vec::new() }; let vfs_roots = res.roots.iter().collect(); @@ -170,7 +170,7 @@ impl Vfs { } pub fn task_receiver(&self) -> &Receiver { - self.worker.receiver() + &self.worker.receiver } pub fn handle_task(&mut self, task: VfsTask) { -- cgit v1.2.3 From d93097a4936b9eea98e4759fa2fde3a052acfb42 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 Feb 2019 16:20:12 +0300 Subject: better comments --- crates/ra_vfs/src/io.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs index b6a057697..5969ee0d0 100644 --- a/crates/ra_vfs/src/io.rs +++ b/crates/ra_vfs/src/io.rs @@ -50,7 +50,7 @@ const WATCHER_DELAY: Duration = Duration::from_millis(250); // Like thread::JoinHandle, but joins the thread on drop. // // This is useful because it guarantees the absence of run-away threads, even if -// code panics. This is important, because we might seem panics in the test and +// code panics. This is important, because we might see panics in the test and // we might be used in an IDE context, where a failed component is just // restarted. // @@ -75,7 +75,13 @@ impl Drop for ScopedThread { } pub(crate) struct Worker { - // XXX: it's important to drop `sender` before `_thread` to avoid deadlock. + // XXX: field order is significant here. + // + // In Rust, fields are dropped in the declaration order, and we rely on this + // here. We must close sender first, so that the `thread` (who holds the + // opposite side of the channel) noticed shutdown. Then, we must join the + // thread, but we must keep receiver alive so that the thread does not + // panic. pub(crate) sender: Sender, _thread: ScopedThread, pub(crate) receiver: Receiver, -- cgit v1.2.3