node: carry the stat watcher refs as RefPtr values

Closed
RO
3mo ago
+202-246

Problem

  • Nothing user-visible breaks. This is a lifetime cleanup of src/runtime/node/node_fs_stat_watcher.rs, the fs.watchFile implementation.
  • Three holders keep a StatWatcher alive: the scheduler's queue, the initial-stat work-pool task, and every hop the pool posts to the JS thread. Each ref was a hand-written ref_() with a deref() (or a RefPtr::from_raw guard) at the other end, seven pairs over the file. The types did not say who held what.

Fix

  • Each holder carries its ref as a RefPtr<StatWatcher>. A private WatcherQueue wraps the intrusive queue: push takes the RefPtr, pop_batch yields them back, WatcherBatch drains on drop. InitialStatTask.watcher is a RefPtr. post_to_js_thread moves one into the posted task and take_hop_ref takes it back.
  • Correct because every ref is still taken and released at the same points, on the same threads, in the same order relative to the ticket that holds the VM for the task.
  • Verified: test/js/node/watch/fs.watchFile.test.ts, fs.watch*.test.ts, and Node's test-fs-watchfile*.js under the debug build. Windows cargo check and clippy are clean.

Background

  • StatWatcher is the native object behind one fs.watchFile call. It is intrusively ref-counted: the count lives inside the object, and the last holder to release frees it.
  • RefPtr<T> (bun_ptr, since #40478) is one counted ref held as a value: new takes the first ref, clone another, drop releases, and into_raw/from_raw move a ref across a raw-pointer boundary without touching the count.
  • StatWatcherScheduler is a per-VM singleton. It keeps queued watchers in an intrusive queue, stats them on the work pool every interval, and posts a hop to the JS thread for each changed file.
Notes

This PR was first stacked on #37665, which added an OwnedRef<T> type for this purpose. #40478 gave RefPtr<T> that role (release on drop, Clone, into_raw/from_raw) and folded the file's WatcherRefGuard/SchedulerRefGuard newtypes into RefPtr::from_raw guards. This PR was redone on top of main with RefPtr in place of OwnedRef. The guards #40478 left at the hop sites are what the values replace here.

StatWatcher::ref_/deref, StatWatcherScheduler::deref, release_hop and as_ctx_ptr go away. finalize uses the shared bun_ptr::finalize_js_box. Net 44 lines and 7 unsafe blocks fewer.

The one into_raw/from_raw pair the intrusive queue needs lives in WatcherQueue::push and WatcherBatch::next, with the invariant stated once. The hop's ref is now released when run_hop returns instead of at the end of each continuation, the next statement on the same thread.

Two things are deliberately unchanged: the scheduler's own ref across its work-pool hop still uses RefPtr::from_raw in work_pool_callback (the task node is intrusive, so there is no value to carry the ref), and each watcher's RefPtr<StatWatcherScheduler> is still released from finalize.

InitialStatTask::run_owned declares the watcher after the ticket so the closed path drops the ref before the ticket lets the VM go, the order the explicit deref had.

Test runs: fs.watchFile.test.ts 10 pass, 2 skip. fs.watch.test.ts, fs.watch.close-exit.test.ts, fs.watch.deadlock.test.ts 47 pass, 7 skip. test-fs-watchfile.js, test-fs-watchfile-bigint.js, test-fs-watchfile-ref-unref.js exit 0.

Original description

Stacked on #37665 (the base of this PR is that branch). The first version of this PR replaced the two hand-rolled ref guards in src/runtime/node/node_fs_stat_watcher.rs with ScopedRef; review asked whether there was something cleaner than guards, and this version is the answer: the refs themselves become values.

fs.watchFile keeps each StatWatcher alive through several intrusive refs: the scheduler's queue holds one per watcher, the initial-stat work-pool task holds one, and every hop posted from the pool to the JS thread carries one. Until now all of them were ref_() calls paired with deref() calls (or guards adopting them) at the other end, seven pairs spread over the file. Now each of them is an OwnedRef<StatWatcher> held by the thing that owns it.