Skip to content
Draft
Show file tree
Hide file tree
Changes from 10 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
27 changes: 27 additions & 0 deletions src/jsc/hot_reloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,33 @@ where
};
}

// Rename-over of a held-open inode delivers only IN_ATTRIB; evict on st_nlink==0.
#[cfg(any(target_os = "linux", target_os = "android"))]
if !event.op.contains(WatchOp::DELETE)
&& event.op.intersects(WatchOp::METADATA | WatchOp::RENAME)
{
let fd = file_descriptors[event.index as usize];
let orphaned = event.op.contains(WatchOp::RENAME)
|| (fd.is_valid()
&& bun_sys::fstat(fd)
.map(|st| st.st_nlink == 0)
.unwrap_or(false));
if orphaned {
// SAFETY: same as the DELETE eviction above.
unsafe {
(*ctx).remove_at_index::<false>(
bun_watcher::Kind::File,
event.index,
0,
&[],
)
};
record_changed_path(file_path);
current_task.append(current_hash);
continue;
Comment thread
robobun marked this conversation as resolved.
Resolved
}
}

if self.verbose {
Self::debug(format_args!(
"File changed: {}",
Expand Down
24 changes: 21 additions & 3 deletions src/runtime/bake/DevServer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5817,9 +5817,27 @@ impl DevServer {

match kind {
bun_watcher::Kind::File => {
if event.op.contains(bun_watcher::Op::DELETE)
|| event.op.contains(bun_watcher::Op::RENAME)
{
#[allow(unused_mut)]
let mut evict = event.op.contains(bun_watcher::Op::DELETE)
|| event.op.contains(bun_watcher::Op::RENAME);
#[cfg(any(target_os = "linux", target_os = "android"))]
if !evict && event.op.contains(bun_watcher::Op::METADATA) {
let fd = slice.items_fd()[event.index as usize];
if fd.is_valid()
&& bun_sys::fstat(fd)
.map(|st| st.st_nlink == 0)
.unwrap_or(false)
{
evict = true;
} else if !event.op.intersects(
bun_watcher::Op::WRITE
| bun_watcher::Op::MOVE_TO
| bun_watcher::Op::CREATE,
) {
continue;
Comment thread
robobun marked this conversation as resolved.
Resolved
}
}
if evict {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// TODO: audit this line heavily
self.bun_watcher.remove_at_index::<false>(
bun_watcher::Kind::File,
Expand Down
12 changes: 10 additions & 2 deletions src/watcher/INotifyWatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,13 @@ impl INotifyWatcher {
use bun_sys::linux::IN;
debug_assert!(self.loaded);
let old_count = self.watch_count.fetch_add(1, Ordering::Release);
let watch_file_mask =
IN::EXCL_UNLINK | IN::MOVE_SELF | IN::DELETE_SELF | IN::MOVED_TO | IN::MODIFY;
// IN_ATTRIB catches the link-count drop when a held-open inode is renamed over.
let watch_file_mask = IN::EXCL_UNLINK
| IN::MOVE_SELF
| IN::DELETE_SELF
| IN::MOVED_TO
| IN::MODIFY
| IN::ATTRIB;
Comment thread
robobun marked this conversation as resolved.
Resolved
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// SAFETY: fd is a valid inotify fd (loaded == true), pathname is NUL-terminated.
let rc = unsafe {
bun_sys::linux::inotify_add_watch(self.fd.native(), pathname.as_ptr(), watch_file_mask)
Expand Down Expand Up @@ -539,6 +544,9 @@ fn watch_event_from_inotify_event(event: &Event, index: WatchItemIndex) -> Watch
if (event.mask & IN::MODIFY) > 0 {
op |= Op::WRITE;
}
if (event.mask & IN::ATTRIB) > 0 {
op |= Op::METADATA;
}
if (event.mask & IN::CREATE) > 0 {
op |= Op::CREATE;
}
Expand Down
16 changes: 9 additions & 7 deletions src/watcher/Watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,14 @@ impl Watcher {

#[inline]
fn is_eligible_directory(&self, dir: &[u8]) -> bool {
strings::contains(dir, self.top_level_dir()) && !strings::contains(dir, b"node_modules")
if strings::contains(dir, b"node_modules") {
return false;
}
if cfg!(windows) {
// ReadDirectoryChangesW is cwd-rooted; POSIX has no such restriction.
return strings::contains(dir, self.top_level_dir());
}
true
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved

#[inline]
Expand Down Expand Up @@ -954,12 +961,7 @@ impl Watcher {
}

pub(crate) fn on_maybe_watch_directory(&mut self, file_path: &[u8], dir_fd: Fd) {
// We don't want to watch:
// - Directories outside the root directory
// - Directories inside node_modules
if !strings::contains(file_path, b"node_modules")
&& strings::contains(file_path, self.top_level_dir())
{
if self.is_eligible_directory(file_path) {
let _ = self.add_directory::<false>(dir_fd, file_path, Self::get_hash(file_path));
}
}
Expand Down
144 changes: 143 additions & 1 deletion test/cli/hot/watch.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { spawn } from "bun";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, forEachLine, isBroken, isWindows, tempDir } from "harness";
import { writeFile } from "node:fs/promises";
import { mkdir, rename, symlink, writeFile } from "node:fs/promises";
import { join } from "node:path";

describe.todoIf(isBroken && isWindows)("--watch works", async () => {
Expand Down Expand Up @@ -48,3 +48,145 @@ describe.todoIf(isBroken && isWindows)("--watch works", async () => {
});
}
});

// A module imported from outside the process cwd (monorepo sibling package,
// or the entrypoint itself when bun is launched from a different directory)
// used to get only a per-inode inotify/kqueue watch and no parent-directory
// watch. The first atomic rename-save (write temp + rename over; the default
// for vim, sed -i, prettier, JetBrains safe-write, git checkout) replaces the
// inode and orphans that watch, so the save and every later save of that file
// were missed, and under --hot the stale pre-save source was served forever
// from the pinned fd.
describe.skipIf(isWindows)("picks up atomic rename-save of a module outside cwd", () => {
async function renameSave(path: string, content: string) {
await writeFile(path + ".next", content);
await rename(path + ".next", path);
}

async function nextEval(iter: AsyncIterator<string>): Promise<string> {
while (true) {
const { value, done } = await iter.next();
if (done) throw new Error("stream ended before an EVAL line");
if (value.startsWith("EVAL ")) return value;
}
}

for (const flag of ["--watch", "--hot"] as const) {
test.concurrent(flag, async () => {
await using dir = tempDir("watch-outside-cwd", {
"app/entry.ts":
`import { sh } from "../shared/lib.ts";\n` +
`globalThis.g = (globalThis.g ?? 0) + 1;\n` +
`console.log("EVAL g=" + globalThis.g + " shared=" + sh);\n`,
"shared/lib.ts": `export const sh = "V0";\n`,
});
const appDir = join(String(dir), "app");
const sharedLib = join(String(dir), "shared", "lib.ts");

await using proc = spawn({
cmd: [bunExe(), flag, "--no-clear-screen", "entry.ts"],
cwd: appDir,
env: bunEnv,
stdio: ["ignore", "pipe", "inherit"],
});
const iter = forEachLine(proc.stdout);

expect(await nextEval(iter)).toBe("EVAL g=1 shared=V0");

// Atomic rename-save of the out-of-cwd dependency. Before the fix this
// produced no reload at all (the per-inode watch is orphaned and the
// parent dir was not watched).
await renameSave(sharedLib, `export const sh = "V1";\n`);
const g2 = flag === "--hot" ? "2" : "1";
expect(await nextEval(iter)).toBe(`EVAL g=${g2} shared=V1`);

// Second rename-save on the (now new) inode.
await renameSave(sharedLib, `export const sh = "V2";\n`);
const g3 = flag === "--hot" ? "3" : "1";
expect(await nextEval(iter)).toBe(`EVAL g=${g3} shared=V2`);

proc.kill("SIGKILL");
await proc.exited;
});
}

// A module reached through an in-cwd directory symlink is resolved to its
// real path (outside cwd) before being watched, so it hit the same gate.
test.concurrent("--hot via an in-cwd directory symlink to an out-of-cwd dir", async () => {
await using dir = tempDir("watch-symlink-outside-cwd", {
"app/entry.ts":
`import { sh } from "./link/dep.ts";\n` +
`globalThis.g = (globalThis.g ?? 0) + 1;\n` +
`console.log("EVAL g=" + globalThis.g + " shared=" + sh);\n`,
"realdir/dep.ts": `export const sh = "V0";\n`,
});
const appDir = join(String(dir), "app");
const realDep = join(String(dir), "realdir", "dep.ts");
await symlink(join(String(dir), "realdir"), join(appDir, "link"), "dir");

await using proc = spawn({
cmd: [bunExe(), "--hot", "--no-clear-screen", "entry.ts"],
cwd: appDir,
env: bunEnv,
stdio: ["ignore", "pipe", "inherit"],
});
const iter = forEachLine(proc.stdout);

expect(await nextEval(iter)).toBe("EVAL g=1 shared=V0");

await renameSave(realDep, `export const sh = "V1";\n`);
expect(await nextEval(iter)).toBe("EVAL g=2 shared=V1");

await renameSave(realDep, `export const sh = "V2";\n`);
expect(await nextEval(iter)).toBe("EVAL g=3 shared=V2");

proc.kill("SIGKILL");
await proc.exited;
});

// Workspace package imported by bare name from the workspace root: everything
// is inside cwd and the real-path parent dir IS watched, but the resolver's
// dir-entry cache is keyed by the `node_modules/lib/` spelling while the
// watchlist entry carries the real path, so the directory IN_MOVED_TO used to
// find no matching file entry and the rename-save was dropped. The per-file
// IN_ATTRIB (st_nlink == 0) eviction recovers it.
for (const flag of ["--watch", "--hot"] as const) {
test.concurrent(`${flag} a workspace package imported by bare name through node_modules`, async () => {
await using dir = tempDir("watch-workspace-bare-import", {
"package.json": JSON.stringify({ name: "root", private: true, workspaces: ["packages/*"] }),
"packages/app/package.json": JSON.stringify({ name: "app", dependencies: { lib: "workspace:*" } }),
"packages/app/entry.ts":
`import { sh } from "lib/index.js";\n` +
`globalThis.g = (globalThis.g ?? 0) + 1;\n` +
`console.log("EVAL g=" + globalThis.g + " shared=" + sh);\n`,
"packages/lib/package.json": JSON.stringify({ name: "lib", version: "1.0.0", main: "index.js" }),
"packages/lib/index.js": `export const sh = "V0";\n`,
});
const root = String(dir);
const libIndex = join(root, "packages", "lib", "index.js");
await mkdir(join(root, "packages", "app", "node_modules"), { recursive: true });
await symlink(join("..", "..", "lib"), join(root, "packages", "app", "node_modules", "lib"), "dir");

await using proc = spawn({
cmd: [bunExe(), flag, "--no-clear-screen", "packages/app/entry.ts"],
cwd: root,
env: bunEnv,
stdio: ["ignore", "pipe", "inherit"],
});
const iter = forEachLine(proc.stdout);

expect(await nextEval(iter)).toBe("EVAL g=1 shared=V0");

await renameSave(libIndex, `export const sh = "V1";\n`);
const g2 = flag === "--hot" ? "2" : "1";
expect(await nextEval(iter)).toBe(`EVAL g=${g2} shared=V1`);

await renameSave(libIndex, `export const sh = "V2";\n`);
const g3 = flag === "--hot" ? "3" : "1";
expect(await nextEval(iter)).toBe(`EVAL g=${g3} shared=V2`);

proc.kill("SIGKILL");
await proc.exited;
});
}
});
Loading