diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a42c37..624632c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,7 +49,7 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Install dependencies - run: sudo apt-get update && sudo apt-get install -y ninja-build libfuse3-dev fuse3 + run: sudo apt-get update && sudo apt-get install -y ninja-build libfuse3-dev fuse3 inotify-tools - name: Build run: | cmake --preset linux-release @@ -59,6 +59,16 @@ jobs: git config --global user.email ci@example.com git config --global user.name CI bash scripts/fs-conformance.sh + # The other half of "the mount behaves like a filesystem": a change made on + # the far side has to raise a real inotify event here, or every watch-mode + # tool silently does nothing. Which kernel hook each operation runs is not + # observable from a unit test, so it can only be checked on a live mount. + # always(): the two batteries cover different things, and when the + # filesystem one fails the change-notification verdict is exactly what + # says whether the two failures share a cause. + - name: Run the change-notification battery + if: always() + run: bash scripts/inotify-conformance.sh # The same idea for Direction A - the feature the product is named for. Mounts # a real drive letter through WinFsp on the Windows runner and runs the Windows diff --git a/CHANGELOG.md b/CHANGELOG.md index 35db6d4..1c89311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ ## Unreleased +### Added + +- **File changes made on the Windows side now fire `inotify` inside WSL.** A Direction B mount + delivers far-side changes to ordinary Linux watchers, so `vite`, `nodemon`, `tsc --watch`, + `jest --watch`, `air` and `cargo-watch` react to an edit made from a Windows editor. Watchers need + no cooperation of any kind. Create, write, delete and rename each raise the matching event type, + and a rename arrives as a paired `IN_MOVED_FROM`/`IN_MOVED_TO` with one cookie rather than as an + unrelated delete and create. This is [microsoft/WSL#4739](https://github.com/microsoft/WSL/issues/4739), + which nothing has solved. Measured against a write made locally on the mount, it adds nothing: + **4 ms either way**, including a write deep inside a 10,000-file tree, and read/write throughput is + untouched because the bridge sits on the invalidation path rather than the data path. On by + default; `wsldrive mount --no-inotify` turns it off. How it works, and what it does not cover, is + in [`docs/inotify.md`](docs/inotify.md). +- `mknod` on a regular file now works on the mount, instead of failing with `ENOSYS`. + +### Changed + +- The wire protocol is at version 4. Every invalidation op carries what the watcher actually saw + (created, modified, removed, or one half of a move) alongside what the mirror should do about it, + plus a cookie pairing the two halves of a rename. A peer that ignores both fields stays correct. + The agent and the client must be the same version, as before. + ### Performance - **The mount lets the kernel cache file pages.** `auto_cache` replaces diff --git a/README.md b/README.md index 3edd39e..23b9291 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,8 @@ driver of its own (WinFsp on Windows, libfuse3 in WSL). Full numbers in [`bench/ Desktop, Explorer/Search, Unity, Office) reach a WSL source tree without paying the `\\wsl.localhost` Plan 9 tax. - **Direction B — a Windows drive mounted inside WSL2.** Linux tools read/write an NTFS tree without the - `/mnt/c` 9P/virtiofs tax. + `/mnt/c` 9P/virtiofs tax — and a Windows-side edit fires `inotify`, so + [watch mode works](#watch-mode-works-inotify-across-the-boundary). WSL2 only. WSL1 has no VM boundary (DrvFs runs in the NT kernel, `\\wsl$` is served in-process), so it has neither problem and is unsupported by design. @@ -120,7 +121,8 @@ Two small user-space binaries, no kernel drivers of wsldrive's own: it for changes (`ReadDirectoryChangesW`+IOCP on Windows, inotify on Linux), pushing coalesced invalidations. - **`wsldrive`** — the *client*: mounts the served tree as a filesystem (WinFsp on Windows, libfuse3 in - WSL, one FUSE3 implementation) backed by an in-RAM metadata mirror and a content cache. + WSL, one FUSE3 implementation) backed by an in-RAM metadata mirror and a content cache. In WSL it + also turns each invalidation into a real `inotify` event, so watch-mode tools see far-side changes. Either binary can host either role, so the same code serves both directions — the direction is just which side runs the agent and which runs the mount. @@ -188,6 +190,25 @@ explicit flag always wins over the probe. A `.wsldriveignore` at the served root (gitignore-style: `node_modules/`, `*.log`, `/build`, …) excludes paths from the mount and from sync. +### Watch mode works (`inotify` across the boundary) + +Edit a file from a Windows editor and `vite`, `nodemon`, `tsc --watch`, `jest --watch`, `air` and +`cargo-watch` reload — on a Direction B mount, with no plugin, no preload and no polling. + +They do not, on `/mnt/c` or anywhere else. A change made by a Windows application raises no `inotify` +event inside WSL2, so a Linux watcher never fires. Nothing errors; the tooling just quietly stops +reacting, which is why people lose an afternoon to it before finding +[microsoft/WSL#4739](https://github.com/microsoft/WSL/issues/4739). + +wsldrive already carries every far-side change across as an invalidation, so the mount knows what +happened within milliseconds. It turns each one into a real kernel event by replaying the operation +on the mount itself — a write becomes an mtime-only `utimensat` (`IN_MODIFY`), a creation a `mknodat` +(`IN_CREATE`), a rename a `renameat`, so the two halves arrive paired under one cookie rather than as +an unrelated delete and create. The events are the kernel's own, so any watcher sees them. + +On by default. `wsldrive mount --no-inotify` turns it off. Mechanism, guarantees and the two things +it does not cover: [`docs/inotify.md`](docs/inotify.md). + ### What gets served (and what doesn't) The default root is your **home directory** (`~`) — that is where the work is, and it keeps the tree @@ -323,7 +344,8 @@ detected (Linux) — so CI and minimal builds are unaffected. `src/core` — platform-independent library (metadata tree, string pool, framed protocol, coalescer, auth token, ignore rules, name escaping, path utils), all unit-tested. `src/net` — sockets (TCP/vsock/hvsocket) and the framed channel. `src/platform` — watchers and process launchers per OS. -`src/agent` — the scanner, `RootServer`, and the `RemoteRoot` client. `src/mount` — the FUSE3 mount. +`src/agent` — the scanner, `RootServer`, and the `RemoteRoot` client. `src/mount` — the FUSE3 mount +and the `inotify` bridge. `src/tools` — the `wsldrive` and `wsldrived` binaries. `tests`, `bench`, `scripts` as named. ## Security diff --git a/docs/inotify.md b/docs/inotify.md new file mode 100644 index 0000000..20a6992 --- /dev/null +++ b/docs/inotify.md @@ -0,0 +1,127 @@ +# Change notification across the boundary + +A file changed by a Windows application does not fire `inotify` inside WSL2. Nothing errors. +`vite`, `webpack --watch`, `nodemon`, `jest --watch`, `tsc --watch`, `air` and `cargo-watch` all +just sit there. It is [microsoft/WSL#4739](https://github.com/microsoft/WSL/issues/4739), and it is +the reason people who keep their source on the Windows side end up polling. + +A wsldrive Direction B mount delivers those events. Editing a file from a Windows editor raises a +real `IN_MODIFY` on the mount, a new file raises `IN_CREATE` in its directory, a rename raises a +paired `IN_MOVED_FROM`/`IN_MOVED_TO`, and a deletion raises `IN_DELETE`. Watchers need no +cooperation: no preload, no plugin, no knowledge that wsldrive is involved. + +It is on by default. `wsldrive mount --no-inotify` turns it off. + +## Why it needs a mechanism at all + +The kernel raises fsnotify events from the VFS, at the point an operation is performed. +`vfs_create` calls `fsnotify_create`, `vfs_unlink` calls `fsnotify_unlink`, `notify_change` calls +`fsnotify_change`. A filesystem cannot raise one itself, and FUSE offers nothing that does: the +`fuse_lowlevel_notify_*` calls invalidate dentries and pages, which is cache coherence, not +notification. `fuse_reverse_inval_entry` never touches fsnotify. So a userspace daemon has no way to +hand an inotify watcher an event, and this is why the problem has outlived so many attempts at it. + +## What wsldrive does + +It asks the kernel to raise the event, by performing on the mount the operation the far side already +performed. A thread inside the mount process replays each change as an ordinary syscall against the +mount's own path: + +| far side | wsldrive replays | kernel raises | +|---|---|---| +| file written | `utimensat`, mtime only | `IN_MODIFY` | +| file created | `mknodat` (regular file) | `IN_CREATE` | +| directory created | `mkdirat` | `IN_CREATE` with `IN_ISDIR` | +| file deleted | `unlinkat` | `IN_DELETE` | +| directory deleted | `unlinkat(AT_REMOVEDIR)` | `IN_DELETE` with `IN_ISDIR` | +| renamed | `renameat` | `IN_MOVED_FROM` + `IN_MOVED_TO`, one cookie | + +The `utimensat` detail matters. `fsnotify_change()` reports a change to *both* timestamps as +`FS_ATTRIB` and a change to mtime alone as `FS_MODIFY`, so touching only mtime is what turns a +far-side write into `IN_MODIFY` rather than the weaker `IN_ATTRIB`. And letting the kernel perform +the rename is what produces a genuine move cookie, so a watcher sees one move instead of an +unrelated delete and create. + +These operations must not cross the boundary a second time — the far side already has this state, +and re-applying it would at best waste a round trip and at worst destroy the file that prompted the +event. So the FUSE handlers recognise the bridge's own requests and answer them locally. That test +is deliberately over-determined, because the cost of getting it wrong is a user's `rm` silently not +removing anything: the request must come from the bridge's own thread, a poke of exactly that kind +must be in flight, and it must name exactly that path. A poke is claimed once and then retired. + +The recognition has to cover every handler the poke can reach, which is not always the obvious one. +A `mknodat` of a regular file arrives as `FUSE_MKNOD`, but libfuse offers it to the `create` handler +first and only falls back to `mknod` if that answers `ENOSYS` — so a mount with a `create` handler +never sees the poke at `mknod` at all. Getting that wrong is not a missed event; it forwards the +poke as a genuine creation and replaces the file that prompted it with an empty one. + +Which is why an unrecognised request from the bridge's thread is refused rather than forwarded. The +bridge performs no genuine mutations, so a mutation from its thread is one of its own replays and +nothing else. A refused poke costs a notification; a forwarded one costs data. + +Two further guards keep the replay inside the mount. The bridge refuses to start unless its root is +a FUSE mount (`statfs` reports `FUSE_SUPER_MAGIC`), and every poke re-checks that its target's +parent is still on that mount's device. Both exist because the pokes are real filesystem calls: aimed +at the wrong tree they would create and delete real files there. + +## Two small fictions + +Both are visible only to the bridge's own thread; every other caller gets the mirror's real answer. + +The invalidation is applied to the metadata mirror before the poke runs, so the mirror is already +telling the truth by the time the kernel looks — which is exactly wrong for making the kernel run +the operation. A creation would find the path already there and never reach `->mknod`; a deletion +would find nothing and never reach `->unlink`. So for the length of one syscall the mount reports a +newly created path as absent, and a newly deleted one as still present. Neither fiction outlives +its call: the create or unlink that follows settles the dentry, and the record is retired the moment +the handler claims it. + +## What it costs + +One syscall per changed path, answered by the FUSE loop out of the in-RAM mirror. No boundary +crossing and no I/O, so the added latency over a write made locally on the mount is the mount's own +round trip. `scripts/inotify-conformance.sh` measures both and prints them side by side; on a GitHub +Actions runner, with a Linux agent on loopback: + +| change to event | | +|---|--:| +| write made locally on the mount | 4 ms | +| write made on the served side | 4 ms | + +Each individual event type lands in the same 4 ms, including a write deep inside a 10,000-file tree. +Nothing measurable is added, which follows from where the work happens: the invalidation had already +crossed the boundary and updated the mirror before the bridge did anything, and the poke never +leaves the local kernel. + +Read and write throughput are untouched. The bridge sits on the invalidation path, not the data +path, and adds no work to `read`, `write` or any other operation a tool performs on the mount. + +The queue is bounded at 65536 pending changes. Past that the oldest are dropped, and `wsldrive mount` +says so on exit. A dropped notification is a late one, not a wrong one: the mirror and the page cache +are already correct, so the file reads right the moment anything looks at it. + +## Limits + +- **Direction B only.** Direction A does not have this problem: a WinFsp volume raises Windows change + notifications through its own mechanism. +- **An overflow cannot name paths.** When the far side's watcher loses events it sends a rescan, the + mirror is rebuilt from a fresh snapshot, and the bridge touches the mount root. A watcher that + re-walks on any event below its root will catch up; one waiting for a specific path will not be + told about it. Making an overflow name what changed means diffing the old tree against the new + snapshot, which is not implemented. In practice the Windows watcher's 1 MiB buffer keeps overflows + rare outside of bursts on the scale of an `npm install`. +- **`inotify` only.** `fanotify` marks are not delivered, and neither are the `IN_OPEN`, `IN_ACCESS` + or `IN_CLOSE` classes of event — nothing on the far side reports those, so there is nothing to + replay. +- **The mount's own watch limits apply.** `max_user_watches` bounds a recursive watcher on the mount + exactly as it would on any local filesystem; wsldrive neither raises nor consumes that budget. + +## Checking it + +`scripts/inotify-conformance.sh` mounts a tree, changes it from the serving side, and asserts that +each change raises the right event type on the mount within a timeout. It also checks the thing that +would be worst to get wrong — that the served tree is untouched by the bridge's own operations — and +exercises a recursive watch over a 10,000-file tree. CI runs it on every change. + +To watch the event stream by hand without a mount, `wsldrive fetch --connect --watch` +prints each invalidation with its change kind and rename cookie. diff --git a/scripts/inotify-conformance.sh b/scripts/inotify-conformance.sh new file mode 100644 index 0000000..237a2c1 --- /dev/null +++ b/scripts/inotify-conformance.sh @@ -0,0 +1,232 @@ +#!/usr/bin/env bash +# Change-notification conformance check for a wsldrive mount. +# +# The claim under test is the one from microsoft/WSL#4739: a file changed on the +# *far* side of the boundary fires a real inotify event on this side, so an +# unmodified watch-mode tool reacts to it. Nothing about that can be checked +# from a unit test — it depends on which kernel hook each operation runs, which +# only happens against a live mount. +# +# The far side here is a Linux agent rather than a Windows one, because CI has +# no Windows peer. That is the same code path: the agent watches its tree, the +# client applies the invalidation, and the bridge turns it into a local event. +# What a Windows agent changes is which platform watcher produces the event, and +# that part is covered by the unit tests. +# +# scripts/inotify-conformance.sh [build-dir] +# +# Exit status is the number of failed checks (0 = all good). + +set -u +BUILD=${1:-build/linux-release} +AGENT="$BUILD/src/tools/wsldrived" +CLI="$BUILD/src/tools/wsldrive" +PORT=${PORT:-51998} +# How long a notification may take to arrive. Generous: the agent coalesces for +# up to 25 ms, and a loaded CI runner adds more. The measured figure is reported +# separately at the end. +TIMEOUT=${TIMEOUT:-15} + +command -v inotifywait >/dev/null || { echo "inotifywait not found (apt install inotify-tools)"; exit 99; } +for exe in "$AGENT" "$CLI"; do + [ -x "$exe" ] || { echo "not built: $exe"; exit 99; } +done + +WORK=$(mktemp -d) # the served tree: stands in for the Windows side +MNT=$(mktemp -d) # the mount: stands in for the Linux side +LOG=$(mktemp -d) +export WSLDRIVE_TOKEN="inotify-conformance-$$" +cleanup() { + [ -n "${WATCH_PID:-}" ] && kill "$WATCH_PID" 2>/dev/null + fusermount3 -u "$MNT" 2>/dev/null + [ -n "${CLI_PID:-}" ] && kill "$CLI_PID" 2>/dev/null + [ -n "${AGENT_PID:-}" ] && kill "$AGENT_PID" 2>/dev/null + rm -rf "$WORK" "$MNT" "$LOG" 2>/dev/null + return 0 +} +trap cleanup EXIT + +mkdir -p "$WORK/src" "$WORK/keep" +echo "original" > "$WORK/src/app.js" +echo "static" > "$WORK/keep/untouched.txt" + +echo "serving $WORK -> $MNT" +"$AGENT" --root "$WORK" --listen "tcp://127.0.0.1:$PORT" >"$LOG/agent.log" 2>&1 & +AGENT_PID=$! +sleep 2 +"$CLI" mount "$MNT" --connect "tcp://127.0.0.1:$PORT" >"$LOG/mount.log" 2>&1 & +CLI_PID=$! +for _ in $(seq 1 60); do sleep 1; mountpoint -q "$MNT" && break; done +mountpoint -q "$MNT" || { echo "MOUNT FAILED"; tail -20 "$LOG/mount.log"; exit 98; } +grep -q "inotify watchers" "$LOG/mount.log" || echo " note: the mount did not report the bridge as enabled" + +PASS=0; FAIL=0 +ok() { PASS=$((PASS+1)); printf ' ok %s\n' "$1"; } +bad() { FAIL=$((FAIL+1)); printf ' FAIL %s\n' "$1"; } +# awk rather than bc: bc is not installed on a stock CI runner. +elapsed_ms() { awk -v a="$2" -v b="$1" 'BEGIN { printf "%.0f", (a - b) * 1000 }'; } + +# One watcher for the whole run, recursive over the mount, writing every event +# to a file. A fresh inotifywait per check would race with the change it is +# meant to see; this way the log is always already listening. +EVENTS="$LOG/events" +: > "$EVENTS" +inotifywait -m -r -q --format '%e %w%f' -o "$EVENTS" "$MNT" & +WATCH_PID=$! +# inotifywait establishes its watches asynchronously and drops anything that +# happens first, so wait until it says it is ready rather than guessing. +for _ in $(seq 1 100); do + touch "$MNT/.probe" 2>/dev/null + grep -q '.probe' "$EVENTS" 2>/dev/null && break + sleep 0.1 +done +rm -f "$MNT/.probe" 2>/dev/null +: > "$EVENTS" + +# Waits for a line matching an event-name pattern and a path, and reports how +# long it took. `$2` is an extended regex over the event names as inotifywait +# prints them (e.g. 'CREATE', 'MOVED_(FROM|TO)'). +await() { + local what="$1" events="$2" path="$3" start now + start=$(date +%s.%N) + for _ in $(seq 1 $((TIMEOUT * 20))); do + if grep -Eq "^[A-Z_,]*($events)[A-Z_,]* $MNT/$path\$" "$EVENTS" 2>/dev/null; then + now=$(date +%s.%N) + printf ' ok %-46s %6.0f ms\n' "$what" "$(elapsed_ms "$start" "$now")" + PASS=$((PASS+1)) + return 0 + fi + sleep 0.05 + done + bad "$what" + echo " wanted: $events on $path" + echo " events seen:" + sed 's/^/ | /' "$EVENTS" | tail -15 + return 1 +} + +echo +echo "== each change on the served tree must raise the matching inotify event ==" + +# Write. The one that matters most: this is a Windows editor saving a file, and +# the event every watch-mode tool is waiting for. +echo "edited by the other side" > "$WORK/src/app.js" +await "write -> MODIFY" "MODIFY" "src/app.js" + +# Create. +echo "brand new" > "$WORK/src/added.ts" +await "create -> CREATE" "CREATE" "src/added.ts" + +# Create a directory. +mkdir "$WORK/src/components" +await "mkdir -> CREATE,ISDIR" "CREATE" "src/components" + +# Rename. Both halves must arrive, which is what tells a watcher this is a move +# and not an unrelated delete plus create. +mv "$WORK/src/added.ts" "$WORK/src/renamed.ts" +await "rename -> MOVED_FROM (old name)" "MOVED_FROM" "src/added.ts" +await "rename -> MOVED_TO (new name)" "MOVED_TO" "src/renamed.ts" + +# Delete. +rm "$WORK/src/renamed.ts" +await "delete -> DELETE" "DELETE" "src/renamed.ts" + +# Delete a directory. +rmdir "$WORK/src/components" +await "rmdir -> DELETE,ISDIR" "DELETE" "src/components" + +echo +echo "== the mount must stay correct while all that is going on ==" +if [ "$(cat "$MNT/src/app.js")" = "edited by the other side" ]; then + ok "the new contents read back through the mount" +else + bad "the new contents read back through the mount" + echo " got: $(cat "$MNT/src/app.js" 2>&1)" +fi +if [ -e "$MNT/src/renamed.ts" ] || [ -e "$MNT/src/added.ts" ]; then + bad "a deleted file is gone from the mount" + ls -la "$MNT/src" +else + ok "a deleted file is gone from the mount" +fi +# The bridge performs real filesystem calls against the mount. If any of them +# were forwarded across the boundary instead of being recognised as its own, +# the served tree would be damaged — this is the check that would catch it. +if [ "$(cat "$WORK/keep/untouched.txt")" = "static" ] && [ "$(cat "$WORK/src/app.js")" = "edited by the other side" ]; then + ok "the served tree is untouched by the bridge" +else + bad "the served tree is untouched by the bridge" + echo " the bridge's own operations must never reach the agent" + ls -laR "$WORK" +fi + +echo +echo "== recursive watch over a large tree ==" +# The scale question from the issue: a Node-sized project is tens of thousands +# of paths, and both the watcher and the bridge have to survive one changing at +# once without overflowing into a rescan that names nothing. +BIG=$WORK/big +mkdir -p "$BIG" +for d in $(seq 1 100); do + mkdir -p "$BIG/d$d" + for f in $(seq 1 100); do echo "$f" > "$BIG/d$d/f$f.js"; done +done +# Let the burst settle, then check that the tree arrived and a change deep +# inside it still notifies. A burst this size overruns the watcher and becomes a +# rescan, which is the slow path on purpose (it is rate-limited), so wait for the +# count rather than assuming one interval is enough. +COUNT=0 +for _ in $(seq 1 60); do + COUNT=$(find "$MNT/big" -type f 2>/dev/null | wc -l) + [ "$COUNT" -eq 10000 ] && break + sleep 1 +done +if [ "$COUNT" -eq 10000 ]; then + ok "10000 files appeared on the mount" +else + bad "10000 files appeared on the mount (saw $COUNT)" +fi +: > "$EVENTS" +echo "deep change" > "$BIG/d50/f50.js" +await "write deep in a 10k-file tree -> MODIFY" "MODIFY" "big/d50/f50.js" + +echo +echo "== latency, against a write made locally on the mount ==" +# The comparison the issue asks for: the same event, once originating on the far +# side and once originating here, so the added cost of crossing the boundary is +# what separates the two numbers. +local_ms() { + : > "$EVENTS" + local start now + start=$(date +%s.%N) + echo "local" > "$MNT/keep/local.txt" + for _ in $(seq 1 200); do + grep -q "$MNT/keep/local.txt" "$EVENTS" 2>/dev/null && break + sleep 0.01 + done + now=$(date +%s.%N) + elapsed_ms "$start" "$now" +} +remote_ms() { + : > "$EVENTS" + local start now + start=$(date +%s.%N) + echo "remote $RANDOM" > "$WORK/keep/remote.txt" + for _ in $(seq 1 200); do + grep -q "$MNT/keep/remote.txt" "$EVENTS" 2>/dev/null && break + sleep 0.01 + done + now=$(date +%s.%N) + elapsed_ms "$start" "$now" +} +printf ' local write -> event: %6.0f ms\n' "$(local_ms)" +printf ' far-side -> event: %6.0f ms\n' "$(remote_ms)" +printf ' far-side -> event: %6.0f ms (warm)\n' "$(remote_ms)" + +echo +echo "passed: $PASS failed: $FAIL" +if [ "$FAIL" -ne 0 ]; then + echo "--- agent log (tail) ---"; tail -20 "$LOG/agent.log" 2>/dev/null + echo "--- mount log (tail) ---"; tail -20 "$LOG/mount.log" 2>/dev/null +fi +exit $FAIL diff --git a/src/agent/client.cpp b/src/agent/client.cpp index 510aaae..e8b3ea1 100644 --- a/src/agent/client.cpp +++ b/src/agent/client.cpp @@ -1,6 +1,7 @@ #include "agent/client.hpp" #include "core/auth_token.hpp" +#include "core/coalescer.hpp" #include "core/path.hpp" #include "core/version.hpp" @@ -568,8 +569,20 @@ void RemoteRoot::rescan_loop() { std::lock_guard lock(pf_mu_); pf_seen_.clear(); } - std::lock_guard s(stats_mu_); - ++stats_.rescans; + // Tell the change-notification consumer that the mirror was replaced, now + // that it has been. Nothing can name what changed - that is what an + // overflow means - so this is the one signal it gets, and it goes out after + // the new snapshot has landed rather than when the Rescan arrived. + InvalidatedPathsHook paths_hook; + { + std::lock_guard s(stats_mu_); + ++stats_.rescans; + paths_hook = paths_hook_; + } + if (paths_hook) { + const AppliedChange rescan_change{ChangeKind::Unknown, std::string{}, 0, NodeKind::Directory, true}; + paths_hook(std::span(&rescan_change, 1)); + } } } @@ -969,11 +982,11 @@ void RemoteRoot::apply_invalidation(std::span payload) { // Directories the batch removes. Collected while the tree can still say they // were directories, and swept out of the content cache once the lock is free. std::vector removed_dirs; - // Paths this batch actually changed. An op discarded as stale relative to + // What this batch actually changed. An op discarded as stale relative to // this client's own mutation did not change the mirror, so it must not be // reported as invalidated either - a consumer that drops caches for it would // be acting on an event we already decided was obsolete. - std::vector applied; + std::vector applied; { std::unique_lock lock(tree_mu_); for (const proto::InvalidationOp& op : batch->ops) { @@ -987,14 +1000,36 @@ void RemoteRoot::apply_invalidation(std::span payload) { local_mutations_.erase(it); // caught up; back to normal } } - if (op.kind != InvalidationKind::Rescan) applied.push_back(op.path); switch (op.kind) { - case InvalidationKind::Upsert: (void)tree_.upsert_path(op.path, op.attr); break; - case InvalidationKind::Remove: - if (const auto id = tree_.lookup(op.path, mode()); id && tree_.node(*id).is_dir()) - removed_dirs.push_back(op.path); + case InvalidationKind::Upsert: { + // Created or modified is decided here, against the mirror, because + // this is the only side that knows what it held a moment ago. The + // agent cannot tell the two apart: both look like "the path exists + // and something touched it" from a stat after the fact. + const bool existed = tree_.lookup(op.path, mode()).has_value(); + ChangeKind change = op.change; + if (!is_move(change)) change = existed ? ChangeKind::Modified : ChangeKind::Created; + applied.push_back(AppliedChange{change, op.path, op.cookie, op.attr.kind, false}); + (void)tree_.upsert_path(op.path, op.attr); + break; + } + case InvalidationKind::Remove: { + const auto id = tree_.lookup(op.path, mode()); + if (id && tree_.node(*id).is_dir()) removed_dirs.push_back(op.path); + // Nothing was there to remove. The mirror is already right, and a + // consumer told a path it never saw has gone away would report a + // deletion that never happened. The source half of a move is the + // exception: its partner's arrival is real either way, and dropping + // half of a pair costs the consumer the move. + if (id || op.change == ChangeKind::MovedFrom) { + const NodeKind kind = id ? tree_.node(*id).attr.kind : NodeKind::File; + applied.push_back(AppliedChange{op.change == ChangeKind::MovedFrom ? ChangeKind::MovedFrom + : ChangeKind::Removed, + op.path, op.cookie, kind, false}); + } (void)tree_.remove_path(op.path, mode()); break; + } case InvalidationKind::Rescan: rescan = true; break; // handled below, off this thread } } @@ -1009,6 +1044,11 @@ void RemoteRoot::apply_invalidation(std::span payload) { else snapshot_replay_overflow_ = true; } } + // Dropping a removal for a path the mirror never had can strand the other + // half of a move, and so can a frame boundary: a batch too large for one + // frame is broadcast in several, and the two halves may land in different + // ones. Either way the surviving half is degraded rather than left dangling. + repair_move_pairs(applied); for (const std::string& d : removed_dirs) drop_cached_prefix(d); if (rescan) { // The agent's watcher overflowed: per-path events were lost, so the mirror diff --git a/src/agent/client.hpp b/src/agent/client.hpp index f939fd7..3761c7f 100644 --- a/src/agent/client.hpp +++ b/src/agent/client.hpp @@ -52,12 +52,32 @@ class RemoteRoot { std::uint64_t rescan_failures = 0; }; + /// One far-side change this client actually applied to its mirror, described + /// the way a change-notification consumer needs it rather than the way the + /// mirror needed it. + /// + /// The difference is what the mirror knows and the agent does not. The agent + /// reports that a path exists and has changed; only the mirror can say + /// whether it had that path a moment ago, which is what separates a file + /// being created from the same file being rewritten. `change` here is that + /// verdict, already made. + struct AppliedChange { + ChangeKind change = ChangeKind::Unknown; + std::string path; // relative, '/'-separated + std::uint32_t cookie = 0; // pairs the two halves of a move + NodeKind kind = NodeKind::File; // what the path is — for a removal, what it was + // The whole mirror was replaced: the agent's watcher lost events, so no + // per-path change can be named. `path` is empty and every other field + // meaningless. Delivered after the replacement snapshot has landed. + bool rescan = false; + }; + using InvalidationHook = std::function; - /// Paths whose mirrored state a batch actually changed - the ops that were - /// applied, not the ones discarded as stale relative to this client's own - /// mutation. Separate from InvalidationHook, which reports the raw batch for + /// Changes a batch actually made to the mirror - the ops that were applied, + /// not the ones discarded as stale relative to this client's own mutation. + /// Separate from InvalidationHook, which reports the raw batch for /// diagnostics (`wsldrive watch`) and so has different callers. - using InvalidatedPathsHook = std::function)>; + using InvalidatedPathsHook = std::function)>; explicit RemoteRoot(std::unique_ptr ch); ~RemoteRoot(); diff --git a/src/agent/server.cpp b/src/agent/server.cpp index 16de04d..cc75eab 100644 --- a/src/agent/server.cpp +++ b/src/agent/server.cpp @@ -123,6 +123,8 @@ void RootServer::flush_loop() { proto::InvalidationOp out; out.kind = op.kind; out.path = std::move(op.path); + out.change = op.change; + out.cookie = op.cookie; if (out.kind != InvalidationKind::Rescan) { // Both kinds are resolved against the disk NOW, not against the event. // A Remove that is not re-checked is a real hazard: git renames @@ -133,11 +135,28 @@ void RootServer::flush_loop() { auto attr = read_attributes(join_relative(opts_.root, out.path)); if (!attr) { out.kind = InvalidationKind::Remove; // gone (or vanished between the event and now) + // The disk overrules the event for the change kind too. Gone means + // removed, unless the event already said this is the source half of a + // move — which is the one reading of "gone" that carries more + // information, so it survives with its cookie. + if (out.change != ChangeKind::MovedFrom) { + out.change = ChangeKind::Removed; + out.cookie = 0; + } } else if (attr->kind == NodeKind::Other) { continue; } else { out.kind = InvalidationKind::Upsert; // present, whatever the event said out.attr = *attr; + // Present, so it is not gone and not the source of a move. Say only + // what is still true: something changed here. A peer that tracks + // change notifications decides between "created" and "modified" from + // whether its own mirror already had the path — it is the only side + // that knows, and it has to make that call regardless. + if (out.change == ChangeKind::Removed || out.change == ChangeKind::MovedFrom) { + out.change = ChangeKind::Modified; + out.cookie = 0; + } } } // A directory that has just appeared (mkdir, or - the case that matters - @@ -149,6 +168,10 @@ void RootServer::flush_loop() { batch.ops.push_back(std::move(out)); } if (!expand_dirs.empty()) append_subtrees(std::move(expand_dirs), batch.ops); + // Re-stating every path above can reclassify one half of a move (its source + // exists again, or its destination is already gone) and leave the other + // half pointing at a partner this batch no longer contains. + repair_move_pairs(batch.ops); if (!batch.ops.empty()) broadcast_batch(batch); lock.lock(); } @@ -178,7 +201,10 @@ void RootServer::append_subtrees(std::vector dirs, std::vector opts_.max_expanded_entries) over = true; - if (!over && !present.contains(p)) ops.push_back(proto::InvalidationOp{InvalidationKind::Upsert, p, e.attr}); + // The whole subtree is arriving at once, so every entry in it is new + // to the peer even though no watcher event named it. + if (!over && !present.contains(p)) + ops.push_back(proto::InvalidationOp{InvalidationKind::Upsert, p, e.attr, ChangeKind::Created, 0}); rels.push_back(std::move(p)); }, // Once over budget, decline every subdirectory so the scan winds down diff --git a/src/core/coalescer.cpp b/src/core/coalescer.cpp index 0d32eeb..d414869 100644 --- a/src/core/coalescer.cpp +++ b/src/core/coalescer.cpp @@ -3,6 +3,7 @@ #include "core/path.hpp" #include +#include namespace wsld { @@ -19,18 +20,32 @@ void Coalescer::push(const FsEvent& ev, clock::time_point now) { InvalidationKind kind; bool appeared = false; + ChangeKind change; + std::uint32_t cookie = 0; switch (ev.kind) { case FsEventKind::Created: + kind = InvalidationKind::Upsert; + appeared = true; + change = ChangeKind::Created; + break; case FsEventKind::RenamedTo: kind = InvalidationKind::Upsert; appeared = true; + change = ChangeKind::MovedTo; + cookie = ev.cookie; break; case FsEventKind::Modified: kind = InvalidationKind::Upsert; + change = ChangeKind::Modified; break; case FsEventKind::Removed: + kind = InvalidationKind::Remove; + change = ChangeKind::Removed; + break; case FsEventKind::RenamedFrom: kind = InvalidationKind::Remove; + change = ChangeKind::MovedFrom; + cookie = ev.cookie; break; case FsEventKind::Overflow: default: @@ -40,9 +55,20 @@ void Coalescer::push(const FsEvent& ev, clock::time_point now) { if (auto it = pending_.find(ev.path); it != pending_.end()) { // A Modified after a Created keeps the entry "new"; a removal resets it. const bool still_new = kind == InvalidationKind::Upsert && (appeared || it->second.appeared); - it->second = Entry{kind, ++seq_, still_new}; + // Same idea for the reported change: writes that land on a path this batch + // has already seen appear (or move) into place do not turn it into a plain + // modification — the consumer still has to be told the path is new, and a + // move must keep the cookie that pairs it with its other half. Every other + // event supersedes what came before it. + const bool keep = change == ChangeKind::Modified && + (it->second.change == ChangeKind::Created || it->second.change == ChangeKind::MovedTo); + if (keep) { + change = it->second.change; + cookie = it->second.cookie; + } + it->second = Entry{kind, ++seq_, still_new, change, cookie}; } else { - pending_.emplace(std::string(ev.path), Entry{kind, ++seq_, appeared}); + pending_.emplace(std::string(ev.path), Entry{kind, ++seq_, appeared, change, cookie}); } } @@ -75,10 +101,13 @@ std::vector Coalescer::take() { InvalidationKind kind; std::uint64_t seq; bool appeared; + ChangeKind change; + std::uint32_t cookie; }; std::vector items; items.reserve(pending_.size()); - for (const auto& [path, e] : pending_) items.push_back(Item{path, e.kind, e.seq, e.appeared}); + for (const auto& [path, e] : pending_) + items.push_back(Item{path, e.kind, e.seq, e.appeared, e.change, e.cookie}); // Sort by path so that a removed directory is immediately followed by its // descendants; drop descendants whose last event predates the removal. @@ -99,8 +128,15 @@ std::vector Coalescer::take() { // Emit in arrival order of each path's final event. std::sort(kept.begin(), kept.end(), [](const Item& a, const Item& b) { return a.seq < b.seq; }); + out.reserve(kept.size()); - for (const Item& it : kept) out.push_back(PlannedOp{it.kind, std::string(it.path), it.appeared}); + for (const Item& it : kept) + out.push_back(PlannedOp{it.kind, std::string(it.path), it.appeared, it.change, it.cookie}); + + // Collapsing can strand one half of a move: its partner is written again and + // keeps the move kind while this one is re-created and loses it, or a removed + // directory takes the partner out of the batch entirely. + repair_move_pairs(out); pending_.clear(); return out; diff --git a/src/core/coalescer.hpp b/src/core/coalescer.hpp index 75c2137..69ef937 100644 --- a/src/core/coalescer.hpp +++ b/src/core/coalescer.hpp @@ -28,6 +28,12 @@ enum class FsEventKind : std::uint8_t { struct FsEvent { FsEventKind kind; std::string_view path; + // Ties the two halves of a rename together. inotify supplies one per move + // pair; the Windows watcher mints one for each adjacent OLD_NAME/NEW_NAME + // pair, since ReadDirectoryChangesW has no equivalent. Zero means "not part + // of a pair we could identify", which a consumer must degrade gracefully on: + // an unpaired RenamedFrom is a removal, an unpaired RenamedTo a creation. + std::uint32_t cookie = 0; }; /// A planned invalidation. Attributes are not resolved here: the watcher side @@ -40,12 +46,46 @@ struct PlannedOp { // the watcher never reports, so the sender enumerates it; a Modified on a // directory the peers already know must not trigger that scan. bool appeared = false; + // What the watcher actually saw, kept alongside `kind` for the change + // notification path. Collapsing preserves the shape of the burst rather than + // its last event: a create followed by three writes is still a creation. + ChangeKind change = ChangeKind::Unknown; + std::uint32_t cookie = 0; // meaningful only for the two move kinds friend bool operator==(const PlannedOp& a, const PlannedOp& b) noexcept { - return a.kind == b.kind && a.path == b.path; // `appeared` is advisory + return a.kind == b.kind && a.path == b.path; // the rest is advisory } }; +/// Degrades every move half in `ops` that has no partner into the plain change +/// it amounts to on its own: a source with no destination is a removal, a +/// destination with no source is a creation. +/// +/// Halves get separated twice on the way out: the coalescer can collapse one of +/// them away, and the sender re-stats every path afterwards and may reclassify +/// one of them. Running this after each step keeps one invariant true for +/// consumers — a move that reaches them is always matched — so none of them has +/// to carry its own half-move fallback. +/// +/// Works on anything with `change` and `cookie` members (PlannedOp on the way +/// out of the coalescer, proto::InvalidationOp on the way onto the wire). +template +void repair_move_pairs(std::vector& ops) { + std::unordered_map halves; // cookie -> bitmask of halves seen + for (const Op& op : ops) { + if (op.cookie == 0 || !is_move(op.change)) continue; + halves[op.cookie] |= op.change == ChangeKind::MovedFrom ? 1 : 2; + } + if (halves.empty()) return; + for (Op& op : ops) { + if (!is_move(op.change)) continue; + const auto it = halves.find(op.cookie); + if (op.cookie != 0 && it != halves.end() && it->second == 3) continue; + op.change = op.change == ChangeKind::MovedFrom ? ChangeKind::Removed : ChangeKind::Created; + op.cookie = 0; + } +} + /// Collapses bursts of watcher events into a minimal ordered batch. /// /// - Repeated events on one path collapse to the last relevant operation. @@ -88,6 +128,8 @@ class Coalescer { InvalidationKind kind; std::uint64_t seq; bool appeared; + ChangeKind change; + std::uint32_t cookie; }; Options opts_; diff --git a/src/core/protocol.cpp b/src/core/protocol.cpp index c9f157d..9e4fc80 100644 --- a/src/core/protocol.cpp +++ b/src/core/protocol.cpp @@ -264,6 +264,10 @@ void write_invalidation(Writer& w, const InvalidationBatch& b) { w.u8(static_cast(op.kind)); w.string(op.path); if (op.kind == InvalidationKind::Upsert) write_attributes(w, op.attr); + // One extra byte per op in the common case; the cookie rides along only for + // the two move kinds, which are a small minority of any real batch. + w.u8(static_cast(op.change)); + if (is_move(op.change)) w.u32(op.cookie); } } @@ -290,6 +294,15 @@ Result read_invalidation(Reader& r) noexcept { if (!attr) return fail(attr.error()); op.attr = *attr; } + auto change = r.u8(); + if (!change) return fail(change.error()); + if (*change > kMaxChangeKind) return fail(Errc::Corrupt); + op.change = static_cast(*change); + if (is_move(op.change)) { + auto cookie = r.u32(); + if (!cookie) return fail(cookie.error()); + op.cookie = *cookie; + } b.ops.push_back(std::move(op)); } return b; diff --git a/src/core/protocol.hpp b/src/core/protocol.hpp index 35f3327..0b4d503 100644 --- a/src/core/protocol.hpp +++ b/src/core/protocol.hpp @@ -22,7 +22,9 @@ namespace wsld::proto { inline constexpr std::uint32_t kMagic = 0x444C5357; // "WSLD" when read as little-endian bytes -inline constexpr std::uint16_t kVersion = 3; // 3: mutual handshake (nonces + proofs, no token on the wire) +// 3: mutual handshake (nonces + proofs, no token on the wire) +// 4: invalidation ops carry the change kind and a rename-pairing cookie +inline constexpr std::uint16_t kVersion = 4; inline constexpr std::size_t kHeaderSize = 24; inline constexpr std::uint32_t kMaxPayload = 64u << 20; // 64 MiB per frame @@ -160,6 +162,10 @@ struct InvalidationOp { InvalidationKind kind; std::string path; // normalised, '/'-separated, relative to the mount root Attributes attr; // meaningful for Upsert only + // What the watcher saw, for peers that deliver change notifications rather + // than only mirroring metadata. Ignoring both fields leaves a peer correct. + ChangeKind change = ChangeKind::Unknown; + std::uint32_t cookie = 0; // pairs the two halves of a move; 0 otherwise }; struct InvalidationBatch { diff --git a/src/core/types.hpp b/src/core/types.hpp index 3182291..f851b3b 100644 --- a/src/core/types.hpp +++ b/src/core/types.hpp @@ -38,4 +38,30 @@ enum class InvalidationKind : std::uint8_t { Rescan = 2, // the watcher lost events; refetch a snapshot of this subtree }; +/// What actually happened to a path, as opposed to what a mirror must do about +/// it. InvalidationKind is all a metadata mirror needs: refresh the node, or +/// drop it. A change-notification consumer needs more — it has to tell a file +/// that appeared from one that was rewritten, and it has to see the two halves +/// of a rename as one move rather than as an unrelated removal and creation. +/// +/// The two travel together in every invalidation op. Nothing in the mirror +/// depends on this field, so a consumer that ignores it stays correct. +enum class ChangeKind : std::uint8_t { + Unknown = 0, // no watcher event behind this op (a subtree expansion, a rescan) + Created = 1, + Modified = 2, + Removed = 3, + MovedFrom = 4, // source half of a rename, paired with its MovedTo by cookie + MovedTo = 5, // destination half +}; + +/// Highest valid ChangeKind value, for validating one off the wire. Derived, so +/// adding a kind cannot leave it behind. +inline constexpr std::uint8_t kMaxChangeKind = static_cast(ChangeKind::MovedTo); + +/// True for the two halves of a rename, the only ops that carry a cookie. +[[nodiscard]] constexpr bool is_move(ChangeKind c) noexcept { + return c == ChangeKind::MovedFrom || c == ChangeKind::MovedTo; +} + } // namespace wsld diff --git a/src/mount/CMakeLists.txt b/src/mount/CMakeLists.txt index 4f31e4b..05834d5 100644 --- a/src/mount/CMakeLists.txt +++ b/src/mount/CMakeLists.txt @@ -1,4 +1,4 @@ -add_library(wsldrive_mount STATIC fuse_mount.cpp) +add_library(wsldrive_mount STATIC fuse_mount.cpp fsnotify_bridge.cpp) add_library(wsldrive::mount ALIAS wsldrive_mount) # WSLDRIVE_HAVE_MOUNT is PUBLIC so the CLI enables the mount subcommand on either diff --git a/src/mount/fsnotify_bridge.cpp b/src/mount/fsnotify_bridge.cpp new file mode 100644 index 0000000..61aaa62 --- /dev/null +++ b/src/mount/fsnotify_bridge.cpp @@ -0,0 +1,374 @@ +#include "mount/fsnotify_bridge.hpp" + +#include +#include +#include +#include +#include + +#ifdef __linux__ +#include +#include +#include +#include +#include +#include + +#ifndef FUSE_SUPER_MAGIC +#define FUSE_SUPER_MAGIC 0x65735546 +#endif +#endif + +namespace wsld::mount { + +namespace { + +const char* change_name(ChangeKind c) noexcept { + switch (c) { + case ChangeKind::Created: return "creation of"; + case ChangeKind::Modified: return "change to"; + case ChangeKind::Removed: return "removal of"; + case ChangeKind::MovedFrom: return "move away from"; + case ChangeKind::MovedTo: return "move onto"; + case ChangeKind::Unknown: break; + } + return "change to"; +} + +#ifdef __linux__ +long this_tid() noexcept { return static_cast(::syscall(SYS_gettid)); } +#endif + +} // namespace + +FsNotifyBridge::~FsNotifyBridge() { stop(); } + +bool FsNotifyBridge::supported() noexcept { +#ifdef __linux__ + return true; +#else + return false; +#endif +} + +std::string FsNotifyBridge::abs(std::string_view rel) const { + std::string out = root_; + if (!out.empty() && out.back() == '/') out.pop_back(); + out.push_back('/'); + out.append(rel); + return out; +} + +FsNotifyBridge::Stats FsNotifyBridge::stats() const { + std::lock_guard lock(stats_mu_); + return stats_; +} + +bool FsNotifyBridge::is_self(int caller) const noexcept { + const long c = static_cast(caller); + // FUSE reports the calling task's pid, which for a threaded caller is its + // tid. Accept the process id as well: some paths report the thread group, + // and either way it is this daemon and nothing else. + return c != 0 && (c == poke_tid_.load(std::memory_order_relaxed) || + c == self_pid_.load(std::memory_order_relaxed)); +} + +void FsNotifyBridge::set_active(Active a) { + std::lock_guard lock(active_mu_); + active_ = std::move(a); +} + +void FsNotifyBridge::clear_active() { + std::lock_guard lock(active_mu_); + active_ = Active{}; +} + +FsNotifyBridge::Pretend FsNotifyBridge::pretend(int caller, std::string_view rel, + NodeKind& kind) const noexcept { + if (!is_self(caller)) return Pretend::Nothing; + std::lock_guard lock(active_mu_); + if (active_.pretend_path == Pretend::Nothing || active_.path != rel) return Pretend::Nothing; + kind = active_.kind; + return active_.pretend_path; +} + +bool FsNotifyBridge::is_poke_thread(int caller) const noexcept { return is_self(caller); } + +bool FsNotifyBridge::claim(int caller, Poke what, std::string_view rel, std::string_view rel2) { + if (!is_self(caller)) return false; + std::lock_guard lock(active_mu_); + if (active_.what != what || active_.path != rel || active_.path2 != rel2) return false; + // Retire the record. The syscall that raised this request is about to return, + // and the pretence must be gone before libfuse re-reads the path to build its + // reply — otherwise the reply would describe the fiction instead of the + // mirror's real state. + active_ = Active{}; + return true; +} + +void FsNotifyBridge::post(std::span changes) { + if (!running_.load(std::memory_order_relaxed)) return; + + // Pair the two halves of each move into one job before queueing, so the poke + // loop only ever sees whole operations. Both sides upstream try to keep pairs + // together, but a batch too large for one frame is broadcast in several and + // the halves can still arrive apart; a half with no partner here degrades to + // what it amounts to alone rather than being dropped. + std::unordered_map from_by_cookie; + std::vector jobs; + jobs.reserve(changes.size()); + for (const auto& c : changes) { + if (c.rescan) { + jobs.push_back(Job{ChangeKind::Unknown, {}, {}, NodeKind::Directory, true}); + continue; + } + if (c.change == ChangeKind::MovedFrom && c.cookie != 0) { + from_by_cookie.emplace(c.cookie, jobs.size()); + jobs.push_back(Job{ChangeKind::MovedFrom, c.path, {}, c.kind, false}); + continue; + } + if (c.change == ChangeKind::MovedTo && c.cookie != 0) { + if (const auto it = from_by_cookie.find(c.cookie); it != from_by_cookie.end()) { + jobs[it->second].path2 = c.path; // completes the pair in place, keeping its order + from_by_cookie.erase(it); + continue; + } + jobs.push_back(Job{ChangeKind::Created, c.path, {}, c.kind, false}); // no partner: it appeared + continue; + } + jobs.push_back(Job{c.change, c.path, {}, c.kind, false}); + } + // Sources still waiting for a destination lost it somewhere upstream; on + // their own they are removals. + for (const auto& [cookie, idx] : from_by_cookie) { + (void)cookie; + jobs[idx].change = ChangeKind::Removed; + } + + std::size_t dropped = 0; + { + std::lock_guard lock(mu_); + for (Job& j : jobs) { + if (queue_.size() >= kQueueCap) { + queue_.pop_front(); + ++dropped; + } + queue_.push_back(std::move(j)); + } + } + if (dropped != 0) { + std::lock_guard lock(stats_mu_); + stats_.dropped += dropped; + } + cv_.notify_one(); +} + +void FsNotifyBridge::run() { + poke_tid_.store(this_tid_or_zero(), std::memory_order_relaxed); + // Verified here rather than in start(): the check reads the mount, and the + // mount is not answering until the FUSE loop is running, which start()'s + // caller is in the middle of arranging. Blocking there would deadlock it. + if (!verify_root()) { + running_.store(false); + return; + } + for (;;) { + Job job; + { + std::unique_lock lock(mu_); + cv_.wait(lock, [this] { return stop_.load() || !queue_.empty(); }); + if (stop_.load()) return; + job = std::move(queue_.front()); + queue_.pop_front(); + } + deliver(job); + } +} + +void FsNotifyBridge::deliver(const Job& job) { + bool ok = false; + { + std::lock_guard lock(stats_mu_); + ++stats_.events; + if (job.rescan) ++stats_.rescans; + } + if (job.rescan) { + // An overflow on the far side means no path can be named. Touch the mount + // root so a watcher that re-walks on any event below its root gets the + // chance to; one that waits for a specific path cannot be told. + ok = poke_modify(Job{ChangeKind::Modified, {}, {}, NodeKind::Directory, false}); + } else if (!job.path2.empty()) { + ok = poke_move(job); + } else { + switch (job.change) { + case ChangeKind::Created: ok = poke_create(job); break; + case ChangeKind::Modified: ok = poke_modify(job); break; + case ChangeKind::Removed: + case ChangeKind::MovedFrom: ok = poke_remove(job); break; + case ChangeKind::MovedTo: ok = poke_create(job); break; + case ChangeKind::Unknown: return; // nothing to say about it + } + } + std::lock_guard lock(stats_mu_); + if (ok) { + ++stats_.delivered; + return; + } + ++stats_.failed; + // A failed poke is a missed notification, which is silent by nature — the + // mount stays correct, so nothing else would ever say it happened. Name the + // first few, then stop: a storm of them must not become the log. + if (stats_.failed <= kMaxReportedFailures) + std::fprintf(stderr, "wsldrive: could not notify watchers of %s '%s'%s\n", change_name(job.change), + job.path.empty() ? "" : job.path.c_str(), + stats_.failed == kMaxReportedFailures ? " (further failures not reported)" : ""); +} + +#ifdef __linux__ + +long FsNotifyBridge::this_tid_or_zero() noexcept { return this_tid(); } + +bool FsNotifyBridge::inside_mount(std::string_view rel) const { + // Check the parent, not the path itself: for a creation the path does not + // exist yet, and for a removal it no longer does. + const std::size_t slash = rel.rfind('/'); + const std::string parent = slash == std::string_view::npos ? root_ : abs(rel.substr(0, slash)); + struct ::stat st {}; + if (::lstat(parent.c_str(), &st) != 0) return false; + return static_cast(st.st_dev) == root_dev_; +} + +bool FsNotifyBridge::verify_root() { + // The pokes are ordinary filesystem calls, so pointing them anywhere but the + // intended FUSE mount would create and delete real files in the wrong tree. + // Two things are established here and relied on for the rest of the run: the + // root is a FUSE mount, and which device that mount is — every poke re-checks + // its target against the latter, so a path that resolves out of the mount is + // refused rather than applied to whatever it landed on. + struct ::statfs fs {}; + struct ::stat st {}; + if (::statfs(root_.c_str(), &fs) != 0 || ::lstat(root_.c_str(), &st) != 0) { + std::fprintf(stderr, "wsldrive: inotify bridge disabled (cannot stat %s)\n", root_.c_str()); + return false; + } + if (static_cast(fs.f_type) != FUSE_SUPER_MAGIC) { + std::fprintf(stderr, "wsldrive: inotify bridge disabled (%s is not a FUSE mount)\n", root_.c_str()); + return false; + } + root_dev_ = static_cast(st.st_dev); + return true; +} + +Result FsNotifyBridge::start(const std::string& mount_root) { + if (running_.load()) return {}; + root_ = mount_root; + self_pid_.store(static_cast(::getpid()), std::memory_order_relaxed); + stop_.store(false); + running_.store(true); + thread_ = std::thread([this] { run(); }); + return {}; +} + +void FsNotifyBridge::stop() { + if (!running_.exchange(false)) return; + { + std::lock_guard lock(mu_); + stop_.store(true); + queue_.clear(); + } + cv_.notify_all(); + if (thread_.joinable()) thread_.join(); + clear_active(); +} + +bool FsNotifyBridge::poke_create(const Job& job) { + if (job.path.empty() || !inside_mount(job.path)) return false; + const bool dir = job.kind == NodeKind::Directory; + // The invalidation has already been applied, so the mirror knows this path + // and the kernel's lookup would find it and never reach ->mknod/->mkdir. + // Hide it from this thread alone for the length of the call. + set_active(Active{dir ? Poke::Mkdir : Poke::Mknod, job.path, {}, Pretend::Absent, job.kind}); + const std::string p = abs(job.path); + const int rc = dir ? ::mkdir(p.c_str(), 0755) : ::mknod(p.c_str(), S_IFREG | 0644, 0); + clear_active(); + return rc == 0; +} + +bool FsNotifyBridge::poke_modify(const Job& job) { + if (!job.path.empty() && !inside_mount(job.path)) return false; + // Only mtime. fsnotify_change() maps a change to both timestamps to + // FS_ATTRIB (IN_ATTRIB) and one to mtime alone to FS_MODIFY (IN_MODIFY), and + // IN_MODIFY is what a watch-mode tool is actually waiting for. No + // interception is needed: the mount already accepts utimens without + // forwarding it, and the mirror keeps reporting the agent's timestamps. + const struct ::timespec times[2] = {{0, UTIME_OMIT}, {0, UTIME_NOW}}; + const std::string p = job.path.empty() ? root_ : abs(job.path); + return ::utimensat(AT_FDCWD, p.c_str(), times, AT_SYMLINK_NOFOLLOW) == 0; +} + +bool FsNotifyBridge::poke_remove(const Job& job) { + if (job.path.empty() || !inside_mount(job.path)) return false; + const bool dir = job.kind == NodeKind::Directory; + // The mirror has already dropped the path, so the kernel would resolve it to + // nothing and ->unlink would never run. Answer this thread's lookup with the + // entry as it was. The ghost dentry does not outlive the call: the unlink + // that follows drops it, and nothing else can see it in between, because + // every other caller gets the mirror's real answer. + set_active(Active{dir ? Poke::Rmdir : Poke::Unlink, job.path, {}, Pretend::Present, job.kind}); + const std::string p = abs(job.path); + const int rc = dir ? ::rmdir(p.c_str()) : ::unlink(p.c_str()); + clear_active(); + // On success the removal itself drops the ghost dentry. On failure it does + // not, and the kernel is left holding an entry built from the fiction — so a + // path the far side deleted would read as still there, to anyone, until the + // entry cache lets go of it. Drop it explicitly instead. + if (rc != 0 && invalidate_) invalidate_(job.path); + return rc == 0; +} + +bool FsNotifyBridge::poke_move(const Job& job) { + if (job.path.empty() || job.path2.empty()) return false; + if (!inside_mount(job.path) || !inside_mount(job.path2)) return false; + // Letting the kernel do the rename is what makes this worth pairing the two + // halves for: vfs_rename raises IN_MOVED_FROM and IN_MOVED_TO together, with + // one cookie, which is exactly what a watcher needs to recognise a move + // rather than an unrelated delete and create. + // + // The source is ghosted with the *destination's* type, because after the + // rename that is the entry the kernel keeps. + set_active(Active{Poke::Rename, job.path, job.path2, Pretend::Present, job.kind}); + const int rc = ::rename(abs(job.path).c_str(), abs(job.path2).c_str()); + clear_active(); + + // The kernel does not build a fresh inode for the destination; `d_move` walks + // the source's dentry over to the new name, and that dentry is the one holding + // the placeholder attributes the ghost handed out. Left alone, the destination + // would report a zero size for as long as the attribute cache holds it. Drop + // the entry so the next lookup goes back to the mirror. + // + // The invalidation hook queues a punch for both paths anyway, but on another + // thread and with no ordering against this one, so it cannot be relied on to + // land after the rename. + // + // A rename that failed leaves the ghost where it was built, on the source — + // a path the far side has moved away from, reading as still present to + // anyone who looks within the entry cache's lifetime. Drop that instead. + if (invalidate_) invalidate_(rc == 0 ? job.path2 : job.path); + return rc == 0; +} + +#else // not Linux + +long FsNotifyBridge::this_tid_or_zero() noexcept { return 0; } +bool FsNotifyBridge::verify_root() { return false; } +bool FsNotifyBridge::inside_mount(std::string_view) const { return false; } +Result FsNotifyBridge::start(const std::string&) { return fail(Errc::Unsupported); } +void FsNotifyBridge::stop() {} +bool FsNotifyBridge::poke_create(const Job&) { return false; } +bool FsNotifyBridge::poke_modify(const Job&) { return false; } +bool FsNotifyBridge::poke_remove(const Job&) { return false; } +bool FsNotifyBridge::poke_move(const Job&) { return false; } + +#endif + +} // namespace wsld::mount diff --git a/src/mount/fsnotify_bridge.hpp b/src/mount/fsnotify_bridge.hpp new file mode 100644 index 0000000..f2a3920 --- /dev/null +++ b/src/mount/fsnotify_bridge.hpp @@ -0,0 +1,228 @@ +#pragma once + +#include "agent/client.hpp" +#include "core/error.hpp" +#include "core/types.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace wsld::mount { + +/// Makes far-side changes visible to ordinary Linux file watchers. +/// +/// # The problem +/// +/// A change made on the Windows side of the boundary reaches this mount as an +/// invalidation, so the metadata mirror and the page cache are correct within +/// milliseconds. What does *not* happen is an `inotify` event, and that is what +/// every watch-mode tool in the Linux ecosystem is waiting for. The tool does +/// not error; it simply never fires. That is the whole of microsoft/WSL#4739. +/// +/// # Why it cannot be delivered directly +/// +/// The kernel raises fsnotify events from the VFS, at the point an operation is +/// performed — `vfs_create` calls `fsnotify_create`, `vfs_unlink` calls +/// `fsnotify_unlink`, and so on. A filesystem cannot raise one on its own, and +/// FUSE offers no notification that does. The three `fuse_lowlevel_notify_*` +/// calls invalidate dentries and pages, which is a cache-coherence mechanism, +/// not a notification one: `fuse_reverse_inval_entry` never touches fsnotify. +/// A userspace daemon has no way to hand an inotify watcher an event. +/// +/// # What this does instead +/// +/// It asks the kernel to raise the event, by performing the operation the far +/// side already performed — on this mount, from a thread of this process. A +/// file created over there becomes a `mknodat` here, a deletion an `unlinkat`, +/// a rename a `renameat`, a write a `utimensat` that touches only mtime (which +/// `fsnotify_change` reports as FS_MODIFY, not FS_ATTRIB). The kernel runs its +/// own hooks and every watcher gets a genuine event of the right type, with a +/// genuine rename cookie pairing the two halves of a move. Consumers need no +/// cooperation, no preload, and no knowledge that wsldrive exists. +/// +/// The operations must not cross the boundary a second time — the far side +/// already has this state, and re-applying it would at best be wasted work and +/// at worst destroy the file that prompted the event. So the FUSE handlers +/// recognise these requests and answer them locally without forwarding. That +/// recognition is the delicate part, and it is deliberately over-determined: +/// see `claim()`. +/// +/// Only the mount's own view is touched. Nothing is written to the served tree. +/// +/// # Cost +/// +/// One syscall per changed path, served by the FUSE loop from the in-RAM +/// mirror. No boundary crossing, no I/O. +/// +/// Linux only. `supported()` is false elsewhere and `start()` fails with +/// Unsupported; Direction A has no equivalent problem, because a WinFsp volume +/// raises Windows change notifications through its own mechanism. +class FsNotifyBridge { + public: + /// The operations the bridge performs on itself. A FUSE handler names the one + /// it implements when asking whether the request in front of it is a poke. + enum class Poke : std::uint8_t { + None, + Mknod, // a file appeared + Mkdir, // a directory appeared + Unlink, // a file went away + Rmdir, // a directory went away + Rename, // both halves of a move + }; + + /// What a handler should pretend about a path, so that the kernel will run + /// the operation that raises the event. Both are visible only to the bridge's + /// own thread: any other caller gets the mirror's real answer. + enum class Pretend : std::uint8_t { + Nothing, + Absent, // the mirror already has the new path; hide it so ->create runs + Present, // the mirror already dropped the old path; ghost it so ->unlink runs + }; + + struct Stats { + std::uint64_t events = 0; // changes taken off the queue + std::uint64_t delivered = 0; // pokes the kernel accepted (an event went out) + std::uint64_t failed = 0; // pokes that returned an error + std::uint64_t dropped = 0; // changes discarded because the queue was full + std::uint64_t rescans = 0; // mirror replacements, which name no path + }; + + FsNotifyBridge() = default; + ~FsNotifyBridge(); + FsNotifyBridge(const FsNotifyBridge&) = delete; + FsNotifyBridge& operator=(const FsNotifyBridge&) = delete; + + /// Drops the kernel's cached dentry and pages for one mount-relative path. + /// The rename poke needs it; see the comment in poke_move(). + using InvalidatePath = std::function; + void set_invalidate(InvalidatePath fn) { invalidate_ = std::move(fn); } + + [[nodiscard]] static bool supported() noexcept; + + /// Starts the poke thread against an already-mounted `mount_root`. Fails with + /// Unsupported off Linux, and with InvalidArgument if `mount_root` is not a + /// FUSE mount — the pokes are real filesystem operations, so pointing them at + /// anything else would modify the wrong tree. + [[nodiscard]] Result start(const std::string& mount_root); + + /// Stops delivery and joins. Must run before the mount goes away. Idempotent. + void stop(); + + [[nodiscard]] bool running() const noexcept { return running_.load(std::memory_order_relaxed); } + + /// Queues changes for delivery. Called from the client's reader thread; never + /// blocks, and drops rather than growing without bound. + void post(std::span changes); + + [[nodiscard]] Stats stats() const; + + // --- consulted by the FUSE handlers --------------------------------------- + + /// Whether `caller` should be told something other than the truth about + /// `rel`, so the kernel proceeds into the operation that raises the event. + /// `kind` receives the type to report for Pretend::Present. + [[nodiscard]] Pretend pretend(int caller, std::string_view rel, NodeKind& kind) const noexcept; + + /// Whether the request in front of a handler is this bridge's own poke, and + /// so must be answered locally instead of forwarded across the boundary. + /// + /// Three independent things must hold, because the cost of a false positive + /// is a silently swallowed user operation — a delete that does not delete: + /// + /// 1. the caller is the bridge's own poke thread (its tid, or this + /// process's pid; no other thread here ever touches the mount), + /// 2. a poke of exactly this kind is in flight, and + /// 3. it names exactly this path (both paths, for a rename). + /// + /// A poke is claimed at most once: the record is retired by this call, so a + /// retry or a duplicate request is forwarded like any other. + [[nodiscard]] bool claim(int caller, Poke what, std::string_view rel, std::string_view rel2 = {}); + + /// Whether `caller` is the bridge's poke thread at all, matching poke or not. + /// + /// This is what makes an unrecognised poke safe. The bridge never performs a + /// genuine mutation, so a mutation request arriving from its thread is one of + /// its own replays and nothing else. If `claim()` did not recognise it — a + /// bug, a path spelled differently than expected, a syscall the kernel turned + /// into a different operation — the only safe answer is to refuse it. The + /// alternative is forwarding it, and forwarding a create means asking the + /// agent to replace the very file whose arrival prompted the notification + /// with an empty one. + [[nodiscard]] bool is_poke_thread(int caller) const noexcept; + + private: + struct Job { + ChangeKind change = ChangeKind::Unknown; + std::string path; + std::string path2; // move destination + NodeKind kind = NodeKind::File; + bool rescan = false; + }; + + // The single poke in flight. One at a time, so the handlers' test is an exact + // match rather than a search, and a stale record cannot outlive its syscall. + struct Active { + Poke what = Poke::None; + std::string path; + std::string path2; + Pretend pretend_path = Pretend::Nothing; + NodeKind kind = NodeKind::File; + }; + + void run(); + void deliver(const Job& job); + bool poke_create(const Job& job); + bool poke_modify(const Job& job); + bool poke_remove(const Job& job); + bool poke_move(const Job& job); + // True if `rel`'s parent directory is inside the mount. The pokes are real + // filesystem calls; this is what keeps one that resolves outside the mount + // (a path that escapes it, a mount that went away under us) from being + // applied to whatever is there instead. + [[nodiscard]] bool inside_mount(std::string_view rel) const; + [[nodiscard]] std::string abs(std::string_view rel) const; + [[nodiscard]] static long this_tid_or_zero() noexcept; + // Confirms `root_` really is the FUSE mount the pokes are meant for, and + // records its device. Runs on the poke thread; see the comment at its call. + [[nodiscard]] bool verify_root(); + void set_active(Active a); + void clear_active(); + [[nodiscard]] bool is_self(int caller) const noexcept; + + std::string root_; + InvalidatePath invalidate_; // set once, before start() + std::uint64_t root_dev_ = 0; // st_dev of the mount root; pokes never leave it + std::thread thread_; + std::atomic running_{false}; + std::atomic stop_{false}; + std::atomic poke_tid_{0}; + std::atomic self_pid_{0}; + + mutable std::mutex mu_; + std::condition_variable cv_; + std::deque queue_; + + mutable std::mutex active_mu_; + Active active_; + + mutable std::mutex stats_mu_; + Stats stats_; + + // Past this many queued changes the oldest are dropped. A watcher that misses + // one gets a late notification, not a wrong one: the mirror and the page + // cache are already correct, so the file reads right the moment anything + // looks at it. Sized for a burst the size of an `npm install`. + static constexpr std::size_t kQueueCap = 65536; + + // How many failed pokes are named individually before the log goes quiet. + static constexpr std::uint64_t kMaxReportedFailures = 8; +}; + +} // namespace wsld::mount diff --git a/src/mount/fuse_mount.cpp b/src/mount/fuse_mount.cpp index 2a97d48..c60b02b 100644 --- a/src/mount/fuse_mount.cpp +++ b/src/mount/fuse_mount.cpp @@ -16,9 +16,13 @@ using StatvfsT = struct fuse_statvfs; using OffT = fuse_off_t; using ModeT = fuse_mode_t; using TimespecT = struct fuse_timespec; +using DevT = fuse_dev_t; #ifndef S_IFLNK #define S_IFLNK 0120000 #endif +#ifndef S_IFMT +#define S_IFMT 0170000 +#endif #else #define FUSE_USE_VERSION 31 #include @@ -30,6 +34,7 @@ using StatvfsT = struct statvfs; using OffT = off_t; using ModeT = mode_t; using TimespecT = struct timespec; +using DevT = dev_t; #endif #include @@ -52,6 +57,10 @@ namespace { struct Context { agent::RemoteRoot* root; bool writeback = false; + // Non-null only where change notification is on (Linux). The handlers below + // consult it before acting, because a handful of the requests they see are + // the bridge's own and must be answered here rather than forwarded. + FsNotifyBridge* notify = nullptr; }; Context* ctx() { return static_cast(fuse_get_context()->private_data); } @@ -159,8 +168,58 @@ void fill_stat(const MetadataTree::Node& n, StatT* st) { st->st_ctim = st->st_mtim; } +// The change-notification bridge's own requests, if there is one. Each returns +// false the moment notification is off, so the ordinary path is one null check. +int caller_pid() { + const struct fuse_context* c = fuse_get_context(); + return c != nullptr ? static_cast(c->pid) : 0; +} + +enum class PokeVerdict { + NotOurs, // an ordinary request; forward it across the boundary + Claimed, // the poke we are waiting for; answer it here and raise the event + Refuse, // from the bridge's thread but not the poke we expected; see below +}; + +// A mutation request from the bridge's own thread is never forwarded, even when +// it is not the poke that was expected. The bridge issues no genuine mutations, +// so forwarding one could only re-apply a change the far side already made — +// and for a creation that means replacing the file whose arrival prompted the +// notification with an empty one. A refused poke costs a missed event; a +// forwarded one costs data. +PokeVerdict claim_poke(FsNotifyBridge::Poke what, std::string_view rel, std::string_view rel2 = {}) { + FsNotifyBridge* b = ctx()->notify; + if (b == nullptr) return PokeVerdict::NotOurs; + const int caller = caller_pid(); + if (b->claim(caller, what, rel, rel2)) return PokeVerdict::Claimed; + return b->is_poke_thread(caller) ? PokeVerdict::Refuse : PokeVerdict::NotOurs; +} + +// Fills `st` with a minimal stat for a path the mirror no longer has, so the +// bridge's own removal or rename can proceed far enough for the kernel to raise +// the event. Only ever returned to the bridge's thread. +void fill_ghost_stat(NodeKind kind, StatT* st) { + std::memset(st, 0, sizeof(*st)); +#ifndef _WIN32 + st->st_uid = ::getuid(); + st->st_gid = ::getgid(); +#endif + st->st_mode = static_cast(kind == NodeKind::Directory ? (S_IFDIR | 0755) : (S_IFREG | 0644)); + st->st_nlink = kind == NodeKind::Directory ? 2 : 1; +} + int op_getattr(const char* path, StatT* st, struct fuse_file_info*) { const std::string rel = to_rel(path); + if (FsNotifyBridge* b = ctx()->notify; b != nullptr) { + NodeKind kind = NodeKind::File; + switch (b->pretend(caller_pid(), rel, kind)) { + case FsNotifyBridge::Pretend::Absent: return -ENOENT; // so ->mknod / ->mkdir runs + case FsNotifyBridge::Pretend::Present: // so ->unlink / ->rename runs + fill_ghost_stat(kind, st); + return 0; + case FsNotifyBridge::Pretend::Nothing: break; + } + } return ctx()->root->with_tree([&](const MetadataTree& t) -> int { const auto id = t.lookup(rel, LookupMode::CaseInsensitive); if (!id) return -ENOENT; @@ -219,6 +278,17 @@ int err_to_errno(Errc e) { int op_create(const char* path, ModeT mode, struct fuse_file_info* fi) { const std::string rel = to_rel(path); + // A creation poke arrives here, not at op_mknod. libfuse's FUSE_MKNOD handler + // tries `create` first for a regular file and only falls back to `mknod` if + // that answers ENOSYS, so the mount having a `create` means `mknod` is never + // reached for the case the bridge uses. Missing this cost the far side a + // file: the poke was forwarded as a genuine create, which replaces the file + // whose arrival prompted the notification with an empty one. + switch (claim_poke(FsNotifyBridge::Poke::Mknod, rel)) { + case PokeVerdict::Claimed: return 0; // fi->fh stays 0: no write handle, nothing to release + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } auto r = ctx()->root->create_file(rel, static_cast(mode) & 0777u); if (!r) return err_to_errno(r.error()); if (ctx()->writeback && fi != nullptr) @@ -272,23 +342,65 @@ int op_truncate(const char* path, OffT size, struct fuse_file_info*) { return r ? 0 : err_to_errno(r.error()); } +// Not previously implemented, which made `mknod` fail with ENOSYS. It is here +// because the change-notification bridge creates a file through it — the kernel +// routes mknod of a regular file to ->create, which is what raises IN_CREATE — +// but the forwarding path below is a real gain of its own: tools that reach for +// mknod instead of open(O_CREAT) now work on the mount. +int op_mknod(const char* path, ModeT mode, DevT) { + const std::string rel = to_rel(path); + switch (claim_poke(FsNotifyBridge::Poke::Mknod, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } + if ((mode & S_IFMT) != 0 && (mode & S_IFMT) != S_IFREG) return -EPERM; // no devices across the boundary + auto r = ctx()->root->create_file(rel, static_cast(mode) & 0777u); + return r ? 0 : err_to_errno(r.error()); +} + int op_mkdir(const char* path, ModeT mode) { - auto r = ctx()->root->mkdir(to_rel(path), static_cast(mode) & 0777u); + const std::string rel = to_rel(path); + switch (claim_poke(FsNotifyBridge::Poke::Mkdir, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } + auto r = ctx()->root->mkdir(rel, static_cast(mode) & 0777u); return r ? 0 : err_to_errno(r.error()); } int op_unlink(const char* path) { - auto r = ctx()->root->unlink(to_rel(path)); + const std::string rel = to_rel(path); + switch (claim_poke(FsNotifyBridge::Poke::Unlink, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } + auto r = ctx()->root->unlink(rel); return r ? 0 : err_to_errno(r.error()); } int op_rmdir(const char* path) { - auto r = ctx()->root->rmdir(to_rel(path)); + const std::string rel = to_rel(path); + switch (claim_poke(FsNotifyBridge::Poke::Rmdir, rel)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } + auto r = ctx()->root->rmdir(rel); return r ? 0 : err_to_errno(r.error()); } int op_rename(const char* from, const char* to, unsigned int) { - auto r = ctx()->root->rename(to_rel(from), to_rel(to)); + const std::string rfrom = to_rel(from); + const std::string rto = to_rel(to); + switch (claim_poke(FsNotifyBridge::Poke::Rename, rfrom, rto)) { + case PokeVerdict::Claimed: return 0; + case PokeVerdict::Refuse: return -EIO; + case PokeVerdict::NotOurs: break; + } + auto r = ctx()->root->rename(rfrom, rto); return r ? 0 : err_to_errno(r.error()); } @@ -436,6 +548,7 @@ fuse_operations make_ops() { ops.create = op_create; ops.write = op_write; ops.truncate = op_truncate; + ops.mknod = op_mknod; ops.mkdir = op_mkdir; ops.unlink = op_unlink; ops.rmdir = op_rmdir; @@ -454,7 +567,7 @@ fuse_operations make_ops() { FuseMount::~FuseMount() { unmount(); } -Result FuseMount::mount(const std::string& mountpoint, bool writeback) { +Result FuseMount::mount(const std::string& mountpoint, bool writeback, bool notify_changes) { #ifdef _WIN32 if (!load_winfsp_dll()) return fail(Errc::Unsupported); #endif @@ -468,6 +581,10 @@ Result FuseMount::mount(const std::string& mountpoint, bool writeback) { // read-ahead, and - against a case-sensitive agent - the file itself. root_.set_lookup_mode(LookupMode::CaseInsensitive); context.writeback = writeback; + // Wired in before the loop starts so no request can reach a handler while + // this is half-set; the bridge itself is started further down, once there is + // a live mount for it to poke. + context.notify = notify_changes && FsNotifyBridge::supported() ? ¬ify_ : nullptr; static fuse_operations ops = make_ops(); struct fuse_args args = FUSE_ARGS_INIT(0, nullptr); @@ -513,14 +630,21 @@ Result FuseMount::mount(const std::string& mountpoint, bool writeback) { // relative to its own mutation changed nothing, and punching for it would act // on an event already judged obsolete. This also leaves the raw-batch // InvalidationHook free for `wsldrive watch`. - root_.set_invalidated_paths_hook([this](std::span paths) { - std::lock_guard lock(inval_mu_); - if (inval_stop_) return; - for (const std::string& p : paths) { - if (inval_queue_.size() >= kInvalQueueCap) break; - inval_queue_.push_back(to_fuse_path(p)); + root_.set_invalidated_paths_hook([this](std::span changes) { + // Two consumers of the same signal, and they want different things from it. + // The page-cache punch only needs to know which paths moved on; the + // notification bridge needs to know what happened to each of them. + { + std::lock_guard lock(inval_mu_); + if (inval_stop_) return; + for (const auto& c : changes) { + if (c.rescan) continue; // no path to punch; the whole mirror was replaced + if (inval_queue_.size() >= kInvalQueueCap) break; + inval_queue_.push_back(to_fuse_path(c.path)); + } + inval_cv_.notify_one(); } - inval_cv_.notify_one(); + notify_.post(changes); }); loop_ = std::thread([this] { @@ -551,6 +675,20 @@ Result FuseMount::mount(const std::string& mountpoint, bool writeback) { fuse_loop(static_cast(fuse_)); mounted_.store(false); }); + + // Last, because the bridge's first act is to confirm it is pointed at a live + // FUSE mount, and nothing answers a request on this mount until the loop + // above is running. It does that on its own thread, so this does not wait. + if (context.notify != nullptr) { + notify_.set_invalidate([this](const std::string& rel) { + std::lock_guard lock(inval_mu_); + if (inval_stop_ || inval_queue_.size() >= kInvalQueueCap) return; + inval_queue_.push_back(to_fuse_path(rel)); + inval_cv_.notify_one(); + }); + if (auto r = notify_.start(mountpoint); !r) + std::fprintf(stderr, "wsldrive: change notification unavailable on this platform\n"); + } return {}; } @@ -576,6 +714,9 @@ void FuseMount::unmount() { // Stop feeding the invalidation thread, and join it, before the fuse handle // it dereferences goes away. Harmless where the thread was never started. root_.set_invalidated_paths_hook({}); + // The bridge issues filesystem calls against this mount, so it has to be + // stopped and joined before the mount is torn out from under it. + notify_.stop(); { std::lock_guard lock(inval_mu_); inval_stop_ = true; diff --git a/src/mount/fuse_mount.hpp b/src/mount/fuse_mount.hpp index a3c278f..efb4ab4 100644 --- a/src/mount/fuse_mount.hpp +++ b/src/mount/fuse_mount.hpp @@ -2,6 +2,7 @@ #include "agent/client.hpp" #include "core/error.hpp" +#include "mount/fsnotify_bridge.hpp" #include #include @@ -31,7 +32,16 @@ class FuseMount { /// starts serving. Returns once the volume is up. With `writeback`, writes to /// a file are buffered and coalesced, flushed on fsync/flush/release — fewer /// round-trips, at the cost of durability only at flush/close (opt-in). - [[nodiscard]] Result mount(const std::string& mountpoint, bool writeback = false); + /// + /// With `notify_changes`, far-side changes are also delivered to local + /// `inotify` watchers, so watch-mode tools react to edits made on the other + /// side of the boundary (see FsNotifyBridge). Linux only; ignored elsewhere. + [[nodiscard]] Result mount(const std::string& mountpoint, bool writeback = false, + bool notify_changes = true); + + /// Counters for the change-notification bridge; all zero when it is off. + [[nodiscard]] FsNotifyBridge::Stats notify_stats() const { return notify_.stats(); } + [[nodiscard]] bool notifying() const noexcept { return notify_.running(); } /// Signals the FUSE loop to exit, unmounts, and joins the loop thread. void unmount(); @@ -43,6 +53,7 @@ class FuseMount { void inval_loop(); agent::RemoteRoot& root_; + FsNotifyBridge notify_; void* fuse_ = nullptr; // struct fuse* std::string mountpoint_; std::thread loop_; diff --git a/src/platform/linux/dir_watcher.cpp b/src/platform/linux/dir_watcher.cpp index db2d2cd..13c25f1 100644 --- a/src/platform/linux/dir_watcher.cpp +++ b/src/platform/linux/dir_watcher.cpp @@ -156,14 +156,18 @@ class InotifyWatcher final : public Watcher { const std::string rel = join_rel(dir, ev.name); const bool is_dir = (ev.mask & IN_ISDIR) != 0; + // inotify already pairs the two halves of a move with a cookie; pass it + // straight through. It is only set on IN_MOVED_FROM/IN_MOVED_TO. if (ev.mask & (IN_CREATE | IN_MOVED_TO)) { if (is_dir) add_watch_recursive(root_ / std::filesystem::path(rel), rel); - cb_(FsEvent{ev.mask & IN_MOVED_TO ? FsEventKind::RenamedTo : FsEventKind::Created, rel}); + const bool moved = (ev.mask & IN_MOVED_TO) != 0; + cb_(FsEvent{moved ? FsEventKind::RenamedTo : FsEventKind::Created, rel, moved ? ev.cookie : 0}); // A directory may have been populated before we armed the watch; a scan/rescan // on the consumer side covers that. For files, CLOSE_WRITE will follow. } else if (ev.mask & (IN_DELETE | IN_MOVED_FROM)) { if (is_dir) drop_watch_subtree(rel); - cb_(FsEvent{ev.mask & IN_MOVED_FROM ? FsEventKind::RenamedFrom : FsEventKind::Removed, rel}); + const bool moved = (ev.mask & IN_MOVED_FROM) != 0; + cb_(FsEvent{moved ? FsEventKind::RenamedFrom : FsEventKind::Removed, rel, moved ? ev.cookie : 0}); } else if (ev.mask & (IN_MODIFY | IN_CLOSE_WRITE | IN_ATTRIB)) { cb_(FsEvent{FsEventKind::Modified, rel}); } diff --git a/src/platform/win/dir_watcher.cpp b/src/platform/win/dir_watcher.cpp index 135036d..7db315e 100644 --- a/src/platform/win/dir_watcher.cpp +++ b/src/platform/win/dir_watcher.cpp @@ -105,23 +105,47 @@ class Win32Watcher final : public Watcher { for (char& c : path) if (c == '\\') c = '/'; FsEventKind kind; + // ReadDirectoryChangesW has no equivalent of inotify's rename cookie: it + // reports the two halves as OLD_NAME immediately followed by NEW_NAME in + // the same buffer. Mint a cookie on the OLD_NAME and hand the same one to + // the NEW_NAME that follows, so a consumer can pair them the way it pairs + // an inotify move. Anything other than a NEW_NAME clears the pending + // cookie, which leaves a truncated pair unpaired rather than joined to + // the wrong partner. + std::uint32_t cookie = 0; switch (info->Action) { case FILE_ACTION_ADDED: kind = FsEventKind::Created; break; case FILE_ACTION_REMOVED: kind = FsEventKind::Removed; break; case FILE_ACTION_MODIFIED: kind = FsEventKind::Modified; break; - case FILE_ACTION_RENAMED_OLD_NAME: kind = FsEventKind::RenamedFrom; break; - case FILE_ACTION_RENAMED_NEW_NAME: kind = FsEventKind::RenamedTo; break; + case FILE_ACTION_RENAMED_OLD_NAME: + kind = FsEventKind::RenamedFrom; + cookie = next_cookie(); + pending_cookie_ = cookie; + break; + case FILE_ACTION_RENAMED_NEW_NAME: + kind = FsEventKind::RenamedTo; + cookie = pending_cookie_ != 0 ? pending_cookie_ : next_cookie(); + break; default: kind = FsEventKind::Modified; break; } - cb_(FsEvent{kind, path}); + if (info->Action != FILE_ACTION_RENAMED_OLD_NAME) pending_cookie_ = 0; + cb_(FsEvent{kind, path, cookie}); if (info->NextEntryOffset == 0) break; off += info->NextEntryOffset; } } + // Never returns 0: zero is the "no pair" marker downstream. + std::uint32_t next_cookie() noexcept { + if (++cookie_seq_ == 0) ++cookie_seq_; + return cookie_seq_; + } + HANDLE dir_; HANDLE iocp_; WatchCallback cb_; + std::uint32_t cookie_seq_ = 0; + std::uint32_t pending_cookie_ = 0; // cookie of an OLD_NAME awaiting its NEW_NAME OVERLAPPED ov_{}; // Heap-allocated, so `new` gives it stricter alignment than // FILE_NOTIFY_INFORMATION needs. diff --git a/src/tools/wsldrive_main.cpp b/src/tools/wsldrive_main.cpp index 05b2eea..e15dfa2 100644 --- a/src/tools/wsldrive_main.cpp +++ b/src/tools/wsldrive_main.cpp @@ -141,7 +141,8 @@ void usage() { " (check the WinFsp + WSL environment)\n" #endif #ifdef WSLDRIVE_HAVE_MOUNT - " wsldrive mount --connect [--writeback] [--no-prefetch] (attach to an agent)\n" + " wsldrive mount --connect [--writeback] [--no-prefetch] [--no-inotify]\n" + " (attach to an agent)\n" #endif #if defined(WSLDRIVE_HAVE_MOUNT) && !defined(_WIN32) " wsldrive mount --win-root --win-agent [--hvsocket [--vm-guid G]]\n" @@ -420,6 +421,7 @@ int main(int argc, char** argv) { bool writeback = false; bool hvsocket = false; bool prefetch = true; + bool notify_changes = true; std::string vm_guid; for (int i = 3; i < argc; ++i) { const std::string_view a = argv[i]; @@ -440,6 +442,8 @@ int main(int argc, char** argv) { writeback = true; else if (a == "--no-prefetch") prefetch = false; + else if (a == "--no-inotify") + notify_changes = false; else if (a == "--distro") { distro = val(); have_distro = true; @@ -649,12 +653,14 @@ int main(int argc, char** argv) { if (dirs > 0) std::printf("prefetching %zu directories in the background...\n", dirs); } wsld::mount::FuseMount fm(root); - if (auto r = fm.mount(mountpoint, writeback); !r) { + if (auto r = fm.mount(mountpoint, writeback, notify_changes); !r) { std::fprintf(stderr, "wsldrive: mount failed: %s\n", wsld::to_string(r.error())); return 1; } install_mount_signal_handler(); std::printf("mounted. Ctrl+C to unmount.\n"); + if (notify_changes && wsld::mount::FsNotifyBridge::supported()) + std::printf("far-side changes are delivered to local inotify watchers (--no-inotify turns this off)\n"); std::fflush(stdout); std::uint64_t rescans_seen = 0, rescan_failures_seen = 0; while (fm.mounted() && root.connected() && !g_mount_stop.load()) { @@ -670,6 +676,14 @@ int main(int argc, char** argv) { static_cast(st.rescans), static_cast(st.rescan_failures)); } } + // Always reported, not only on loss. Change delivery is invisible when it + // works and equally invisible when it does not, so a count of what actually + // went out is the only way to tell the two apart after the fact. + if (const auto ns = fm.notify_stats(); ns.events != 0) + std::printf("change notifications: %llu delivered, %llu failed, %llu dropped, %llu rescans (of %llu)\n", + static_cast(ns.delivered), static_cast(ns.failed), + static_cast(ns.dropped), static_cast(ns.rescans), + static_cast(ns.events)); std::printf("unmounting...\n"); std::fflush(stdout); fm.unmount(); // agent (if auto-launched) is stopped by its destructor on return @@ -821,9 +835,24 @@ int main(int argc, char** argv) { std::printf("[gen %llu +%lld ms] %zu ops\n", static_cast(b.generation), static_cast(std::chrono::duration_cast(now).count() % 100000), b.ops.size()); - for (const auto& op : b.ops) - std::printf(" %s %s\n", op.kind == wsld::InvalidationKind::Upsert ? "upsert" : op.kind == wsld::InvalidationKind::Remove ? "remove" : "rescan", - op.path.c_str()); + for (const auto& op : b.ops) { + const char* change = ""; + switch (op.change) { + case wsld::ChangeKind::Created: change = " (created)"; break; + case wsld::ChangeKind::Modified: change = " (modified)"; break; + case wsld::ChangeKind::Removed: change = " (removed)"; break; + case wsld::ChangeKind::MovedFrom: change = " (moved from)"; break; + case wsld::ChangeKind::MovedTo: change = " (moved to)"; break; + case wsld::ChangeKind::Unknown: break; + } + char cookie[32] = ""; + if (op.cookie != 0) std::snprintf(cookie, sizeof(cookie), " #%u", op.cookie); + std::printf(" %s %s%s%s\n", + op.kind == wsld::InvalidationKind::Upsert ? "upsert" + : op.kind == wsld::InvalidationKind::Remove ? "remove" + : "rescan", + op.path.c_str(), change, cookie); + } std::fflush(stdout); }); while (root.connected()) std::this_thread::sleep_for(std::chrono::milliseconds(200)); diff --git a/tests/agent_test.cpp b/tests/agent_test.cpp index 77cfda9..404b424 100644 --- a/tests/agent_test.cpp +++ b/tests/agent_test.cpp @@ -990,6 +990,145 @@ TEST_F(AgentTest, LiveInvalidationsReachTheClient) { EXPECT_GT(client->stats().generation, 1u); } +// The change kind is what a notification consumer keys off, and it is decided +// in three places: the watcher names the event, the agent re-stats the path and +// may overrule it, and the client compares against its own mirror to settle +// created-versus-modified. This drives events in directly (RootServer::notify) +// rather than through a platform watcher, so it exercises the whole chain +// wherever the suite runs. +TEST_F(AgentTest, ChangeKindsReachTheClient) { + LoopbackServer srv(root_, /*watch=*/false); + auto client = connect_client(srv.endpoint()); + ASSERT_NE(client, nullptr); + ASSERT_TRUE(client->connect().has_value()); + ASSERT_TRUE(client->fetch_snapshot().has_value()); + + std::mutex mu; + std::condition_variable cv; + std::vector seen; + client->set_invalidated_paths_hook([&](std::span changes) { + std::lock_guard lock(mu); + seen.insert(seen.end(), changes.begin(), changes.end()); + cv.notify_all(); + }); + auto wait_for = [&](auto pred) { + std::unique_lock lock(mu); + return cv.wait_for(lock, 10s, [&] { return pred(seen); }); + }; + auto change_of = [&](std::string_view path) { + std::lock_guard lock(mu); + for (const auto& c : seen) + if (c.path == path) return c.change; + return ChangeKind::Unknown; + }; + auto forget = [&] { + std::lock_guard lock(mu); + seen.clear(); + }; + auto saw = [&](std::string_view path) { + return [path](const auto& v) { + for (const auto& c : v) + if (c.path == path) return true; + return false; + }; + }; + + // A path the mirror does not have yet is a creation. + write_file(root_ / "src" / "fresh.cpp", "// fresh\n"); + srv.server().notify(FsEvent{FsEventKind::Created, "src/fresh.cpp"}); + ASSERT_TRUE(wait_for(saw("src/fresh.cpp"))); + EXPECT_EQ(change_of("src/fresh.cpp"), ChangeKind::Created); + + // The same path again, now that the mirror has it, is a modification — even + // though the agent reports exactly the same thing about it either time. + forget(); + write_file(root_ / "src" / "fresh.cpp", "// fresher\n"); + srv.server().notify(FsEvent{FsEventKind::Modified, "src/fresh.cpp"}); + ASSERT_TRUE(wait_for(saw("src/fresh.cpp"))); + EXPECT_EQ(change_of("src/fresh.cpp"), ChangeKind::Modified); + + // A rename keeps both halves and the cookie that ties them together. + forget(); + fs::rename(root_ / "src" / "fresh.cpp", root_ / "src" / "renamed.cpp"); + srv.server().notify(FsEvent{FsEventKind::RenamedFrom, "src/fresh.cpp", 1234}); + srv.server().notify(FsEvent{FsEventKind::RenamedTo, "src/renamed.cpp", 1234}); + ASSERT_TRUE(wait_for([](const auto& v) { + bool from = false, to = false; + for (const auto& c : v) { + from |= c.change == ChangeKind::MovedFrom; + to |= c.change == ChangeKind::MovedTo; + } + return from && to; + })); + { + std::lock_guard lock(mu); + std::uint32_t from_cookie = 0, to_cookie = 0; + for (const auto& c : seen) { + if (c.change == ChangeKind::MovedFrom) { + EXPECT_EQ(c.path, "src/fresh.cpp"); + from_cookie = c.cookie; + } + if (c.change == ChangeKind::MovedTo) { + EXPECT_EQ(c.path, "src/renamed.cpp"); + to_cookie = c.cookie; + } + } + EXPECT_NE(from_cookie, 0u); + EXPECT_EQ(from_cookie, to_cookie); + } + + // A deletion names what was removed, and says whether it was a directory — + // the mirror is the only side that still knows, since the path is gone from + // the disk by the time anything asks. + forget(); + fs::remove_all(root_ / "Docs"); + srv.server().notify(FsEvent{FsEventKind::Removed, "Docs"}); + ASSERT_TRUE(wait_for(saw("Docs"))); + EXPECT_EQ(change_of("Docs"), ChangeKind::Removed); + { + std::lock_guard lock(mu); + for (const auto& c : seen) { + if (c.path == "Docs") { + EXPECT_EQ(c.kind, NodeKind::Directory); + } + } + } +} + +// A removal for a path the mirror never had changed nothing, so reporting it +// would tell a consumer a file it never saw had just been deleted. +TEST_F(AgentTest, RemovalOfAnUnknownPathIsNotReported) { + LoopbackServer srv(root_, /*watch=*/false); + auto client = connect_client(srv.endpoint()); + ASSERT_NE(client, nullptr); + ASSERT_TRUE(client->connect().has_value()); + ASSERT_TRUE(client->fetch_snapshot().has_value()); + + std::mutex mu; + std::condition_variable cv; + std::vector seen; + int batches = 0; + client->set_invalidated_paths_hook([&](std::span changes) { + std::lock_guard lock(mu); + seen.insert(seen.end(), changes.begin(), changes.end()); + cv.notify_all(); + }); + client->set_invalidation_hook([&](const proto::InvalidationBatch&) { + std::lock_guard lock(mu); + ++batches; + cv.notify_all(); + }); + + srv.server().notify(FsEvent{FsEventKind::Removed, "never-existed.txt"}); + // Wait on the raw batch, which does arrive, then check that nothing was + // reported as applied: the two hooks differ exactly here. + { + std::unique_lock lock(mu); + ASSERT_TRUE(cv.wait_for(lock, 10s, [&] { return batches > 0; })); + EXPECT_TRUE(seen.empty()); + } +} + TEST_F(AgentTest, RescanRefetchesTheSnapshot) { // When the agent's watcher overflows it sends a single Rescan instead of the // events it lost. Ignoring it left the mirror stale until remount — exactly in diff --git a/tests/coalescer_test.cpp b/tests/coalescer_test.cpp index e14a419..dfced28 100644 --- a/tests/coalescer_test.cpp +++ b/tests/coalescer_test.cpp @@ -140,5 +140,127 @@ TEST(Coalescer, MarksEntriesThatAppeared) { EXPECT_TRUE(find("gone").appeared); } +// --- change kinds and rename pairing (the inotify bridge's input) ------------ + +// Every consumer downstream keys off `change`, and the whole point of keeping +// it separate from `kind` is that a burst collapses to one op without losing +// what the burst was. +TEST(Coalescer, ChangeKindSurvivesCollapsing) { + Coalescer c; + c.push({FsEventKind::Created, "new.txt"}, t0); + c.push({FsEventKind::Modified, "new.txt"}, t0); // still a creation, not a modification + c.push({FsEventKind::Modified, "old.txt"}, t0); + c.push({FsEventKind::Created, "gone.txt"}, t0); + c.push({FsEventKind::Removed, "gone.txt"}, t0); // last event wins + + auto out = c.take(); + auto change_of = [&](std::string_view p) { + for (const auto& op : out) + if (op.path == p) return op.change; + return ChangeKind::Unknown; + }; + EXPECT_EQ(change_of("new.txt"), ChangeKind::Created); + EXPECT_EQ(change_of("old.txt"), ChangeKind::Modified); + EXPECT_EQ(change_of("gone.txt"), ChangeKind::Removed); +} + +TEST(Coalescer, RenamePairSharesACookie) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "old.txt", 77}, t0); + c.push({FsEventKind::RenamedTo, "new.txt", 77}, t0); + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + EXPECT_EQ(out[0].change, ChangeKind::MovedFrom); + EXPECT_EQ(out[0].path, "old.txt"); + EXPECT_EQ(out[1].change, ChangeKind::MovedTo); + EXPECT_EQ(out[1].path, "new.txt"); + EXPECT_EQ(out[0].cookie, 77u); + EXPECT_EQ(out[1].cookie, 77u); +} + +// A write to the destination after the move must not cost the pairing: the +// consumer still has to see one move rather than a delete and an unrelated +// write. This is the git lock-file dance (rename into place, then touch). +TEST(Coalescer, WriteAfterMoveKeepsThePair) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "a", 5}, t0); + c.push({FsEventKind::RenamedTo, "b", 5}, t0); + c.push({FsEventKind::Modified, "b"}, t0); + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + EXPECT_EQ(out[0].change, ChangeKind::MovedFrom); + EXPECT_EQ(out[1].change, ChangeKind::MovedTo); + EXPECT_EQ(out[1].cookie, 5u); +} + +// The source coming back under its old name is exactly what git does, and it +// leaves the destination's half with nobody to pair with. A half-move must not +// reach a consumer: on its own the destination simply appeared. +TEST(Coalescer, StrandedMoveHalfDegrades) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "config", 9}, t0); + c.push({FsEventKind::RenamedTo, "config.lock", 9}, t0); + c.push({FsEventKind::Created, "config", 0}, t0); // recreated, so it is no longer a move source + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + for (const auto& op : out) { + EXPECT_FALSE(is_move(op.change)) << op.path; + EXPECT_EQ(op.cookie, 0u) << op.path; + EXPECT_EQ(op.change, ChangeKind::Created) << op.path; + } +} + +// The other direction: a removed directory takes the destination out of the +// batch, so the source is left as the surviving half. +TEST(Coalescer, MoveIntoARemovedDirectoryDegradesToRemoval) { + Coalescer c; + c.push({FsEventKind::RenamedFrom, "src.txt", 3}, t0); + c.push({FsEventKind::RenamedTo, "dir/dst.txt", 3}, t0); + c.push({FsEventKind::Removed, "dir"}, t0); // drops dir/dst.txt with it + + auto out = c.take(); + ASSERT_EQ(out.size(), 2u); + auto change_of = [&](std::string_view p) { + for (const auto& op : out) + if (op.path == p) return op.change; + return ChangeKind::Unknown; + }; + EXPECT_EQ(change_of("src.txt"), ChangeKind::Removed); + EXPECT_EQ(change_of("dir"), ChangeKind::Removed); +} + +TEST(Coalescer, OverflowCarriesNoChangeKind) { + Coalescer c; + c.push({FsEventKind::Created, "a"}, t0); + c.push({FsEventKind::Overflow, {}}, t0); + auto out = c.take(); + ASSERT_EQ(out.size(), 1u); + EXPECT_EQ(out[0].kind, InvalidationKind::Rescan); + EXPECT_EQ(out[0].change, ChangeKind::Unknown); +} + +// repair_move_pairs is applied again by the sender and by the receiver, on +// their own op types, so it is exercised directly here too. +TEST(RepairMovePairs, MatchedPairSurvivesAndOrphansDegrade) { + std::vector ops{ + PlannedOp{InvalidationKind::Remove, "a", false, ChangeKind::MovedFrom, 1}, + PlannedOp{InvalidationKind::Upsert, "b", true, ChangeKind::MovedTo, 1}, + PlannedOp{InvalidationKind::Upsert, "c", true, ChangeKind::MovedTo, 2}, // no partner + PlannedOp{InvalidationKind::Remove, "d", false, ChangeKind::MovedFrom, 3}, // no partner + PlannedOp{InvalidationKind::Upsert, "e", true, ChangeKind::MovedTo, 0}, // no cookie at all + }; + repair_move_pairs(ops); + EXPECT_EQ(ops[0].change, ChangeKind::MovedFrom); + EXPECT_EQ(ops[1].change, ChangeKind::MovedTo); + EXPECT_EQ(ops[2].change, ChangeKind::Created); + EXPECT_EQ(ops[3].change, ChangeKind::Removed); + EXPECT_EQ(ops[4].change, ChangeKind::Created); + EXPECT_EQ(ops[2].cookie, 0u); + EXPECT_EQ(ops[3].cookie, 0u); +} + } // namespace } // namespace wsld diff --git a/tests/protocol_test.cpp b/tests/protocol_test.cpp index 7aeee85..41ae309 100644 --- a/tests/protocol_test.cpp +++ b/tests/protocol_test.cpp @@ -198,6 +198,52 @@ TEST(Messages, InvalidationRoundTrip) { EXPECT_EQ(got->ops[1].path, "gone"); EXPECT_EQ(got->ops[2].kind, InvalidationKind::Rescan); EXPECT_TRUE(r.empty()); + for (const auto& op : got->ops) { + EXPECT_EQ(op.change, ChangeKind::Unknown); // default when the sender says nothing + EXPECT_EQ(op.cookie, 0u); + } +} + +TEST(Messages, InvalidationCarriesChangeKindAndMoveCookie) { + InvalidationBatch b; + b.generation = 7; + b.ops.push_back(InvalidationOp{InvalidationKind::Upsert, "made", kFile, ChangeKind::Created, 0}); + b.ops.push_back(InvalidationOp{InvalidationKind::Upsert, "written", kFile, ChangeKind::Modified, 0}); + b.ops.push_back(InvalidationOp{InvalidationKind::Remove, "old", {}, ChangeKind::MovedFrom, 4242}); + b.ops.push_back(InvalidationOp{InvalidationKind::Upsert, "new", kFile, ChangeKind::MovedTo, 4242}); + b.ops.push_back(InvalidationOp{InvalidationKind::Remove, "deleted", {}, ChangeKind::Removed, 0}); + + std::vector buf; + Writer w(buf); + write_invalidation(w, b); + Reader r(buf); + auto got = read_invalidation(r); + ASSERT_TRUE(got.has_value()); + ASSERT_EQ(got->ops.size(), 5u); + EXPECT_EQ(got->ops[0].change, ChangeKind::Created); + EXPECT_EQ(got->ops[1].change, ChangeKind::Modified); + EXPECT_EQ(got->ops[2].change, ChangeKind::MovedFrom); + EXPECT_EQ(got->ops[3].change, ChangeKind::MovedTo); + EXPECT_EQ(got->ops[4].change, ChangeKind::Removed); + EXPECT_EQ(got->ops[2].cookie, 4242u); + EXPECT_EQ(got->ops[3].cookie, 4242u); + // The cookie rides along only for the two move kinds, so a batch without a + // move costs exactly one byte per op more than it used to. + EXPECT_EQ(got->ops[0].cookie, 0u); + EXPECT_EQ(got->ops[4].cookie, 0u); + EXPECT_TRUE(r.empty()); +} + +TEST(Messages, InvalidationRejectsAnUnknownChangeKind) { + std::vector buf; + Writer w(buf); + w.u64(1); // generation + w.varint(1); // one op + w.u8(static_cast(InvalidationKind::Remove)); + w.string("p"); + w.u8(200); // not a ChangeKind + Reader r(buf); + EXPECT_EQ(read_invalidation(r).error(), Errc::Corrupt); } TEST(Messages, HugeClaimedCountsFailWithoutHugeAllocation) {