notify icon indicating copy to clipboard operation
notify copied to clipboard

Non-recursive `KqueueWatcher` reports wrong event paths

Open gabi-250 opened this issue 1 year ago • 2 comments

System details

  • OS/Platform name and version: FreeBSD freebsd 14.1-RELEASE FreeBSD 14.1-RELEASE releng/14.1-n267679-10e31f0946d8 GENERIC amd64 (running under QEMU)
  • Rust version (if building from source): rustc --version: rustc 1.81.0 (eeb90cda1 2024-09-04)
  • Notify version (or commit hash if building from git): 6.1.1

Note: the issue doesn't seem to be FreeBSD-specific (as far as I can tell, it happens on all platforms that use kqueue).

  • If you're coming from a project that makes use of Notify, what it is, and a link to the downstream issue if there is one: in Arti, we're using notify to watch configuration files for changes. The bug described here is causing one of our tests to stall: https://gitlab.torproject.org/tpo/core/arti/-/issues/1644

What you did (as detailed as you can)

I set up a non-recursive watcher for a directory that contains a single file foo. I created another file bar in the directory, expecting to receive an Event associated with the path of bar, but instead got an event for foo.

use notify::{Config, RecommendedWatcher, RecursiveMode, Watcher};
use std::path::PathBuf;
use std::sync::mpsc;
use temp_dir::TempDir;

fn write_file(dir: &TempDir, name: &str, data: &[u8]) -> PathBuf {
    let path = dir.path().join(name);
    std::fs::write(&path, data).unwrap();
    path
}

fn main() {
    let temp_dir = TempDir::new().unwrap();

    // Prepopulate temp_dir with a file.
    let _: PathBuf = write_file(&temp_dir, "foo", b"hello");

    let (tx, rx) = mpsc::channel();
    let mut watcher =
        RecommendedWatcher::new(move |event| tx.send(event).unwrap(), Config::default()).unwrap();

    println!("watching {}", temp_dir.path().display());
    watcher
        .watch(temp_dir.path(), RecursiveMode::NonRecursive)
        .unwrap();

    // We haven't triggered any events since starting the watcher
    assert!(rx.try_recv().is_err());

    // Write a file to the watched directory.
    // This should trigger an event where one of the paths is
    // "<temp_dir>/bar".
    let expected_path = write_file(&temp_dir, "bar", b"good-bye");

    let ev = rx.recv().unwrap().unwrap();
    println!("{:#?}", ev);

    assert!(
        ev.paths.contains(&expected_path),
        "expected event for {}, but got event for {:?}?!",
        expected_path.display(),
        ev.paths,
    );

    // Note: as expected, writing the second file triggers a single event:
    //   * with the inotify backend, the event is associated with the newly created file "bar",
    //     as expected
    //   * with the kqueue backend, the event is wrongly associated with file "foo"
}

with Cargo.toml:

[package]
name = "notify-repro"
version = "0.1.0"
edition = "2021"

[dependencies]
notify = "6.1.1"
temp-dir = "0.1.14"

The assertion fails on platforms using the kqueue backend. With the inotify backend, it works as expected.

What you expected

watching /tmp/tb59-0
Event {
    kind: Create(
        File,
    ),
    paths: [
        "/tmp/tb59-0/bar",
    ],
    attr:tracker: None,
    attr:flag: None,
    attr:info: None,
    attr:source: None,
}

What happened

The assertion failed

watching /tmp/tb59-0
Event {
    kind: Create(
        File,
    ),
    paths: [
        "/tmp/tb59-0/foo",
    ],
    attr:tracker: None,
    attr:flag: None,
    attr:info: None,
    attr:source: None,
}
thread main panicked at src/main.rs:38:5:
expected event for /tmp/tb59-0/bar, but got event for ["/tmp/tb59-0/foo"]?!
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I think the bug is in handle_kqueue's handling of kqueue::Vnode::Write:

  • it assumes that all the files from the watched directory are already in self.watches (which isn't the case for non-recursive watchers)
  • it then looks for the first entry from std::fs::read_dir(&path) that is not in self.watches. However, if the watcher is non-recursive, this entry won't necessarily be the one that triggered the kqueue::Vnode::Write event

I can try to come up with a patch to fix this, if you'd like.

gabi-250 avatar Oct 09 '24 14:10 gabi-250

I'm also observing this problem with RecursiveMode::Recursive – sometimes, the kqueue backends emits events for unchanged files. In my case, it might also have something to do with symbolic links, so that even if I disable symbolic links via the new flag shipped on notify 8.0.0, I get emissions for events for files that are located at the symbolic link target.

squidfunk avatar Jan 26 '25 08:01 squidfunk

Hmm, I see this occurring still on HEAD, but it's inconsistent. Some runs get an event for bar, some runs get an event for foo.


From a kqueue perspective, this is not possible. The underlying kevent syscall does not return this information in any way, so there's not much that can be done. Perhaps the best way forward is instead dropping the behavior of trying to figure out what file was created when non-recursive mode is used with the kqueue backend? It cannot return accurate information, so perhaps it's better to return nothing.

worr avatar May 06 '25 18:05 worr