Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-recursive KqueueWatcher reports wrong event paths #644

Open
gabi-250 opened this issue Oct 9, 2024 · 0 comments
Open

Non-recursive KqueueWatcher reports wrong event paths #644

gabi-250 opened this issue Oct 9, 2024 · 0 comments

Comments

@gabi-250
Copy link

gabi-250 commented Oct 9, 2024

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).

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.

zydou pushed a commit to zydou/arti that referenced this issue Oct 17, 2024
… have inotify.

On windows and platforms that support inotify (i.e. linux and android),
we continue using the recommended watcher. On platforms that use kqueue,
we switch to a polling watcher to work around a [notify bug] that
manifests when using a non-recursive watcher to watch a directory.

This commit is best reviewed with `git diff --ignore-all-space`.

Closes #1644

[notify bug]: notify-rs/notify#644
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant