Merge upstream notify 9.0.0-rc.5 - #13
Open
cole-miller wants to merge 159 commits into
Open
cole-miller wants to merge 159 commits into
cole-miller wants to merge 159 commits into
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…rs#816) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…rs#817) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…rs#818) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…rs#821) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…rs#822) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…rs#826) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…rs#828) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#829) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#830) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#831) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#833) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#836) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#838) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#839) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-rs#840) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* feat: get watched paths * Add changelog entry
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…#957) Kqueue queued watches for newly discovered entries but invoked the event handler before applying them. If a handler reacted to a directory-create event by modifying that directory, the change could happen before its recursive watch was installed and be lost permanently. Buffer events until the queued watch removals and additions have been applied. Delivering a directory-create event now guarantees that recursive coverage for the new directory is already active. Add a regression test whose handler creates a file in the newly reported directory and verifies that the resulting event is observed. Signed-off-by: Daan De Meyer <daan@amutable.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
- detect zero-byte and enumeration-required completions - rearm the watch before emitting a pathless rescan event - preserve existing record parsing and unrelated error handling - add focused Windows regression coverage and a changelog entry
* feat(notify): use inotify as the FreeBSD backend FreeBSD 15.0+ provides in-kernel inotify(2) and libc wrappers. Enable the freebsd_inotify feature by default so INotifyWatcher is RecommendedWatcher on FreeBSD (same as Linux/Android), keep kqueue available, and bump inotify to 0.11.4. FreeBSD 14.x has no inotify in base: build with default-features = false to use kqueue. Native linkage is handled in inotify-sys 0.1.8. Test adjustments for FreeBSD event semantics: - set_file_mtime accepts any Modify (Linux: IN_MODIFY, FreeBSD: IN_ATTRIB) - hardlink write-through test is Linux-only (dirent- vs inode-centric) CI: FreeBSD 15.1 with defaults; FreeBSD 14.4 with --no-default-features. Document the platform split in README/changelog. * feat(freebsd): default to kqueue --------- Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
Linux removes watch descriptors automatically when a watched inode is deleted and emits DELETE_SELF and IGNORED. The event loop previously queued the same path multiple times, so cleanup called inotify_rm_watch on an invalid descriptor and then reported WatchNotFound for the duplicate removal. Coalesce pending removals by path and record whether the kernel already removed the root descriptor. Process removals deepest-first, handle IGNORED even when remove events are filtered, and remove potentially live descendant descriptors separately. Treat expected EINVAL deletion races as trace output while warning on unexpected failures. Add regression coverage for IGNORED-only cleanup, duplicate removal coalescing, and moved-out descendant descriptor cleanup. Signed-off-by: Daan De Meyer <daan@amutable.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…tify-rs#971) * fsevent: refuse to create a stream that would corrupt the process FSEvents closes a file descriptor it does not own once a stream carries more than roughly RLIMIT_NOFILE/10 paths. The descriptor is fd 0, which then becomes the lowest free descriptor, so the next open anywhere in the process lands on it and the following oversized stream closes that instead. The damage surfaces far from FSEvents, as EBADF on unrelated files, which is why it is so hard to trace back: it was originally found as intermittent EBADF from closedir inside a remove_dir_all that had nothing to do with watching. Reproduced on macOS 14, 15 and 26 with a standalone C program that uses nothing but CoreServices, so this is not something notify is doing wrong. It is deterministic rather than racy, and the threshold scales with the descriptor limit: with the common default of 256 it is around 27 paths. The reproducer is at https://github.com/daandemeyer/fsevents-fd-repro. Compute the budget from getrlimit and return MaxFilesWatch rather than making the call. Refusing to watch is worse for the caller than watching would have been, but far better than silently corrupting the descriptor table of the host process, and unlike a corrupted descriptor table it is something they can respond to. The error message points at the three things that actually help: raising RLIMIT_NOFILE, watching fewer paths, or the kqueue backend. The budget uses a twelfth rather than a tenth. The exact edge shifts with how many descriptors the process already holds, so a little headroom avoids sitting on it. This subsumes error_properly_on_stream_start_failure, which watched 4097 paths to provoke an FSEventStreamStart failure. Those 4097 paths are what closes fd 0, so the test was corrupting the process it ran in: it carried a catch_unwind around its own temporary directory cleanup to survive the resulting EBADF, and on CI it sometimes aborted the whole test binary instead, which catch_unwind cannot intercept. The guard now rejects that call before FSEvents sees it, so replace the test with one that derives an over-budget path count from the current rlimit and asserts MaxFilesWatch. No stream is created, so the cleanup workaround goes away with it. There is no longer a way to reach a genuine start failure from a test without deliberately re-entering the range that corrupts the process, so that path is left uncovered on purpose. Signed-off-by: Daan De Meyer <daan@amutable.com> * fsevent: account for paths across live streams --------- Signed-off-by: Daan De Meyer <daan@amutable.com> Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* fsevent: coalesce watches nested inside other watches An FSEvents stream root reports the entire hierarchy below it, and `callback_impl` attributes every event to the longest watch it falls under, so passing both a directory and something below it to `FSEventStreamCreate` adds nothing while costing a slot against the path limit that macOS enforces by corrupting the process descriptor table above roughly RLIMIT_NOFILE/10 paths. Derive the CFArray handed to `FSEventStreamCreate` from the watch map in `run()` instead of maintaining it alongside, dropping any watch whose ancestors include another watch on the same volume. FSEvents does not promise that a stream root reports events from a volume mounted below it, so a covering ancestor across a mount point does not count. Unwatching a covering directory promotes the nested watches back to stream roots on the next run with no separate bookkeeping, and `remove_cf_path` goes away. One behavioural difference: with `kFSEventStreamCreateFlagWatchRoot`, a nested watch no longer receives a root-changed event of its own when an ancestor is renamed. A recursive covering watch still reports the rename; a non-recursive one reports nothing, since `callback_impl` accepts only direct children for those. Covered by unit tests for the stream roots produced by nested and by sibling watches, and by black-box tests that a nested watch still delivers events after its covering watch is removed and that a covering watch still delivers events outside the nested one. Signed-off-by: Daan De Meyer <daan@amutable.com> * fsevent: only coalesce watches under recursive ancestors --------- Signed-off-by: Daan De Meyer <daan@amutable.com> Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
* refactor(inotify): keep add-time resolution flags out of the stored mask `Watch.watch_mask` is merged into later adds for the same path, so flags that control path resolution or mask merging must not be stored there. Split the add-time mask from the stored event mask and assert the invariant. * feat(inotify): add per-path control over dereferencing symlinks `Watcher::watch_with` passes a `WatchPathConfig` through to the backend, and `with_dereference_symlinks(false)` places a non-recursive watch on the link rather than its destination, which also makes a dangling link watchable. A recursive watch resolves the path in order to walk it, and a walk never overrules the choice an explicit watch made for its own path. * test(inotify): port merged tests to the WatchPathConfig signature * fix some cases that `dereference_symlinks` do not work well --------- Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
PathData::mtime has whole-second resolution and is compared before the content hash, so the kind of event a change produces depends on whether it crossed a wall-clock second. Six poll tests asserted one of the two outcomes and failed about one run in ten under the load of the full suite. Mutating a watched directory bumps its own write time, so create_file, create_dir, remove_file and rename_file can get an extra Modify(Metadata(WriteTime)) for the directory; accept it as optional. modify_file and create_write_overwrite leave an entry with both new contents and a new write time, reported as Data within a second and as Metadata(WriteTime) across one. Set that entry's write time an hour ahead before the baseline scan and the comparison always falls through to the hash. Restoring the old write time after the change is not enough: on macOS the scan can still stat the write time of the change. poll-watcher-hashing.rs did restore it, but polled every 10ms, so a scan could land between the write and the restore. Poll it manually instead and skip the directory's own event. Trailing optional expectations also needed a harness fix, since is_empty counted a queue holding only optionals as non-empty and the waiter blocked until it timed out. Sleeping a second before each mutation fails all six tests before this change and passes after. Signed-off-by: Daan De Meyer <daan@amutable.com> Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Just refactoring, no behavior change. Signed-off-by: Daan De Meyer <daan@amutable.com>
…otify-rs#980) Co-authored-by: 翠 <green@sapphi.red>
* Fix PollWatcher subsecond mtime detection * Document PollWatcher mtime fix --------- Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
…rs#987) Co-authored-by: 翠 <green@sapphi.red>
Native inotify ships in FreeBSD 14.5, so auto-select by major.minor instead of treating only 15.0+ as supported.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Resyncs `main` with notify-rs/notify at the 9.0.0-rc.5 tag (853729a). The fork's previous upstream sync (#7) was squash-merged, so git's merge base was the February state and nearly every file reported a conflict even though the fork's content matched upstream's June 8 state exactly. Every conflicted file except `notify/src/fsevent.rs` was verified to be byte-identical between the fork's `main` and upstream at that June state and resolved by taking rc.5. `fsevent.rs` was resolved with a three-way merge against the fork's real base (upstream 9e61716), which reapplies the fork's patches on top of upstream's changes: - #7: stop the run loop through a `CFRunLoopSource` rather than an atomic flag, avoiding a shutdown race. - #9: keep the old stream running until the replacement has started, and keep it if the replacement fails to start. - #11: do not set `kFSEventStreamCreateFlagWatchRoot`. Notable upstream changes picked up: `fsevent: refuse to create a stream that would corrupt the process (notify-rs#971)`, `fsevent: coalesce watches nested inside other watches (notify-rs#975)`, `windows: handle watch shutdown races (notify-rs#958)`, `inotify: coalesce kernel-driven watch cleanup (notify-rs#960)`, `fix(windows): emit rescan events for lost change details (notify-rs#964)`.
The budget from notify-rs#971 guards against FSEvents closing fd 0 once a stream carries more than ~RLIMIT_NOFILE/10 paths. That happens because `kFSEventStreamCreateFlagWatchRoot` makes the stream hold a descriptor on every ancestor of every path; without the flag a stream holds no descriptors per path and the corruption does not occur (verified with the create-then-release rebuild pattern up to 1500 paths at RLIMIT_NOFILE=2560, where WatchRoot corrupts at ~136). This fork does not set the flag (#11), so the budget would only impose an artificial ceiling: the reservation is held by every live stream, and the fork keeps the old stream running until the replacement has started (#9), so a rebuild would fail once 2N paths exceed soft/12, i.e. at about 100 paths for a GUI process. The over-budget test sets the flag explicitly so it still exercises the guard.
Brings in #6 (typed FsEventStreamStart error). Its assertion on the removed 4097-path test is dropped in favour of upstream's budget test, which covers the same failure path without corrupting the test process.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resyncs
mainwith notify-rs/notify at the 9.0.0-rc.5 tag (853729a) as a real merge, so the fork's ancestry includes upstream's history again and future syncs are plain merges (upstream/mainpost-rc.5 already merges cleanly on top of this).What's in it
980b41e: the previous sync (fix(fsevent): stop the run loop via a CFRunLoopSource to avoid a shutdown race #7) was squash-merged, so git's merge base was February and nearly every file conflicted despite the fork's content being byte-identical to upstream's June 8 state. Every conflicted file exceptfsevent.rswas verified identical to that June state and resolved by taking rc.5.fsevent.rswas merged three-way against the fork's real base (upstream9e61716), reapplying fix(fsevent): stop the run loop via a CFRunLoopSource to avoid a shutdown race #7 (CFRunLoopSourceshutdown), fsevents: Keep a stream running across watch/unwatch rebuilds #9 (keep the old stream running until the replacement starts), and fsevents: Stop setting kFSEventStreamCreateFlagWatchRoot #11 (noWatchRoot).a76576b— only apply the fsevent: refuse to create a stream that would corrupt the process notify-rs/notify#971 path budget whenWatchRootis set. Upstream's budget guards against FSEvents closing fd 0 above ~RLIMIT_NOFILE/10paths, but that only happens withWatchRoot(verified: without it, 1500 paths, 3 fds total, fd 0 intact; with it, corruption at ~136). Since the reservation is held by every live stream and fsevents: Keep a stream running across watch/unwatch rebuilds #9 overlaps old and new streams during a rebuild, the budget would otherwise cap this fork at ~100 paths for a GUI process — an artificial limit with nothing behind it. The over-budget test sets the flag so it still exercises the guard.main(AddFsEventStreamStarterror variant #6, typedFsEventStreamStart). Its assertion on the removed 4097-path test is dropped in favour of upstream's budget test.Notable upstream changes picked up
fsevent: refuse to create a stream that would corrupt the process (#971)fsevent: coalesce watches nested inside other watches (#975)— nested paths no longer occupy stream slotswindows: handle watch shutdown races (#958)— thedelete_self_dirflake (supersedes windows: Handle watch shutdown races (cherry-pick upstream #958) #12)inotify: coalesce kernel-driven watch cleanup (#960)fix(windows): emit rescan events for lost change details (#964)PollWatcher: detect subsecond mtime changes (#981),feat: update to edition 2024 (#990)Result
git diff notify-9.0.0-rc.5..HEADtouches three files:fsevent.rs(the fork's patches),error.rsandnotify-debouncer-full/src/testing.rs(typed error from #6).Tested:
cargo test --workspaceon macOS (78 fsevent unit tests + integration + doctests);cargo check -p notifyforx86_64-pc-windows-msvcandx86_64-unknown-linux-gnu.Closes #12 (superseded by upstream notify-rs#958 arriving here).