diff --git a/src/css/css_parser.rs b/src/css/css_parser.rs index 9aec1fb989a9..97b990dc451a 100644 --- a/src/css/css_parser.rs +++ b/src/css/css_parser.rs @@ -3913,7 +3913,7 @@ pub mod nth { } else if strings::eql_case_insensitive_asciii_check_length(unit, b"n-") { return parse_signless_b(input, a, -1); } else { - if let Ok(b) = parse_n_dash_digits(unit) { + if let Ok(b) = parse_n_dash_digits(input.arena(), unit) { return Ok((a, b)); } else { return Err(input.new_unexpected_token_error(Token::Ident(unit))); @@ -3941,7 +3941,7 @@ pub mod nth { } else { (value, 1) }; - if let Ok(b) = parse_n_dash_digits(slice) { + if let Ok(b) = parse_n_dash_digits(input.arena(), slice) { return Ok((a, b)); } return Err(input.new_unexpected_token_error(Token::Ident(value))); @@ -3956,7 +3956,7 @@ pub mod nth { } else if strings::eql_case_insensitive_asciii_check_length(value, b"-n") { return parse_signless_b(input, 1, -1); } else { - if let Ok(b) = parse_n_dash_digits(value) { + if let Ok(b) = parse_n_dash_digits(input.arena(), value) { return Ok((1, b)); } else { return Err(input.new_unexpected_token_error(Token::Ident(value))); @@ -4011,21 +4011,20 @@ pub mod nth { Err(input.new_unexpected_token_error(tok)) } - fn parse_n_dash_digits(str: &[u8]) -> Maybe { + fn parse_n_dash_digits(arena: &Bump, str: &[u8]) -> Maybe { let bytes = str; if bytes.len() >= 3 && strings::eql_case_insensitive_asciii_check_length(&bytes[0..2], b"n-") && bytes[2..].iter().all(|&b| b >= b'0' && b <= b'9') { - parse_number_saturate(&str[1..]) // Include the minus sign + parse_number_saturate(arena, &str[1..]) // Include the minus sign } else { Err(()) } } - fn parse_number_saturate(string: &[u8]) -> Maybe { - let arena = Bump::new(); - let mut input = ParserInput::new(string, &arena); + fn parse_number_saturate(arena: &Bump, string: &[u8]) -> Maybe { + let mut input = ParserInput::new(string, arena); let mut parser = Parser::new(&mut input, None, ParserOpts::default(), None); let tok = match parser.next_including_whitespace_and_comments() { Ok(v) => v, diff --git a/src/runtime/api/filesystem_router.rs b/src/runtime/api/filesystem_router.rs index 53e99ce00403..c0c97dcda850 100644 --- a/src/runtime/api/filesystem_router.rs +++ b/src/runtime/api/filesystem_router.rs @@ -22,7 +22,6 @@ pub(crate) mod kind_enum { use bun_paths::strings; use core::cell::UnsafeCell; -use bun_alloc::Arena as ArenaAllocator; use bun_ast as Log; use bun_core::{EncodedSlice, String as BunString, Utf8Bytes}; use bun_jsc::bun_string_jsc; @@ -82,7 +81,7 @@ impl<'a, 'r> Router::ResolverLike for RouterResolver<'a, 'r> { bun_jsc::codegen_cached_accessors!("FileSystemRouter"; routes); // R-2 (host-fn re-entrancy): every JS-exposed method takes `&self`; per-field -// interior mutability via `JsCell` for the two fields that `reload`/`match` +// interior mutability via `JsCell` for `router`, the field that `reload`/`match` // mutate. The codegen shim may still emit `this: &mut FileSystemRouter` — // `&mut T` reborrows to `&T` so the impls compile against either. #[bun_jsc::JsClass] @@ -92,10 +91,6 @@ pub struct FileSystemRouter { pub(crate) origin: Option>, pub(crate) base_dir: Option>, pub(crate) router: JsCell, - // Router borrows slices from this arena across calls; - // kept as boxed arena per LIFETIMES.tsv (OWNED). `bun_alloc::Arena` (mimalloc heap) is - // the runtime-wide arena type, so allocations here are individually freeable too. - pub(crate) arena: JsCell>, pub(crate) asset_prefix: Option>, } @@ -167,9 +162,7 @@ impl FileSystemRouter { global_this.throw_invalid_arguments(format_args!("Expected dir to be a string")) ); } - // extensions/asset_prefix/log are all allocated from this arena. - let arena = Box::new(ArenaAllocator::new()); - let mut extensions: Vec<&[u8]> = Vec::new(); + let mut extensions: Vec> = Vec::new(); if let Some(file_extensions) = argument.get(global_this, "fileExtensions")? { if !file_extensions.js_type().is_array() { return Err(global_this.throw_invalid_arguments(format_args!( @@ -189,12 +182,7 @@ impl FileSystemRouter { continue; } let utf8 = val.to_utf8(global_this)?; - // SAFETY: arena is boxed and moved into the returned `FileSystemRouter`, so the - // backing allocation outlives this slice. Cast through raw ptr to detach the - // borrow from `arena` so it can be moved below. - let leaked: &'static [u8] = - unsafe { bun_ptr::detach_lifetime(arena.alloc_slice_copy(utf8.slice())) }; - extensions.push(&leaked[1..]); + extensions.push(Box::from(&utf8.slice()[1..])); } } @@ -245,7 +233,7 @@ impl FileSystemRouter { let mut router = Router::Router::init(RouteConfig { dir: Box::from(&path_to_use[..]), extensions: if !extensions.is_empty() { - extensions.iter().map(|s| Box::<[u8]>::from(*s)).collect() + extensions.into_boxed_slice() } else { DEFAULT_EXTENSIONS .iter() @@ -323,7 +311,6 @@ impl FileSystemRouter { None }, router: JsCell::new(router), - arena: JsCell::new(arena), }); // RouteConfig::dir is an owned `Box<[u8]>`, so copy the bytes; the @@ -436,7 +423,6 @@ impl FileSystemRouter { ) -> JsResult { let this_value = callframe.this(); - let arena = Box::new(ArenaAllocator::new()); // SAFETY: `bun_vm()` returns the live VM raw pointer for this global. let vm_ptr = global_this.bun_vm_ptr(); @@ -503,11 +489,7 @@ impl FileSystemRouter { } } - // `this.router.deinit(); this.arena.deinit(); destroy(this.arena)` — drop old values. - // Note: order matters — old router borrows slices from old arena, so it must drop - // first. this.router.set(router); - this.arena.set(arena); // `js.routesSetCached` — wired via `codegen_cached_accessors!` above. routes_set_cached(this_value, global_this, JSValue::ZERO); Ok(this_value) diff --git a/test/js/bun/css/nth-anplusb-ident.test.ts b/test/js/bun/css/nth-anplusb-ident.test.ts index 380268eabacb..e5da6cb55c12 100644 --- a/test/js/bun/css/nth-anplusb-ident.test.ts +++ b/test/js/bun/css/nth-anplusb-ident.test.ts @@ -1,4 +1,5 @@ import { cssInternals } from "bun:internal-for-testing"; +import { heapStats } from "bun:jsc"; import { expect, test } from "bun:test"; import { bunEnv, bunExe } from "harness"; @@ -27,6 +28,27 @@ test("class and id selectors inside an of-list keep their escapes when printed", expect(minifyTest(":nth-last-child(1 of #a\\}b) {width: 20px}", "")).toBe(":nth-last-child(1 of #a\\}b){width:20px}"); }); +// `2n-3` is one dimension token with the unit `n-3`. `n-5`, `-n-5` and the `n-7` of `+n-7` are one ident token. The +// parser reads the digits after `n-` with a second tokenizer, which borrows the arena of the stylesheet. +test.each([ + ["2n-3", "2n-3"], + ["n-5", "n-5"], + ["-n-5", "-n-5"], + ["+n-7", "n-7"], +])(":nth-child(%s) creates no allocator heap per selector", (arg, printed) => { + // `heaps.total` counts `mi_heap_new()` calls. A thread that starts adds to `theaps`, not to `heaps`. + const heapsCreatedByRules = (count: number) => { + const rules = (arg: string, separator: string) => + Array.from({ length: count }, (_, i) => `.a${i}:nth-child(${arg}){width:${i + 1}px}`).join(separator); + const before = heapStats().mimalloc.heaps.total; + const output = minifyTest(rules(arg, "\n"), ""); + const heaps = heapStats().mimalloc.heaps.total - before; + expect(output).toBe(rules(printed, "")); + return heaps; + }; + expect(heapsCreatedByRules(100)).toBe(heapsCreatedByRules(1)); +}); + test("fuzzer-minimized input: unterminated :nth-child( with an `Nn` ident", async () => { // Run in a child process so a crash doesn't take down the test runner. await using proc = Bun.spawn({ diff --git a/test/js/bun/util/filesystem_router.test.ts b/test/js/bun/util/filesystem_router.test.ts index e8603999512d..92d1d8bb7e55 100644 --- a/test/js/bun/util/filesystem_router.test.ts +++ b/test/js/bun/util/filesystem_router.test.ts @@ -1,4 +1,5 @@ import { FileSystemRouter } from "bun"; +import { heapStats } from "bun:jsc"; import { expect, it } from "bun:test"; import fs, { mkdirSync, rmSync } from "fs"; import { bunEnv, bunExe, isASAN, isMacOS, isWindows, normalizeBunSnapshot, tempDir, tmpdirSync } from "harness"; @@ -427,6 +428,29 @@ it("reload() works with new dirs/files", () => { expect(router.match("/test/test2")!.name).toBe("/test/test2"); }); +it("the constructor and reload() create no allocator heap", () => { + const { dir } = make(["index.tsx", "posts/[id].tsx"]); + // `heaps.total` counts `mi_heap_new()` calls. A thread that starts adds to `theaps`, not to `heaps`. + const heapsCreatedBy = (fn: () => void) => { + const before = heapStats().mimalloc.heaps.total; + for (let i = 0; i < 50; i++) fn(); + return heapStats().mimalloc.heaps.total - before; + }; + + for (const fileExtensions of [undefined, [".tsx", ".ts"]]) { + const create = () => new FileSystemRouter({ dir, style: "nextjs", fileExtensions }); + // The first router fills the resolver's directory cache. + const router = create(); + router.reload(); + + expect({ + create: heapsCreatedBy(create), + reload: heapsCreatedBy(() => router.reload()), + }).toEqual({ create: 0, reload: 0 }); + expect(Object.keys(router.routes).sort()).toEqual(["/", "/posts/[id]"]); + } +}); + it(".query works with dynamic routes, including params", () => { // set up the test const { dir } = make(["posts/[id].tsx"]);