fix(rspack_watcher): keep mtime baseline persistent across watch() cycles#14077
Conversation
…cles The previous mtime gate was correct in concept but had a fatal race in its lifecycle handling. `PathManager::reset()` cleared `file_mtimes` and `wait_for_event()` re-snapshotted every registered file's current mtime as the new baseline -- on EVERY `FsWatcher::watch()` call, which rspack invokes on every rebuild cycle (aggregate -> pause -> rebuild -> rewatch). On a slow runner (observed on GHA macos-14 + node 24) FSEvents queues the user-triggered Change event, rspack re-enters watch() in the gap, and the snapshot captures the post-write mtime. When the queued FSEvent finally lands, `has_mtime_changed` sees `current_mtime == baseline` and silently suppresses the event, producing rstest's "No test files need re-run" failure on PR #1261. Adopt the watchpack model: mtime baselines are lifetime-tied to registered files, not to individual watch() calls. * `PathManager::reset()` no longer clears `file_mtimes`; only the per-cycle added/removed diff sets are reset. * `PathManager::update()` now removes mtime entries for files that have been unregistered this cycle, so the map cannot grow unboundedly across rebuilds. * `set_file_mtime` is replaced with `set_file_mtime_if_absent` (entry-or-insert), and `wait_for_event` calls `record_initial_file_mtimes` which only stats files that have no baseline yet. Existing baselines are advanced atomically by `has_mtime_changed` itself when a real change is observed. This keeps the original "filter stale FSEvent replays" semantics for the first watch() call after registration -- exactly when stale replays from the per-path ring buffer can fire -- while making subsequent watch() calls a no-op for already-tracked files, so an in-flight user write can never be captured as a baseline. Verified locally on macos-14 node24 against the failing watch e2e from rstest PR #1261.
📦 Binary Size-limit
🙈 Size remains the same at 61.75MB |
Rsdoctor Bundle Diff AnalysisFound 5 projects in monorepo, 5 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 popular-libsPath:
📁 react-10kPath:
📁 react-5kPath:
📁 react-1kPath:
📁 ui-componentsPath:
Generated by Rsdoctor GitHub Action |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts rspack_watcher’s mtime baseline lifecycle so registered files keep their baseline across repeated FsWatcher::watch() calls, preventing delayed macOS FSEvents from being incorrectly suppressed after rebuild/rewatch cycles.
Changes:
- Keeps
file_mtimesacrossPathManager::reset()and prunes entries only when files are removed. - Replaces unconditional mtime insertion with
set_file_mtime_if_absent. - Renames and documents initial mtime recording in
FsWatcher.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/rspack_watcher/src/paths.rs |
Makes mtime baselines persistent across watch cycles and cleans them up for removed files. |
crates/rspack_watcher/src/lib.rs |
Updates initial mtime recording to preserve existing baselines across rewatch cycles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Deploying rspack with
|
| Latest commit: |
fad4f00
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d7d6bb68.rspack-v2.pages.dev |
| Branch Preview URL: | https://fix-fsevents-mtime-race.rspack-v2.pages.dev |
…ycles
Regression for the FSEvents stale-event race fixed in this PR. Walks
the lifecycle a single `PathManager` sees during rspack's
aggregate -> pause -> rebuild -> rewatch loop, with a real file write
landing between the two `watch()` calls -- the slow-runner case where
the FSEvent is queued in the kernel but not yet delivered when the
second cycle starts.
Asserts the three invariants the fix introduces:
1. `file_mtimes` survives `reset()`.
2. `set_file_mtime_if_absent` is a no-op for an already-baselined
path, so the post-write mtime can't overwrite the original.
3. The delayed change event is therefore not suppressed by
`has_mtime_changed`, and the baseline advances atomically.
…ile_mtimes Iterate `files.added` instead of `files.all` so the loop becomes a no-op in steady state (no add/remove churn this cycle) instead of issuing O(N) `metadata()` syscalls on every rebuild rewatch. The previous behavior was correct (`set_file_mtime_if_absent` discarded the post-write mtime via entry-or-insert) but expensive on large projects, and contradicted the PR's stated invariant "only stat files that do NOT already have a baseline". Side effect: this also strengthens the race fix. Post-write mtimes no longer reach `set_file_mtime_if_absent` for persisted files at all, so the FSEvents stale-event race remains closed even if the or-insert contract is accidentally weakened in the future. Addresses review feedback on #14077.
Picks up the final fix for the FSEvents mtime baseline race + the Option 2 perf follow-up (record_initial_file_mtimes now only stats files added in this watch cycle, not the full registered set). Tracks web-infra-dev/rspack#14077.
Why
On slow macOS runners,
FsWatcher(used byexperiments.nativeWatcher) silently drops a real user edit between rebuild cycles. Symptom:No test files need re-runwith stale output. Observed deterministically in rstest #1261 onmacos-14 + node 24.Root cause:
PathManager::reset()wipedfile_mtimesandwait_for_event()re-stat'd every registered file on everyFsWatcher::watch()call. If the user's write landed between aggregate and rewatch — possible because FSEvents delivery is async — the re-stat captured the post-write mtime as the new baseline. The in-flight FSEvent then arrived to findcurrent == baselineand was suppressed as a duplicate.What
Tie mtime baselines to the file's lifetime, not to a single
watch()call (watchpack model).reset()no longer clearsfile_mtimes; only the per-cycleadded/removeddiff is reset.update()prunes mtime entries for files unregistered this cycle so the map cannot grow unboundedly.record_initial_file_mtimesiteratesfiles.added(this cycle's new files) instead offiles.all. Steady-state rewatch now issues 0 stats instead of O(N).Test plan
test_baseline_persists_across_consecutive_watch_cyclescovers reset → post-write → delayed-event-not-suppressed.