diff --git a/CHANGELOG.md b/CHANGELOG.md index 841055a9..57554f3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,12 @@ # Changelog -## notify 9.0.0 (unreleased) +## notify 9.0.0 (unreleased) - CHANGE: raise MSRV to 1.82 **breaking** - -## notify (unreleased) - FIX: Fix the bug that `FsEventWatcher` crashes when dealing with empty path [#718] +- FIX: Fix the bug that `INotifyWatcher` keeps watching deleted paths [#720] [#718]: https://github.com/notify-rs/notify/pull/718 - +[#720]: https://github.com/notify-rs/notify/pull/720 ## notify 8.2.0 (2025-08-03) - FEATURE: notify user if inotify's `max_user_watches` has been reached [#698] diff --git a/notify/src/inotify.rs b/notify/src/inotify.rs index fd27a39a..94f13f8b 100644 --- a/notify/src/inotify.rs +++ b/notify/src/inotify.rs @@ -377,7 +377,9 @@ impl EventLoop { } for path in remove_watches { - self.remove_watch(path, true).ok(); + if let Err(err) = self.remove_watch(path, true) { + log::warn!("Unable to remove the path from the watches: {err:?}"); + } } for path in add_watches { @@ -476,20 +478,16 @@ impl EventLoop { Some((w, _, is_recursive, _)) => { if let Some(ref mut inotify) = self.inotify { let mut inotify_watches = inotify.watches(); - log::trace!("removing inotify watch: {}", path.display()); + log::trace!("removing inotify watch for {path:?}, remove_recursive: {remove_recursive:?}"); - inotify_watches - .remove(w.clone()) - .map_err(|e| Error::io(e).add_path(path.clone()))?; + Self::remove_single_descriptor(&mut inotify_watches, w.clone()); self.paths.remove(&w); if is_recursive || remove_recursive { let mut remove_list = Vec::new(); for (w, p) in &self.paths { if p.starts_with(&path) { - inotify_watches - .remove(w.clone()) - .map_err(|e| Error::io(e).add_path(p.into()))?; + Self::remove_single_descriptor(&mut inotify_watches, w.clone()); self.watches.remove(p); remove_list.push(w.clone()); } @@ -504,6 +502,34 @@ impl EventLoop { Ok(()) } + /// As long as we use the `inotify` crate its behaviour is specified by the documentation of + /// a [`inotify::Watches::remove`] method: + /// ```text + /// Directly returns the error from the call to [inotify_rm_watch]. + /// Returns an [io::Error] with [ErrorKind]::InvalidInput, + /// if the given WatchDescriptor did not originate from this [Inotify] instance. + /// ``` + /// + /// inotify documentation says, that `inotify_rm_watch` may fail with two specific errors: + /// * EBADF - fd is not a valid file descriptor. + /// * EINVAL - The watch descriptor wd is not valid or fd is not an inotify file descriptor. + /// + /// Therefore, we can ignore this errors (and log it), because + /// * in the case, when we are removing a watch because of an caught `DELETE` or `DELETE_SELF` event we want the + /// path to be not watched, and in error cases it's already done (unknown file descriptor == it is not watched) + /// * in the case, when user is trying to remove the watch, they can do nothing with that kind of an error, + /// it's totally internal. BUT, if there are no "strange" states (like races between user call and internal call, + /// when internal inotify file descriptor has already been invalidated, but the event still hasn't been handled) + /// they will get an [`ErrorKind::WatchNotFound`] error and can deal with it + /// + /// Log level is info, because it is not a "real" error. Expectedly, it may occurred only by race condition + /// (like described above), in other cases it is a bug (but we aren't able to distinguish that states) + fn remove_single_descriptor(watches: &mut inotify::Watches, wd: WatchDescriptor) { + if let Err(err) = watches.remove(wd) { + log::info!("unable to remove watch descriptor from inotify: {err:?}"); + } + } + fn remove_all_watches(&mut self) -> Result<()> { if let Some(ref mut inotify) = self.inotify { let mut inotify_watches = inotify.watches(); @@ -762,4 +788,27 @@ mod tests { &events[..LOG_LEN.min(events_len)] ); } + + /// https://github.com/notify-rs/notify/issues/709 + #[test] + fn remove_a_subdir_in_a_recursively_watched_parent() { + let tmpdir = tempfile::tempdir().expect("tmpdir"); + let subdirectory_path_1 = tmpdir.path().join("subdir"); + let subdirectory_path_2 = subdirectory_path_1.join("nested"); + std::fs::create_dir(&subdirectory_path_1).expect("unable to create a subdir"); + std::fs::create_dir(&subdirectory_path_2).expect("unable to create a nested dir"); + + let mut watcher = + INotifyWatcher::new(|_| (), Config::default()).expect("unable to create watcher"); + watcher + .watch(tmpdir.path(), RecursiveMode::Recursive) + .expect("unable to watch"); + std::fs::remove_dir_all(&subdirectory_path_1).expect("unable to remove a subdir"); + let unwatch_result = watcher.unwatch(tmpdir.path()); + + assert!( + matches!(unwatch_result, Ok(())), + "error: {unwatch_result:#?}" + ); + } }