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
1 change: 1 addition & 0 deletions src/codegen/generate-js2native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const rustIdentifierPaths: Record<string, string> = {
"escapeRegExp.rs": "string/escapeRegExp.rs",
"event_loop.rs": "jsc/event_loop.rs",
"ffi.rs": "runtime/ffi/ffi.rs",
"filesystem_router.rs": "runtime/api/filesystem_router.rs",
"h2_frame_parser.rs": "runtime/api/bun/h2_frame_parser.rs",
"hosted_git_info.rs": "install/hosted_git_info.rs",
"http/H2Client.rs": "http/H2Client.rs",
Expand Down
2 changes: 2 additions & 0 deletions src/js/internal-for-testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const shellParse = $newRustFunction("shell.rs", "TestingAPIs.shellParse", 2);

export const sslCtxLiveCount = $newRustFunction("SecureContext.rs", "jsLiveCount", 0);

export const dirnameStoreAppendCount = $newRustFunction("filesystem_router.rs", "jsDirnameStoreAppendCount", 0);

export const napiThreadsafeFunctionLiveCount = $newRustFunction("napi_body.rs", "jsThreadsafeFunctionLiveCount", 0);

export const escapeRegExp = $newRustFunction("escapeRegExp.rs", "jsEscapeRegExp", 1);
Expand Down
82 changes: 78 additions & 4 deletions src/resolver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,87 @@ pub mod fs {
// SAFETY: see `append_slice`.
unsafe { &*$backing() }.exists(value)
}
/// Total number of `append*` calls (inline + overflow). Exposed via
/// `bun:internal-for-testing` so leak tests can assert "N reloads
/// performed zero new appends".
pub fn append_count(&self) -> u32 {
let this = $backing();
// SAFETY: `this` is the live process-lifetime singleton; `Mutex: Sync`
// so concurrent `&Mutex` formation is sound. With the inner mutex
// held no other thread holds `&mut *this`, so the raw-place reads
// of `slice_buf_used` / `overflow_list.count` are race-free.
unsafe {
let _g = (*this).mutex.lock();
(*this).slice_buf_used as u32 + (*this).overflow_list.count
}
}
Comment thread
robobun marked this conversation as resolved.
}
};
}
string_store_impl!(DirnameStore, DIRNAME_STORE_ZST, dirname_store_backing);
string_store_impl!(FilenameStore, FILENAME_STORE_ZST, filename_store_backing);

// Process-wide (not thread-local): the backing store is process-global, so the
// dedupe index must match its lifetime or resolver worker threads each re-append.
static DIRNAME_INTERN: std::sync::LazyLock<
bun_core::Mutex<bun_collections::HashMap<&'static [u8], ()>>,
> = std::sync::LazyLock::new(Default::default);

impl DirnameStore {
/// Content-deduplicated [`append_slice`]. `BSSStringList::append` does not
/// dedupe, so a bust-then-reread cycle (hot reload, `FileSystemRouter.reload`)
/// would otherwise re-append the same directory path on every miss, exhausting
/// the store's slot capacity and leaking one heap buffer per overflow append.
pub fn intern_slice(&self, value: &[u8]) -> crate::CrateResult<&'static [u8]> {
if self.exists(value) {
// SAFETY: `exists` is a pointer-range check — `value` lies wholly
// within the process-lifetime backing buffer, so widening is sound.
return Ok(unsafe { core::slice::from_raw_parts(value.as_ptr(), value.len()) });
}
let mut set = DIRNAME_INTERN.lock();
if let Some((interned, ())) = set.get_key_value(value) {
return Ok(*interned);
}
let interned = self.append_slice(value)?;
set.insert(interned, ());
Ok(interned)
}

/// Content-deduplicated [`append_parts`]. See [`intern_slice`].
pub fn intern_parts(&self, parts: &[&[u8]]) -> crate::CrateResult<&'static [u8]> {
const STACK: usize = 512;
let total: usize = parts.iter().map(|p| p.len()).sum();
let mut stack = [0u8; STACK];
let mut heap: Vec<u8>;
let scratch: &mut [u8] = if total <= STACK {
&mut stack[..total]
} else {
heap = vec![0u8; total];
&mut heap[..]
};
let mut at = 0;
for p in parts {
scratch[at..at + p.len()].copy_from_slice(p);
at += p.len();
}
self.intern_slice(&scratch[..at])
}

