diff --git a/src/bun_alloc/lib.rs b/src/bun_alloc/lib.rs index 64a4a3b40677..48bdb7edbced 100644 --- a/src/bun_alloc/lib.rs +++ b/src/bun_alloc/lib.rs @@ -2343,6 +2343,43 @@ impl let _key = Self::key_hash(denormalized_key); self.index.remove(&_key).is_some() } + + /// Removes every present entry whose value satisfies `pred`. Not-found + /// markers carry no value and stay. Returns how many were removed. The + /// values stay allocated: the map never frees a slot. + pub fn remove_where(&mut self, mut pred: impl FnMut(&ValueType) -> bool) -> usize { + let _guard = self.mutex.lock(); + let mut doomed: Vec = Vec::new(); + for (&hash, &index) in &self.index { + if index.index() == NOT_FOUND.index() || index.index() == UNASSIGNED.index() { + continue; + } + let value: &ValueType = if index.is_overflow() { + self.overflow_list.at_index_mut(index) + } else { + // SAFETY: a non-sentinel, non-overflow index was assigned by + // `put`, which initialized this slot via `.write()`. + unsafe { self.backing_buf[index.index() as usize].assume_init_ref() } + }; + if pred(value) { + doomed.try_reserve(1).unwrap_or_else(|_| out_of_memory()); + doomed.push(hash); + } + } + for hash in &doomed { + self.index.remove(hash); + } + doomed.len() + } + + /// Drops every not-found marker. Returns how many there were. + pub fn clear_not_found(&mut self) -> usize { + let _guard = self.mutex.lock(); + let before = self.index.len(); + self.index + .retain(|_, index| index.index() != NOT_FOUND.index()); + before - self.index.len() + } } // ────────────────────────────────────────────────────────────────────────── diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index 3660c0526a98..7d6d423acbcc 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -7,6 +7,8 @@ use bun_collections::StringSet; use bun_core::Output; use bun_core::ZStr; #[cfg(not(windows))] +use bun_paths::PathBuffer; +#[cfg(not(windows))] use bun_paths::SEP; use bun_paths::strings; #[cfg(not(windows))] @@ -117,6 +119,10 @@ impl HotReloaderCtx for VirtualMachine { VirtualMachine::bust_dir_cache(self, path) } + fn bust_dir_cache_tree(&mut self, path: &[u8]) -> bool { + self.transpiler.resolver.bust_dir_cache_tree(path) + } + fn get_loaders(&self) -> &bun_ast::LoaderHashTable { &self.transpiler.options.loaders } @@ -210,6 +216,9 @@ pub trait HotReloaderCtx { /// Returns whether anything was busted. fn bust_dir_cache(&mut self, path: &[u8]) -> bool; + /// `bust_dir_cache` for `path` and every cached directory below it. + fn bust_dir_cache_tree(&mut self, path: &[u8]) -> bool; + /// `&transpiler.options.loaders`. fn get_loaders(&self) -> &bun_ast::LoaderHashTable; @@ -786,6 +795,83 @@ where self.tombstones.get(key).copied() } + /// Writes `dir/name` into `buf`, NUL-terminated. `None` when it does not + /// fit. + #[cfg(not(windows))] + fn join_entry_path<'b>(buf: &'b mut PathBuffer, dir: &[u8], name: &[u8]) -> Option<&'b ZStr> { + let dir = strings::trim_right(dir, &[SEP]); + let len = dir.len() + 1 + name.len(); + if len >= buf.len() { + return None; + } + buf[..dir.len()].copy_from_slice(dir); + buf[dir.len()] = SEP; + buf[dir.len() + 1..len].copy_from_slice(name); + buf[len] = 0; + // SAFETY: buf[len] == 0 written above. + Some(ZStr::from_buf(&buf[..], len)) + } + + /// Drops what the resolver cached under `dir/name` and below. Returns + /// true when there was something to drop, which means a resolution went + /// through `dir/name` as a directory. + #[cfg(not(windows))] + fn bust_changed_entry( + &mut self, + rfs: &mut Fs::file_system::RealFS, + dir: &[u8], + name: &[u8], + ) -> bool { + let mut buf = bun_paths::path_buffer_pool::get(); + let Some(child) = Self::join_entry_path(&mut buf, dir, name) else { + return false; + }; + self.tombstone_entries(rfs, child.as_bytes()); + self.ctx_mut().bust_dir_cache_tree(child.as_bytes()) + } + + /// Whether the symlink `entry` inside `dir` now resolves somewhere else + /// than where the resolver followed it. Drops the resolver cache for the + /// link when it does. + #[cfg(not(windows))] + fn symlink_entry_changed( + &mut self, + rfs: &mut Fs::file_system::RealFS, + dir: &[u8], + entry: &Fs::Entry, + ) -> bool { + let mut buf = bun_paths::path_buffer_pool::get(); + let Some(child) = Self::join_entry_path(&mut buf, dir, entry.base()) else { + return false; + }; + let mut real_buf = bun_paths::path_buffer_pool::get(); + if bun_sys::realpath(child, &mut real_buf) + .is_ok_and(|real| real == entry.followed_symlink()) + { + return false; + } + self.tombstone_entries(rfs, child.as_bytes()); + self.ctx_mut().bust_dir_cache_tree(child.as_bytes()); + true + } + + /// Keeps the listing of `dir` reachable for later events on it after + /// the resolver cache is busted. + #[cfg(not(windows))] + fn tombstone_entries(&mut self, rfs: &mut Fs::file_system::RealFS, dir: &[u8]) { + let Some(existing) = rfs.entries.get(dir) else { + return; + }; + let existing: *mut Fs::EntriesOption = existing; + let mut key = bun_paths::path_buffer_pool::get(); + if dir.len() + 1 >= key.len() { + return; + } + key[..dir.len()].copy_from_slice(dir); + key[dir.len()] = SEP; + self.put_tombstone(&key[..dir.len() + 1], existing); + } + pub(crate) fn on_error(_: &mut Self, err: &bun_sys::Error) { // `bun_sys::Error::name()` does the errno→tag-name lookup. Output::err(err.name(), "Watcher crashed", ()); @@ -1118,6 +1204,74 @@ where } } + // A changed entry the module graph resolved through + // (a retargeted symlink, a replaced directory) has a + // stale real path in the resolver cache. + if IS_KQUEUE { + // kqueue names no entry: check every followed symlink. + if let Some(dir_ent) = entries_option { + // SAFETY: dir_ent points into rfs.entries (or a + // tombstoned copy); both outlive this loop + // iteration. Shared access only. + let dir_ent = unsafe { &*dir_ent }; + // `bust_dir_cache` takes `entries_mutex` too. + let mut followed: Vec<&'static Fs::Entry> = Vec::new(); + { + let _entries_lock = rfs.entries_mutex.lock_guard(); + if let Fs::EntriesOption::Entries(listing) = dir_ent { + for &entry_ptr in listing.data.values() { + // SAFETY: EntryStore-owned slot, never freed. + let entry: &'static Fs::Entry = unsafe { &*entry_ptr }; + if !entry.followed_symlink().is_empty() { + bun_core::handle_oom(followed.try_reserve(1)); + followed.push(entry); + } + } + } + } + for entry in followed { + if self.symlink_entry_changed(rfs, file_path, entry) { + current_task.append(current_hash); + } + } + } + } else { + for changed_name_ in affected_inotify { + let changed_name: &[u8] = match changed_name_ { + Some(z) => z.as_bytes(), + None => continue, + }; + if changed_name.is_empty() + || changed_name[0] == b'~' + || changed_name[0] == b'.' + { + continue; + } + let followed_symlink = match entries_option { + Some(dir_ent) => { + let _entries_lock = rfs.entries_mutex.lock_guard(); + // SAFETY: dir_ent points into rfs.entries + // (or a tombstoned copy); both outlive + // this loop iteration. Shared access only. + match unsafe { &*dir_ent } { + Fs::EntriesOption::Entries(listing) => { + listing.get(changed_name).is_some_and(|e| { + !e.entry().followed_symlink().is_empty() + }) + } + Fs::EntriesOption::Err(_) => false, + } + } + None => false, + }; + if self.bust_changed_entry(rfs, file_path, changed_name) + || followed_symlink + { + current_task.append(current_hash); + } + } + } + if let Some(dir_ent) = entries_option { // SAFETY: dir_ent points into rfs.entries (or a tombstoned copy); // both outlive this loop iteration. Shared access only — @@ -1342,6 +1496,10 @@ impl<'a> HotReloaderCtx for bun_bundler::BundleV2<'a> { bun_bundler::BundleV2::bust_dir_cache(self, path) } + fn bust_dir_cache_tree(&mut self, path: &[u8]) -> bool { + self.transpiler.resolver.bust_dir_cache_tree(path) + } + fn get_loaders(&self) -> &bun_ast::LoaderHashTable { &self.transpiler.options.loaders } diff --git a/src/resolver/fs.rs b/src/resolver/fs.rs index ace3e96f4b76..98eac71e43c9 100644 --- a/src/resolver/fs.rs +++ b/src/resolver/fs.rs @@ -165,6 +165,14 @@ impl Entry { self.cache.set(c); } + /// The real path a resolution followed this symlink to. Empty when the + /// entry is not a symlink or was never stat'd. No I/O. Safe from any + /// thread: every `cache` rewrite holds `mutex`. + pub fn followed_symlink(&self) -> &'static [u8] { + let _guard = self.mutex.lock_guard(); + self.cache().symlink.as_bytes() + } + #[inline(always)] pub(crate) fn set_cache_symlink(&self, symlink: Interned) { let mut c = self.cache.get(); diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index f1b75bb06a32..b21745434db9 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -1044,6 +1044,12 @@ pub mod fs { pub(crate) fn remove(&mut self, key: &[u8]) -> bool { self.inner().remove(key) } + pub(crate) fn remove_where(&mut self, pred: impl FnMut(&EntriesOption) -> bool) -> usize { + self.inner().remove_where(pred) + } + pub(crate) fn clear_not_found(&mut self) -> usize { + self.inner().clear_not_found() + } } /// The active filesystem backend (always the real filesystem). @@ -1358,6 +1364,22 @@ pub mod fs { self.entries.remove(file_path) } + /// Evicts every cached listing of a directory below `dir_with_slash` + /// (`dir_with_slash` itself excluded). Returns how many were removed. + pub(crate) fn bust_entries_cache_below(&mut self, dir_with_slash: &[u8]) -> usize { + let _g = self.entries_mutex.lock_guard(); + let listings = self.entries.remove_where(|entry| match entry { + EntriesOption::Entries(listing) => { + listing.dir.len() > dir_with_slash.len() + && listing.dir.starts_with(dir_with_slash) + } + EntriesOption::Err(_) => false, + }); + // A not-found marker has no key to match, and a path that did + // not exist under the old target can exist under the new one. + listings + self.entries.clear_not_found() + } + /// lstat + (if symlink) open + fstat + /// readlink to populate an `EntryCache`. Windows: `GetFileAttributesW` + /// (if reparse point) `CreateFileW`-follow + `GetFinalPathNameByHandle` diff --git a/src/resolver/resolver.rs b/src/resolver/resolver.rs index 31dd498e7972..9b4e42058ebb 100644 --- a/src/resolver/resolver.rs +++ b/src/resolver/resolver.rs @@ -488,6 +488,10 @@ pub struct Resolver<'a> { pub elapsed: u64, // tracing pub watcher: Option, + /// Directories that hold a followed symlink, to watch once + /// `entries_mutex` is released (the watcher thread takes `entries_mutex` + /// under the watcher mutex). + link_dir_watches: Vec<(&'static [u8], FD)>, pub caches: CacheSet, pub generation: Generation, @@ -625,6 +629,7 @@ impl<'a> Resolver<'a> { debug_logs: None, elapsed: 0, watcher: from.watcher, + link_dir_watches: Vec::new(), caches: CacheSet::init(), generation: from.generation, package_manager: from.package_manager, @@ -925,6 +930,7 @@ impl<'a> Resolver<'a> { debug_logs: None, elapsed: 0, watcher: None, + link_dir_watches: Vec::new(), generation: 0, package_manager: None, on_wake_package_manager: Default::default(), @@ -1598,6 +1604,22 @@ impl<'a> Resolver<'a> { bstr::BStr::new(symlink_path) )); } + + // Watch the link's directory: a retarget is invisible + // from the real path. + if FeatureFlags::WATCH_DIRECTORIES { + if let Some(watcher) = self.watcher { + if let Some((link_dir, fd)) = self + .fs_mut() + .fs + .entries + .at_index(dir.entries) + .and_then(|e| e.dir_and_fd()) + { + watcher.watch(link_dir, fd); + } + } + } } else if !dir.abs_real_path.is_empty() { // When the directory is a symlink, we don't need to call getFdPath. let parts = [dir.abs_real_path, query.entry().base()]; @@ -2428,6 +2450,58 @@ impl<'a> Resolver<'a> { first_bust || second_bust } + /// Registers the directory watches queued by `dir_info_uncached`. Call + /// without `entries_mutex` held. + fn flush_link_dir_watches(&mut self) { + if self.link_dir_watches.is_empty() { + return; + } + let Some(watcher) = self.watcher else { + self.link_dir_watches.clear(); + return; + }; + for (dir, fd) in self.link_dir_watches.drain(..) { + watcher.watch(dir, fd); + } + } + + /// `bust_dir_cache` for `path` and for every cached directory below it. + /// A retargeted directory symlink makes the real path cached by each of + /// them stale at once. Returns whether anything was busted. + pub fn bust_dir_cache_tree(&mut self, path: &[u8]) -> bool { + // Directories are cached from the top down, so nothing is cached + // below a path that is not cached itself. + if !self.bust_dir_cache(path) { + return false; + } + let mut buf = bun_paths::path_buffer_pool::get(); + let path = strings::without_trailing_slash_windows_path(path); + if path.len() + 1 >= buf.len() { + return true; + } + buf[..path.len()].copy_from_slice(path); + buf[path.len()] = SEP; + let dir_with_slash: &[u8] = &buf[..path.len() + 1]; + let entries_below = self.fs_mut().fs.bust_entries_cache_below(dir_with_slash); + // `DirInfo` slots are written under `entries_mutex`. + let _entries_lock = self.fs_ref().fs.entries_mutex.lock_guard(); + let dirs_below = self.dir_cache_mut().remove_where(|info| { + info.abs_path.len() > dir_with_slash.len() && info.abs_path.starts_with(dir_with_slash) + }); + // A not-found marker has no key to match, and a path that did not + // exist under the old target can exist under the new one. + let not_found = self.dir_cache_mut().clear_not_found(); + bun_core::scoped_log!( + ResolverDev, + "Bust below {} = {}, {}, {}", + bstr::BStr::new(dir_with_slash), + entries_below, + dirs_below, + not_found + ); + true + } + /// bust both the named file and a parent directory, because `./hello` can resolve /// to `./hello.js` or `./hello/index.js` pub fn bust_dir_cache_from_specifier( @@ -4205,7 +4279,9 @@ impl<'a> Resolver<'a> { .map(DirInfoRef::from_slot)); } - self.dir_info_cached_miss(enable_logging, input_path, top_result) + let result = self.dir_info_cached_miss(enable_logging, input_path, top_result); + self.flush_link_dir_watches(); + result } /// Cold tail of [`dir_info_cached_maybe_log`]: the directory walk + @@ -6270,6 +6346,13 @@ impl<'a> Resolver<'a> { logs.add_note(buf); } info.abs_real_path = symlink; + // Watch the link's directory: a retarget is + // invisible from the real path. + if FeatureFlags::WATCH_DIRECTORIES && self.watcher.is_some() { + bun_core::handle_oom(self.link_dir_watches.try_reserve(1)); + self.link_dir_watches + .push((parent_entries.dir, parent_entries.fd)); + } } else if !parent_.abs_real_path.is_empty() { // this might leak a little i'm not sure let parts = [parent_.abs_real_path, base]; diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 8ab6f31dd9e6..b2546a162efe 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -1,7 +1,7 @@ import { spawn } from "bun"; -import { beforeEach, expect, it } from "bun:test"; -import { copyFileSync, cpSync, readFileSync, renameSync, rmSync, unlinkSync, writeFileSync } from "fs"; -import { bunEnv, bunExe, isDebug, isWindows, tmpdirSync, waitForFileToExist } from "harness"; +import { beforeEach, describe, expect, it } from "bun:test"; +import { copyFileSync, cpSync, readFileSync, renameSync, rmSync, symlinkSync, unlinkSync, writeFileSync } from "fs"; +import { bunEnv, bunExe, isDebug, isWindows, tempDir, tmpdirSync, waitForFileToExist } from "harness"; import { join } from "path"; const timeout = isDebug ? Infinity : 10_000; @@ -776,3 +776,152 @@ ${Buffer.alloc(counter * 2, " ").toString()}throw new Error(${counter});`, }, longTimeout, ); + +// Reads stdout line by line. `next()` resolves with the next non-empty line. +function lineReader(stream: ReadableStream) { + const reader = stream.getReader(); + const decoder = new TextDecoder(); + let buffered = ""; + const lines: string[] = []; + return { + async next(): Promise { + while (lines.length === 0) { + const { value, done } = await reader.read(); + if (done) throw new Error("stdout closed"); + buffered += decoder.decode(value, { stream: true }); + const parts = buffered.split("\n"); + buffered = parts.pop() ?? ""; + lines.push(...parts.filter(line => line.length > 0)); + } + return lines.shift()!; + }, + }; +} + +// Retargets the link at `linkPath` with a rename over it, as `ln -sfn` does. +function retargetSymlink(linkPath: string, target: string) { + symlinkSync(target, linkPath + ".tmp"); + renameSync(linkPath + ".tmp", linkPath); +} + +// The resolver keys a module by its real path and the watcher watches that +// real path. An import that goes through a symlink has to follow the link when +// the link changes, not stay bound to the file the link pointed to at first. +describe.skipIf(isWindows)("--hot follows a retargeted symlink on the import path", () => { + async function run(files: Record, links: Record) { + const dir = tempDir("hot-symlink", files); + for (const [link, target] of Object.entries(links)) { + symlinkSync(target, join(String(dir), link)); + } + const runner = spawn({ + cmd: [bunExe(), "--hot", "entry.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "inherit", + stdin: "ignore", + }); + return { dir, runner, stdout: lineReader(runner.stdout), path: (p: string) => join(String(dir), p) }; + } + + it("directory symlink", async () => { + const { dir, runner, stdout, path } = await run( + { + "entry.ts": `import "./cur/app.ts";`, + "v1/app.ts": `console.log("MARK_V1");`, + "v2/app.ts": `console.log("MARK_V2");`, + }, + { cur: "v1" }, + ); + using _dir = dir; + await using _runner = runner; + expect(await stdout.next()).toBe("MARK_V1"); + + retargetSymlink(path("cur"), "v2"); + expect(await stdout.next()).toBe("MARK_V2"); + + // An edit to the new target is seen. + writeFileSync(path("v2/app.ts"), `console.log("MARK_V2_EDITED");`); + expect(await stdout.next()).toBe("MARK_V2_EDITED"); + }); + + it("directory symlink with a directory below it", async () => { + const { dir, runner, stdout, path } = await run( + { + "entry.ts": `import "./cur/sub/app.ts";`, + "v1/sub/app.ts": `console.log("MARK_V1");`, + "v2/sub/app.ts": `console.log("MARK_V2");`, + }, + { cur: "v1" }, + ); + using _dir = dir; + await using _runner = runner; + expect(await stdout.next()).toBe("MARK_V1"); + + retargetSymlink(path("cur"), "v2"); + expect(await stdout.next()).toBe("MARK_V2"); + + writeFileSync(path("v2/sub/app.ts"), `console.log("MARK_V2_EDITED");`); + expect(await stdout.next()).toBe("MARK_V2_EDITED"); + }); + + it("directory symlink in a directory with no loaded module", async () => { + const { dir, runner, stdout, path } = await run( + { + "src/entry.ts": `import "../links/cur/app.ts";`, + "entry.ts": `import "./src/entry.ts";`, + "v1/app.ts": `console.log("MARK_V1");`, + "v2/app.ts": `console.log("MARK_V2");`, + "links/.keep": "", + }, + { "links/cur": "../v1" }, + ); + using _dir = dir; + await using _runner = runner; + expect(await stdout.next()).toBe("MARK_V1"); + + retargetSymlink(path("links/cur"), "../v2"); + expect(await stdout.next()).toBe("MARK_V2"); + + writeFileSync(path("v2/app.ts"), `console.log("MARK_V2_EDITED");`); + expect(await stdout.next()).toBe("MARK_V2_EDITED"); + }); + + it("directory symlink whose new target has a directory the old one lacked", async () => { + const { dir, runner, stdout, path } = await run( + { + "entry.ts": `import("./cur/sub/app.ts").catch(() => console.log("MARK_MISSING"));`, + "v1/app.ts": `console.log("MARK_V1");`, + "v2/sub/app.ts": `console.log("MARK_V2");`, + }, + { cur: "v1" }, + ); + using _dir = dir; + await using _runner = runner; + expect(await stdout.next()).toBe("MARK_MISSING"); + + // The resolver cached `cur/sub` as not found under v1. + retargetSymlink(path("cur"), "v2"); + expect(await stdout.next()).toBe("MARK_V2"); + }); + + it("file symlink", async () => { + const { dir, runner, stdout, path } = await run( + { + "entry.ts": `import "./app.ts";`, + "v1/app.ts": `console.log("MARK_V1");`, + "v2/app.ts": `console.log("MARK_V2");`, + }, + { "app.ts": "v1/app.ts" }, + ); + using _dir = dir; + await using _runner = runner; + expect(await stdout.next()).toBe("MARK_V1"); + + retargetSymlink(path("app.ts"), "v2/app.ts"); + expect(await stdout.next()).toBe("MARK_V2"); + + writeFileSync(path("v2/app.ts"), `console.log("MARK_V2_EDITED");`); + expect(await stdout.next()).toBe("MARK_V2_EDITED"); + }); +});