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
15 changes: 7 additions & 8 deletions src/css/css_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down Expand Up @@ -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)));
Expand All @@ -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)));
Expand Down Expand Up @@ -4011,21 +4011,20 @@ pub mod nth {
Err(input.new_unexpected_token_error(tok))
}

fn parse_n_dash_digits(str: &[u8]) -> Maybe<i32, ()> {
fn parse_n_dash_digits(arena: &Bump, str: &[u8]) -> Maybe<i32, ()> {
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<i32, ()> {
let arena = Bump::new();
let mut input = ParserInput::new(string, &arena);
fn parse_number_saturate(arena: &Bump, string: &[u8]) -> Maybe<i32, ()> {
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,
Expand Down
26 changes: 4 additions & 22 deletions src/runtime/api/filesystem_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]
Expand All @@ -92,10 +91,6 @@ pub struct FileSystemRouter {
pub(crate) origin: Option<BackRef<RefString>>,
pub(crate) base_dir: Option<BackRef<RefString>>,
pub(crate) router: JsCell<Router::Router>,
// 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<Box<ArenaAllocator>>,
pub(crate) asset_prefix: Option<BackRef<RefString>>,
}

Expand Down Expand Up @@ -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<Box<[u8]>> = 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!(
Expand All @@ -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..]));
}
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -436,7 +423,6 @@ impl FileSystemRouter {
) -> JsResult<JSValue> {
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();

Expand Down Expand Up @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions test/js/bun/css/nth-anplusb-ident.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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({
Expand Down
24 changes: 24 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 { 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";
Expand Down Expand Up @@ -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"]);
Expand Down
Loading