Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6812,19 +6812,24 @@ pub mod bv2_impl {
.text;
let loader = this.graph.input_files.items_loader()[source_index as usize];
if this.should_add_watcher(source_path) {
// const generic `CLONE_FILE_PATH = isWindows`
// matches `cfg!(windows)` at compile time.
let _ = this
.bun_watcher_mut()
.unwrap()
.add_file::<{ cfg!(windows) }>(
parse_result.watcher_data.fd,
source_path,
bun_wyhash::hash(source_path) as u32,
bun_watcher::Loader(loader as u8),
parse_result.watcher_data.dir_fd,
None,
);
// CLONE_FILE_PATH=true: `source_path` is only interned
// process-lifetime in the common case. `dupe_alloc`'s
// disjoint text/pretty branch (imports outside the
// project root, bake's `ssr:`-prefixed pretty paths)
// allocates `path.text` in the per-build arena
// (`graph.heap`), which DevServer frees at the end of
// every build — while the watcher keeps the path for
// the process lifetime and reads it on the watcher
// thread. Borrowing here (Zig passed
// `Environment.isWindows`) dangled those entries.
let _ = this.bun_watcher_mut().unwrap().add_file::<true>(
parse_result.watcher_data.fd,
source_path,
bun_wyhash::hash(source_path) as u32,
bun_watcher::Loader(loader as u8),
parse_result.watcher_data.dir_fd,
None,
);
}
}
}
Expand Down
26 changes: 22 additions & 4 deletions src/runtime/node/node_fs_stat_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,11 +993,29 @@ impl StatWatcher {
// (`FileSystem::init` runs before any JS module loads).
let top_level_dir = fs::FileSystem::get().top_level_dir;
let parts: [&[u8]; 1] = [slice];
let file_path =
Path::join_abs_string_buf::<platform::Auto>(top_level_dir, &mut buf[..], &parts);

// The cwd-joined result can exceed the pooled buffer even when the raw
// path passed per-platform length validation; the unchecked join
// overflowed the buffer (panic) on such inputs. Fall back to a heap
// scratch sized like `join_abs_string_buf_checked`'s slow path: the
// watcher is still created and its stat polls fail with ENAMETOOLONG,
// so the listener observes zeroed stats — matching Node, which never
// throws for un-stat-able `watchFile` paths.
// allocSentinel + memcpy → owned NUL-terminated copy (ZBox)
let alloc_file_path = ZBox::from_bytes(file_path);
let alloc_file_path = match Path::join_abs_string_buf_checked::<platform::Auto>(
top_level_dir,
&mut buf[..],
&parts,
) {
Some(file_path) => ZBox::from_bytes(file_path),
None => {
let mut scratch = vec![0u8; top_level_dir.len() + slice.len() + 3];
ZBox::from_bytes(Path::join_abs_string_buf::<platform::Auto>(
top_level_dir,
&mut scratch,
&parts,
))
}
};
// errdefer free → Drop handles it

// `args.global_this` is a `BackRef` (JSC_BORROW); safe Deref.
Expand Down
50 changes: 16 additions & 34 deletions src/watcher/KEventWatcher.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use core::ffi::c_int;

use bun_core::output as Output;
use bun_sys::Fd;

Expand Down Expand Up @@ -58,7 +56,10 @@ pub(crate) fn watch_event_from_kevent(kevent: &libc::kevent) -> WatchEvent {
}

pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> {
use bun_sys::c;
// Zig: `defer Output.flush()` — flushes on the `?` early returns below
// too (mirrors INotifyWatcher's watch_loop_cycle).
let _flush = Output::flush_guard();

let fd: Fd = this
.platform
.fd
Expand All @@ -69,42 +70,26 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> {
let mut changelist_array: [libc::kevent; CHANGELIST_COUNT] = bun_core::ffi::zeroed();
let changelist = &mut changelist_array;

// SAFETY: fd is a valid kqueue fd; changelist points to CHANGELIST_COUNT zeroed entries
let mut count: c_int = unsafe {
c::kevent(
fd.native(),
changelist.as_ptr(),
0,
changelist.as_mut_ptr(),
CHANGELIST_COUNT as c_int,
core::ptr::null(), // timeout
)
};
// kevent(2) returns -1 on failure (e.g. EINTR). Zig called the raw syscall
// and let a negative count fall through `@max(0, count)` into an empty
// batch; the direct port's checked `usize::try_from(count)` panicked on
// that -1 instead. Use the EINTR-retrying `bun_sys::kevent` wrapper and
// propagate real errors to the watch loop.
let mut count: usize = bun_sys::kevent(fd, &[], changelist, None)?;
Comment thread
robobun marked this conversation as resolved.

// Give the events more time to coalesce
if count < 128 / 2 {
let remain: c_int = 128 - count;
let off = usize::try_from(count).expect("int cast");
if count < CHANGELIST_COUNT / 2 {
let ts = libc::timespec {
tv_sec: 0,
tv_nsec: 100_000,
}; // 0.0001 seconds
// SAFETY: off < CHANGELIST_COUNT (count < 64), remain entries fit in the buffer
let extra: c_int = unsafe {
c::kevent(
fd.native(),
changelist.as_ptr().add(off),
0,
changelist.as_mut_ptr().add(off),
remain,
&raw const ts,
)
};

count += extra;
// Best-effort coalescing read: deliver the events we already have even
// if this extra poll fails; a persistent kqueue error resurfaces on the
// next cycle's blocking call.
count += bun_sys::kevent(fd, &[], &mut changelist[count..], Some(&ts)).unwrap_or(0);
}

let changes_len = usize::try_from(count.max(0)).expect("int cast");
let changes_len = count;
let changes = &changelist[0..changes_len];
// Track out_len and slice once at the end to avoid overlapping &mut
// borrows of `this`.
Expand Down Expand Up @@ -140,8 +125,5 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> {
(this.on_file_update)(this.ctx, &mut deduped, changed, &this.watchlist);
}

// No early returns above, so flush once at the single exit point instead
// of via scopeguard.
Output::flush();
Ok(())
}
36 changes: 33 additions & 3 deletions src/watcher/WindowsWatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,19 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> {
// NOTE: using a 1ms timeout would be ideal, but that actually makes the thread wait for at least 10ms more than it should
// Instead we use a 0ms timeout, which may not do as much coalescing but is more responsive.
timeout = Timeout::None;
// PORT NOTE: locked — diverges from Zig spec (which scanned the
// `file_path` column unlocked; `WindowsWatcher.zig:217`). The JS
// thread, transpiler workers and the bundle thread append watch items
// under `this.mutex`, which can grow the MultiArrayList — freeing the
// old backing slab mid-scan — so the unlocked `items_file_path()` read
// raced the realloc and `is_parent_or_equal` dereferenced freed
// memory. Mirrors the locked snapshot in INotifyWatcher's
// `watch_loop_cycle`. The guard is dropped and re-acquired around
// `process_watch_event_batch`, which takes the same (non-recursive)
// mutex internally. This never holds the lock across a blocking wait:
// `platform.next()` runs before the guard is taken and `iter.next()`
// only parses the already-filled completion buffer.
let mut guard = this.mutex.lock_guard();
bun_core::scoped_log!(
watcher,
"number of watched items: {}",
Expand Down Expand Up @@ -439,8 +452,18 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> {
// to implement and maintain.
// - others that i'm not thinking of

let n_items = this.watchlist.items_file_path().len();
for item_idx in 0..n_items {
// The length is re-read every iteration: releasing the lock around
// `process_watch_event_batch` lets `on_file_update` evict entries
// and compact the watchlist. Known trade-off: that compaction is
// `swap_remove`-based, so a mid-batch flush can move a not-yet-
// scanned tail entry into a slot behind `item_idx`, skipping it
// for the *current* OS event (requires 128+ matches for one event
// plus a concurrent eviction below the cursor). Rescanning from 0
// instead would risk duplicate notifications; a missed coalesced
// event is the safer failure mode, and the next event for that
// path re-delivers.
let mut item_idx: usize = 0;
while item_idx < this.watchlist.items_file_path().len() {
// reshaped for borrowck — `rel` is computed in a scoped
// block so the borrows of `this.watchlist` / `this.platform.buf`
// are released before we touch `this.watch_events` or hand the
Expand All @@ -463,13 +486,16 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> {
};
// skip unrelated items
if rel == ParentEqual::Unrelated {
item_idx += 1;
continue;
}
// if the event is for a parent dir of the item, only emit it if it's a delete or rename

// Check if we're about to exceed the watch_events array capacity
if event_id >= this.watch_events.len() {
// Process current batch of events
// Process current batch of events; it locks `this.mutex`
// itself, so release our guard first (non-recursive mutex).
drop(guard);
process_watch_event_batch(this, event_id)?;
// passing `this: &mut Watcher` above materialises a fresh Unique
// borrow over the whole `Watcher`, which under Stacked Borrows pops the
Expand All @@ -481,13 +507,17 @@ pub(crate) fn watch_loop_cycle(this: &mut Watcher) -> bun_sys::Result<()> {
iter.watcher = BackRef::new(&this.platform.watcher);
// Reset event_id to start a new batch
event_id = 0;
guard = this.mutex.lock_guard();
continue;
Comment thread
robobun marked this conversation as resolved.
}

this.watch_events[event_id] =
create_watch_event(&event, item_idx as WatchItemIndex);
event_id += 1;
item_idx += 1;
}
}
drop(guard);
}

// Process any remaining events in the final batch
Expand Down
44 changes: 44 additions & 0 deletions test/bake/dev/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -814,3 +814,47 @@ devTest("barrel optimization: two import statements from the same barrel (#28886
await c.expectMessage("got: ALPHA BETA");
},
});

// The watcher entry for a file whose "pretty" path is disjoint from its
// absolute path (here: an import from outside the project root, whose pretty
// path starts with "../") used to borrow path bytes out of the per-build
// bundle arena instead of cloning them. DevServer frees that arena when the
// build finishes, so a later watch event for the file made the watcher thread
// read freed memory instead of hot-reloading. The rebuild loop below churns a
// few more per-build arenas so the freed pages are recycled before the
// outside-root file is touched, which turns the stale read into a SEGV on an
// unfixed build (the watcher-thread crash seen in production).
devTest("hot reload of a file imported from outside the project root", {
// The Windows watcher is a single recursive ReadDirectoryChangesW rooted at
// the project root and refuses outside-root paths outright ("will not be
// watched"), so the dev.write below would never produce a watch event there
// (and the dangling-borrow bug was POSIX-only: Windows always cloned).
skip: ["win32"],
files: {
"../outside-root-dep/db.ts": `export const abc = "123";`,
"routes/index.ts": `
import { abc } from '../../outside-root-dep/db';
export default function (req, meta) {
return new Response('Hello, ' + abc + '!');
}
`,
},
framework: minimalFramework,
async test(dev) {
await dev.fetch("/").equals("Hello, 123!");
for (let i = 0; i < 4; i++) {
await dev.write(
"routes/index.ts",
`
import { abc } from '../../outside-root-dep/db';
export default function (req, meta) {
return new Response('Hello v${i}, ' + abc + '!');
}
`,
);
await dev.fetch("/").equals(`Hello v${i}, 123!`);
}
await dev.write("../outside-root-dep/db.ts", `export const abc = "456";`);
await dev.fetch("/").equals("Hello v3, 456!");
Comment thread
robobun marked this conversation as resolved.
},
});
Loading