diff --git a/CHANGELOG.md b/CHANGELOG.md index f303456d..3757c90a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,11 @@ ## unreleased +- CHANGE: Make `Watcher` object safe [#336] - CHANGE: Change EventFn to take FnMut [#333] [#333]: https://github.com/notify-rs/notify/pull/333 +[#336]: https://github.com/notify-rs/notify/pull/336 ## 5.0.0-pre.10 (2021-06-04) diff --git a/examples/async_monitor.rs b/examples/async_monitor.rs index 5dccb68a..a0a9ab87 100644 --- a/examples/async_monitor.rs +++ b/examples/async_monitor.rs @@ -7,7 +7,7 @@ fn async_watcher() -> notify::Result<(RecommendedWatcher, Receiver>(path: P) -> notify::Result<()> { // Add a path to be watched. All files and directories at that path and // below will be monitored for changes. - watcher.watch(path, RecursiveMode::Recursive)?; + watcher.watch(path.as_ref(), RecursiveMode::Recursive)?; while let Some(res) = rx.next().await { match res { diff --git a/examples/hot_reload_tide/src/main.rs b/examples/hot_reload_tide/src/main.rs index 0f2899b6..71f9acfd 100644 --- a/examples/hot_reload_tide/src/main.rs +++ b/examples/hot_reload_tide/src/main.rs @@ -8,6 +8,7 @@ use notify::{ event::ModifyKind, Error, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher, }; +use std::path::Path; use std::sync::{Arc, Mutex}; use tide::{Body, Response}; @@ -28,11 +29,11 @@ async fn main() -> tide::Result<()> { // We listen to file changes by giving Notify // a function that will get called when events happen - let mut watcher: RecommendedWatcher = + let mut watcher = // To make sure that the config lives as long as the function // we need to move the ownership of the config inside the function // To learn more about move please read [Using move Closures with Threads](https://doc.rust-lang.org/book/ch16-01-threads.html?highlight=move#using-move-closures-with-threads) - Watcher::new_immediate(move |result: Result| { + RecommendedWatcher::new(move |result: Result| { let event = result.unwrap(); if event.kind == EventKind::Modify(ModifyKind::Any) { @@ -43,7 +44,7 @@ async fn main() -> tide::Result<()> { } })?; - watcher.watch(CONFIG_PATH, RecursiveMode::Recursive)?; + watcher.watch(Path::new(CONFIG_PATH), RecursiveMode::Recursive)?; // We set up a web server using [Tide](https://github.com/http-rs/tide) let mut app = tide::with_state(config); diff --git a/examples/monitor_raw.rs b/examples/monitor_raw.rs index 0906eadd..39b35845 100644 --- a/examples/monitor_raw.rs +++ b/examples/monitor_raw.rs @@ -6,11 +6,11 @@ fn watch>(path: P) -> notify::Result<()> { // Automatically select the best implementation for your platform. // You can also access each implementation directly e.g. INotifyWatcher. - let mut watcher: RecommendedWatcher = Watcher::new_immediate(move |res| tx.send(res).unwrap())?; + let mut watcher = RecommendedWatcher::new(move |res| tx.send(res).unwrap())?; // Add a path to be watched. All files and directories at that path and // below will be monitored for changes. - watcher.watch(path, RecursiveMode::Recursive)?; + watcher.watch(path.as_ref(), RecursiveMode::Recursive)?; for res in rx { match res { diff --git a/src/fsevent.rs b/src/fsevent.rs index 72ac79a0..be3588de 100644 --- a/src/fsevent.rs +++ b/src/fsevent.rs @@ -20,7 +20,6 @@ use crossbeam_channel::{unbounded, Receiver, Sender}; use fsevent_sys as fs; use fsevent_sys::core_foundation as cf; use std::collections::HashMap; -use std::convert::AsRef; use std::ffi::CStr; use std::os::raw; use std::path::{Path, PathBuf}; @@ -250,6 +249,11 @@ extern "C" { } impl FsEventWatcher { + /// Create a new watcher. + pub fn new(event_fn: F) -> Result { + Self::from_event_fn(Arc::new(Mutex::new(event_fn))) + } + fn from_event_fn(event_fn: Arc>) -> Result { Ok(FsEventWatcher { paths: unsafe { @@ -309,14 +313,14 @@ impl FsEventWatcher { } } - fn remove_path>(&mut self, path: P) -> Result<()> { - let str_path = path.as_ref().to_str().unwrap(); + fn remove_path(&mut self, path: &Path) -> Result<()> { + let str_path = path.to_str().unwrap(); unsafe { let mut err: cf::CFErrorRef = ptr::null_mut(); let cf_path = cf::str_path_to_cfstring_ref(str_path, &mut err); if cf_path.is_null() { cf::CFRelease(err as cf::CFRef); - return Err(Error::watch_not_found().add_path(path.as_ref().into())); + return Err(Error::watch_not_found().add_path(path.into())); } let mut to_remove = Vec::new(); @@ -335,10 +339,10 @@ impl FsEventWatcher { cf::CFArrayRemoveValueAtIndex(self.paths, *idx); } } - let p = if let Ok(canonicalized_path) = path.as_ref().canonicalize() { + let p = if let Ok(canonicalized_path) = path.canonicalize() { canonicalized_path } else { - path.as_ref().to_owned() + path.to_owned() }; match self.recursive_info.remove(&p) { Some(_) => Ok(()), @@ -347,15 +351,15 @@ impl FsEventWatcher { } // https://github.com/thibaudgg/rb-fsevent/blob/master/ext/fsevent_watch/main.c - fn append_path>( + fn append_path( &mut self, - path: P, + path: &Path, recursive_mode: RecursiveMode, ) -> Result<()> { - if !path.as_ref().exists() { - return Err(Error::path_not_found().add_path(path.as_ref().into())); + if !path.exists() { + return Err(Error::path_not_found().add_path(path.into())); } - let str_path = path.as_ref().to_str().unwrap(); + let str_path = path.to_str().unwrap(); unsafe { let mut err: cf::CFErrorRef = ptr::null_mut(); let cf_path = cf::str_path_to_cfstring_ref(str_path, &mut err); @@ -363,13 +367,13 @@ impl FsEventWatcher { // Most likely the directory was deleted, or permissions changed, // while the above code was running. cf::CFRelease(err as cf::CFRef); - return Err(Error::path_not_found().add_path(path.as_ref().into())); + return Err(Error::path_not_found().add_path(path.into())); } cf::CFArrayAppendValue(self.paths, cf_path); cf::CFRelease(cf_path); } self.recursive_info.insert( - path.as_ref().to_path_buf().canonicalize().unwrap(), + path.to_path_buf().canonicalize().unwrap(), recursive_mode.is_recursive(), ); Ok(()) @@ -539,16 +543,12 @@ unsafe fn callback_impl( } impl Watcher for FsEventWatcher { - fn new_immediate(event_fn: F) -> Result { - FsEventWatcher::from_event_fn(Arc::new(Mutex::new(event_fn))) - } - - fn watch>(&mut self, path: P, recursive_mode: RecursiveMode) -> Result<()> { - self.watch_inner(path.as_ref(), recursive_mode) + fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { + self.watch_inner(path, recursive_mode) } - fn unwatch>(&mut self, path: P) -> Result<()> { - self.unwatch_inner(path.as_ref()) + fn unwatch(&mut self, path: &Path) -> Result<()> { + self.unwatch_inner(path) } fn configure(&mut self, config: Config) -> Result { @@ -578,7 +578,7 @@ fn test_fsevent_watcher_drop() { let event_fn = move |res| tx.send(res).unwrap(); { - let mut watcher: RecommendedWatcher = Watcher::new_immediate(event_fn).unwrap(); + let mut watcher = FsEventWatcher::new(event_fn).unwrap(); watcher.watch(dir.path(), RecursiveMode::Recursive).unwrap(); thread::sleep(Duration::from_millis(2000)); println!("is running -> {}", watcher.is_running()); @@ -599,7 +599,7 @@ fn test_fsevent_watcher_drop() { } #[test] -fn test_steam_context_info_send() { - fn check_send() {} +fn test_steam_context_info_send_and_sync() { + fn check_send() {} check_send::(); } diff --git a/src/inotify.rs b/src/inotify.rs index 01e36842..8891d679 100644 --- a/src/inotify.rs +++ b/src/inotify.rs @@ -554,6 +554,11 @@ fn filter_dir(e: walkdir::Result) -> Option(event_fn: F) -> Result { + Self::from_event_fn(Box::new(event_fn)) + } + fn from_event_fn(event_fn: Box) -> Result { let inotify = Inotify::init()?; let event_loop = EventLoop::new(inotify, event_fn)?; @@ -597,16 +602,12 @@ impl INotifyWatcher { } impl Watcher for INotifyWatcher { - fn new_immediate(event_fn: F) -> Result { - INotifyWatcher::from_event_fn(Box::new(event_fn)) - } - - fn watch>(&mut self, path: P, recursive_mode: RecursiveMode) -> Result<()> { - self.watch_inner(path.as_ref(), recursive_mode) + fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { + self.watch_inner(path, recursive_mode) } - fn unwatch>(&mut self, path: P) -> Result<()> { - self.unwatch_inner(path.as_ref()) + fn unwatch(&mut self, path: &Path) -> Result<()> { + self.unwatch_inner(path) } fn configure(&mut self, config: Config) -> Result { @@ -624,3 +625,9 @@ impl Drop for INotifyWatcher { self.waker.wake().unwrap(); } } + +#[test] +fn inotify_watcher_is_send_and_sync() { + fn check() {} + check::(); +} diff --git a/src/lib.rs b/src/lib.rs index 41ca40a2..1a700382 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,11 +20,12 @@ //! # Examples //! //! ``` +//! # use std::path::Path; //! use notify::{Watcher, RecommendedWatcher, RecursiveMode, Result}; //! //! fn main() -> Result<()> { //! // Automatically select the best implementation for your platform. -//! let mut watcher: RecommendedWatcher = Watcher::new_immediate(|res| { +//! let mut watcher = notify::recommended_watcher(|res| { //! match res { //! Ok(event) => println!("event: {:?}", event), //! Err(e) => println!("watch error: {:?}", e), @@ -33,7 +34,7 @@ //! //! // Add a path to be watched. All files and directories at that path and //! // below will be monitored for changes. -//! watcher.watch(".", RecursiveMode::Recursive)?; +//! watcher.watch(Path::new("."), RecursiveMode::Recursive)?; //! //! Ok(()) //! } @@ -48,10 +49,11 @@ //! //! ``` //! # use notify::{Watcher, RecommendedWatcher, RecursiveMode, Result}; +//! # use std::path::Path; //! # use std::time::Duration; //! # fn main() -> Result<()> { //! # // Automatically select the best implementation for your platform. -//! # let mut watcher: RecommendedWatcher = Watcher::new_immediate(|res| { +//! # let mut watcher = RecommendedWatcher::new(|res| { //! # match res { //! # Ok(event) => println!("event: {:?}", event), //! # Err(e) => println!("watch error: {:?}", e), @@ -60,7 +62,7 @@ //! //! # // Add a path to be watched. All files and directories at that path and //! # // below will be monitored for changes. -//! # watcher.watch(".", RecursiveMode::Recursive)?; +//! # watcher.watch(Path::new("."), RecursiveMode::Recursive)?; //! //! use notify::Config; //! watcher.configure(Config::PreciseEvents(true))?; @@ -77,6 +79,7 @@ //! //! ``` //! # use notify::{RecommendedWatcher, RecursiveMode, Result, Watcher}; +//! # use std::path::Path; //! # //! # fn main() -> Result<()> { //! fn event_fn(res: Result) { @@ -86,10 +89,10 @@ //! } //! } //! -//! let mut watcher1: RecommendedWatcher = Watcher::new_immediate(event_fn)?; -//! let mut watcher2: RecommendedWatcher = Watcher::new_immediate(event_fn)?; -//! # watcher1.watch(".", RecursiveMode::Recursive)?; -//! # watcher2.watch(".", RecursiveMode::Recursive)?; +//! let mut watcher1 = notify::recommended_watcher(event_fn)?; +//! let mut watcher2 = notify::recommended_watcher(event_fn)?; +//! # watcher1.watch(Path::new("."), RecursiveMode::Recursive)?; +//! # watcher2.watch(Path::new("."), RecursiveMode::Recursive)?; //! # //! # Ok(()) //! # } @@ -100,7 +103,6 @@ pub use config::{Config, RecursiveMode}; pub use error::{Error, ErrorKind, Result}; pub use event::{Event, EventKind}; -use std::convert::AsRef; use std::path::Path; #[cfg(target_os = "macos")] @@ -136,14 +138,7 @@ impl EventFn for F where F: 'static + FnMut(Result) + Send {} /// Watcher is implemented per platform using the best implementation available on that platform. /// In addition to such event driven implementations, a polling implementation is also provided /// that should work on any platform. -pub trait Watcher: Sized { - /// Create a new watcher in _immediate_ mode. - /// - /// Events will be sent using the provided `tx` immediately after they occur. - fn new_immediate(event_fn: F) -> Result - where - F: EventFn; - +pub trait Watcher { /// Begin watching a new path. /// /// If the `path` is a directory, `recursive_mode` will be evaluated. If `recursive_mode` is @@ -159,7 +154,7 @@ pub trait Watcher: Sized { /// /// [#165]: https://github.com/notify-rs/notify/issues/165 /// [#166]: https://github.com/notify-rs/notify/issues/166 - fn watch>(&mut self, path: P, recursive_mode: RecursiveMode) -> Result<()>; + fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()>; /// Stop watching a path. /// @@ -167,7 +162,7 @@ pub trait Watcher: Sized { /// /// Returns an error in the case that `path` has not been watched or if removing the watch /// fails. - fn unwatch>(&mut self, path: P) -> Result<()>; + fn unwatch(&mut self, path: &Path) -> Result<()>; /// Configure the watcher at runtime. /// @@ -200,9 +195,20 @@ pub type RecommendedWatcher = PollWatcher; /// _immediate_ mode. /// /// See [`Watcher::new_immediate`](trait.Watcher.html#tymethod.new_immediate). -pub fn immediate_watcher(event_fn: F) -> Result +pub fn recommended_watcher(event_fn: F) -> Result where F: EventFn, { - Watcher::new_immediate(event_fn) + // All recommended watchers currently implement `new`, so just call that. + RecommendedWatcher::new(event_fn) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_object_safe() { + let _watcher: &dyn Watcher = &NullWatcher; + } } diff --git a/src/null.rs b/src/null.rs index 5bfdf3bb..0911740c 100644 --- a/src/null.rs +++ b/src/null.rs @@ -2,7 +2,7 @@ #![allow(unused_variables)] -use super::{EventFn, RecursiveMode, Result, Watcher}; +use super::{RecursiveMode, Result, Watcher}; use std::path::Path; /// Stub `Watcher` implementation @@ -11,15 +11,11 @@ use std::path::Path; pub struct NullWatcher; impl Watcher for NullWatcher { - fn new_immediate(_event_fn: F) -> Result { - Ok(NullWatcher) - } - - fn watch>(&mut self, path: P, recursive_mode: RecursiveMode) -> Result<()> { + fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { Ok(()) } - fn unwatch>(&mut self, path: P) -> Result<()> { + fn unwatch(&mut self, path: &Path) -> Result<()> { Ok(()) } } diff --git a/src/poll.rs b/src/poll.rs index 750edb77..ab51f166 100644 --- a/src/poll.rs +++ b/src/poll.rs @@ -43,7 +43,14 @@ fn emit_event(event_fn: &Mutex, res: Result) { } impl PollWatcher { - /// Create a PollWatcher which polls every `delay` milliseconds + /// Create a new [PollWatcher]. + pub fn new(event_fn: F) -> Result { + let event_fn = Arc::new(Mutex::new(event_fn)); + let delay = Duration::from_secs(30); + Self::with_delay(event_fn, delay) + } + + /// Create a [PollWatcher] which polls every `delay` milliseconds pub fn with_delay(event_fn: Arc>, delay: Duration) -> Result { let mut p = PollWatcher { event_fn, @@ -272,18 +279,12 @@ impl PollWatcher { } impl Watcher for PollWatcher { - fn new_immediate(event_fn: F) -> Result { - let event_fn = Arc::new(Mutex::new(event_fn)); - let delay = Duration::from_secs(30); - PollWatcher::with_delay(event_fn, delay) - } - - fn watch>(&mut self, path: P, recursive_mode: RecursiveMode) -> Result<()> { - self.watch_inner(path.as_ref(), recursive_mode) + fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { + self.watch_inner(path, recursive_mode) } - fn unwatch>(&mut self, path: P) -> Result<()> { - self.unwatch_inner(path.as_ref()) + fn unwatch(&mut self, path: &Path) -> Result<()> { + self.unwatch_inner(path) } } @@ -293,5 +294,8 @@ impl Drop for PollWatcher { } } -// Because all public methods are `&mut self` it's also perfectly safe to share references. -unsafe impl Sync for PollWatcher {} +#[test] +fn poll_watcher_is_send_and_sync() { + fn check() {} + check::(); +} diff --git a/src/windows.rs b/src/windows.rs index ffe77045..c3f334ee 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -403,6 +403,14 @@ pub struct ReadDirectoryChangesWatcher { } impl ReadDirectoryChangesWatcher { + pub fn new(event_fn: F) -> Result { + // create dummy channel for meta event + // TODO: determine the original purpose of this - can we remove it? + let (meta_tx, _) = unbounded(); + let event_fn = Arc::new(Mutex::new(event_fn)); + Self::create(event_fn, meta_tx) + } + pub fn create( event_fn: Arc>, meta_tx: Sender, @@ -491,20 +499,12 @@ impl ReadDirectoryChangesWatcher { } impl Watcher for ReadDirectoryChangesWatcher { - fn new_immediate(event_fn: F) -> Result { - // create dummy channel for meta event - // TODO: determine the original purpose of this - can we remove it? - let (meta_tx, _) = unbounded(); - let event_fn = Arc::new(Mutex::new(event_fn)); - ReadDirectoryChangesWatcher::create(event_fn, meta_tx) - } - - fn watch>(&mut self, path: P, recursive_mode: RecursiveMode) -> Result<()> { - self.watch_inner(path.as_ref(), recursive_mode) + fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) -> Result<()> { + self.watch_inner(path, recursive_mode) } - fn unwatch>(&mut self, path: P) -> Result<()> { - self.unwatch_inner(path.as_ref()) + fn unwatch(&mut self, path: &Path) -> Result<()> { + self.unwatch_inner(path) } fn configure(&mut self, config: Config) -> Result { diff --git a/tests/race-with-remove-dir.rs b/tests/race-with-remove-dir.rs index fd753a5b..a09f53ff 100644 --- a/tests/race-with-remove-dir.rs +++ b/tests/race-with-remove-dir.rs @@ -1,6 +1,6 @@ use std::{fs, thread, time::Duration}; -use notify::{immediate_watcher, RecursiveMode, Watcher}; +use notify::{RecursiveMode, Watcher}; /// Test for . /// Note: This test will fail if your temp directory is not writable. @@ -11,12 +11,12 @@ fn test_race_with_remove_dir() { { let tmpdir = tmpdir.path().to_path_buf(); thread::spawn(move || { - let mut watcher = immediate_watcher(move |result| { + let mut watcher = notify::recommended_watcher(move |result| { eprintln!("received event: {:?}", result); }) .unwrap(); - watcher.watch(tmpdir, RecursiveMode::NonRecursive).unwrap(); + watcher.watch(&tmpdir, RecursiveMode::NonRecursive).unwrap(); }); }