/// Content-deduplicated [`append_lower_case`]. See [`intern_slice`].
pub fn intern_lower_case(&self, value: &[u8]) -> crate::CrateResult<&'static [u8]> {
const STACK: usize = 256;
let mut stack = [0u8; STACK];
let mut heap: Vec<u8>;
let scratch: &mut [u8] = if value.len() <= STACK {
&mut stack[..value.len()]
} else {
heap = vec![0u8; value.len()];
&mut heap[..]
};
self.intern_slice(bun_core::strings::copy_lowercase(value, scratch))
}
}

macro_rules! string_store_append_impl {
($t:ty, $backing:ident) => {
impl $t {
Expand Down Expand Up @@ -1297,11 +1372,10 @@ pub mod fs {
// its `dir` field is DirnameStore-interned (&'static).
unsafe { (*existing).dir }
} else if !had_handle {
DirnameStore::instance().append_slice(dir_maybe_trail_slash)?
DirnameStore::instance().intern_slice(dir_maybe_trail_slash)?
} else {
// Intern into DirnameStore so the cache entry never dangles —
// `append_slice` is a bump-pointer copy, cost is bounded.
DirnameStore::instance().append_slice(dir)?
// Intern into DirnameStore so the cache entry never dangles.
DirnameStore::instance().intern_slice(dir)?
};

// Cache miss: read the directory entries
Expand Down
8 changes: 4 additions & 4 deletions src/resolver/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3509,7 +3509,7 @@ impl<'a> Resolver<'a> {
unsafe { &*existing }.dir
} else {
Fs::file_system::DirnameStore::instance()
.append_slice(dir_path)
.intern_slice(dir_path)
.expect("unreachable")
},
self.generation,
Expand Down Expand Up @@ -4517,9 +4517,9 @@ impl<'a> Resolver<'a> {
let input = &path[..input_path_len];
if input[input.len() - 1] != SEP {
let parts: [&[u8]; 2] = [input, SEP_STR.as_bytes()];
_safe_path = Some(self.fs_ref().dirname_store.append_parts(&parts)?);
_safe_path = Some(self.fs_ref().dirname_store.intern_parts(&parts)?);
} else {
_safe_path = Some(self.fs_ref().dirname_store.append_slice(input)?);
_safe_path = Some(self.fs_ref().dirname_store.intern_slice(input)?);
}
}

Expand Down Expand Up @@ -4578,7 +4578,7 @@ impl<'a> Resolver<'a> {
unsafe { &*existing }.dir
} else {
Fs::file_system::DirnameStore::instance()
.append_slice(dir_path)
.intern_slice(dir_path)
.expect("unreachable")
},
self.generation,
Expand Down
24 changes: 13 additions & 11 deletions src/router/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1123,17 +1123,18 @@ impl Route {
let name_offset = name.as_ptr() as usize - public_path.as_ptr() as usize;
let name_len = name.len();

// NOTE: DirnameStore::append returns `&'static [u8]` (process-
// NOTE: DirnameStore::intern_* returns `&'static [u8]` (process-
// lifetime arena), so rebinding here drops the borrow on
// `route_file_buf` and avoids needing lifetime transmutes
// below.
let dirname_store = FileSystem::instance().dirname_store();
let public_path: &'static [u8] =
dirname_store.append(public_path).expect("unreachable");
let public_path: &'static [u8] = dirname_store
.intern_slice(public_path)
.expect("unreachable");
let name: &'static [u8] = &public_path[name_offset..][0..name_len];
let match_name: &'static [u8] = if has_uppercase {
dirname_store
.append_lower_case(&name[1..])
.intern_lower_case(&name[1..])
.expect("unreachable")
} else {
&name[1..]
Expand All @@ -1144,8 +1145,9 @@ impl Route {
(public_path, name, match_name)
} else {
let dirname_store = FileSystem::instance().dirname_store();
let public_path: &'static [u8] =
dirname_store.append(public_path).expect("unreachable");
let public_path: &'static [u8] = dirname_store
.intern_slice(public_path)
.expect("unreachable");
(
public_path,
Route::INDEX_ROUTE_NAME,
Expand Down Expand Up @@ -1233,7 +1235,7 @@ impl Route {

abs_path_str = FileSystem::instance()
.dirname_store()
.append(_abs)
.intern_slice(_abs)
.expect("unreachable");

// SAFETY: sole mutation; `base_`/`extname` (which may borrow
Expand All @@ -1251,7 +1253,7 @@ impl Route {
);
let interned: &'static [u8] = FileSystem::instance()
.dirname_store()
.append(normalized)
.intern_slice(normalized)
.expect("unreachable");
Interned::from_static(interned)
};
Expand All @@ -1269,13 +1271,13 @@ impl Route {
}

// NOTE: name/match_name/public_path are already `&'static` via
// DirnameStore::append above. `entry.base()` borrows the entry (it
// may be inline-stored for ≤31-byte names); intern it
// DirnameStore::intern_slice above. `entry.base()` borrows the entry
// (it may be inline-stored for ≤31-byte names); intern it
// explicitly to get `&'static` without a lifetime transmute.
// SAFETY: read-only reborrow; the `&mut` write above is dead.
let basename: &'static [u8] = FileSystem::instance()
.dirname_store()
.append(unsafe { &*entry }.base())
.intern_slice(unsafe { &*entry }.base())
.expect("unreachable");

Some(Route {
Expand Down
12 changes: 12 additions & 0 deletions src/runtime/api/filesystem_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,18 @@ pub use crate::bake::framework_router::JSFrameworkRouter as FrameworkFileSystemR

pub(crate) const DEFAULT_EXTENSIONS: &[&[u8]] = &[b"tsx", b"jsx", b"ts", b"mjs", b"cjs", b"js"];

/// Exposed via `bun:internal-for-testing` so leak tests can assert
/// `reload()` performed zero new `DirnameStore` appends.
#[bun_jsc::host_fn]
pub fn js_dirname_store_append_count(
_global: &JSGlobalObject,
_callframe: &CallFrame,
) -> JsResult<JSValue> {
Ok(JSValue::js_number(
Fs::DirnameStore::instance().append_count() as f64,
))
}

// ── local shims ───────────────────────────────────────────────────────────
// `to_js` lives on the `bun_jsc::ZigStringJsc` extension trait; `from_bytes`
// auto-detects UTF-8.
Expand Down
27 changes: 27 additions & 0 deletions test/js/bun/util/filesystem_router.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { FileSystemRouter } from "bun";
import { dirnameStoreAppendCount } from "bun:internal-for-testing";
import { expect, it } from "bun:test";
import fs, { mkdirSync, rmSync } from "fs";
import { bunEnv, bunExe, isASAN, isMacOS, isWindows, normalizeBunSnapshot, tempDir, tmpdirSync } from "harness";
Expand Down Expand Up @@ -402,6 +403,32 @@ it("reload() works", () => {
expect(router.match("/posts")!.name).toBe("/posts");
});

it("reload() does not re-intern directory paths into DirnameStore on every bust+reread", () => {
const files: Record<string, string> = { "pages/sub/nested.tsx": "export default 1;\n" };
for (let i = 0; i < 20; i++) files[`pages/p${i}.tsx`] = "export default 1;\n";
using dir = tempDir("fsr-reload-dirname-intern", files);

const router = new Bun.FileSystemRouter({
dir: path.join(String(dir), "pages"),
style: "nextjs",
fileExtensions: [".tsx"],
});
// First reload primes the intern cache for the bust+reread paths.
router.reload();
const before = dirnameStoreAppendCount();
for (let i = 0; i < 50; i++) router.reload();
const delta = dirnameStoreAppendCount() - before;
// Without the fix each reload re-appended identical bytes into the never-freed
// store: `dir_info_cached_miss` re-interned safe_path + DirEntry.dir per dir
// (6 per reload here) and `Route::parse` re-interned public_path/abs_path/
// basename per route (63 per reload here), so 50 reloads grew the store by
// ~3450 slots and eventually overflowed it with `AllocError`.
expect({ delta, match: router.match("/sub/nested")?.filePath.endsWith("nested.tsx") }).toEqual({
delta: 0,
match: true,
});
});

it("reload() works with new dirs/files", () => {
const { dir } = make(["posts.tsx"]);

Expand Down
Loading