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
27 changes: 21 additions & 6 deletions src/bun_alloc/MimallocArena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ fn debug_thread_stamp() -> u64 {
ID.with(|id| *id)
}

/// What [`MimallocArena::usage`] measures.
#[derive(Clone, Copy, Default, Debug, PartialEq, Eq)]
pub struct ArenaUsage {
/// Bytes of the blocks in use.
pub allocated: usize,
/// Bytes of the blocks, in use or free, that the heap's pages have made available.
pub committed: usize,
}

/// A mimalloc heap. Owns a `mi_heap_t`; all allocations are bulk-freed on
/// `Drop` via `mi_heap_destroy`.
///
Expand Down Expand Up @@ -277,6 +286,11 @@ impl MimallocArena {
/// in this heap. Walks the heap's areas (not its individual blocks); cost
/// is O(areas), which is cheap. Intended for GC `estimatedSize` reporting.
pub fn allocated_bytes(&self) -> usize {
self.usage().allocated
}

/// One walk of the heap's areas: [`Self::allocated_bytes`] and what the pages hold for it.
pub fn usage(&self) -> ArenaUsage {
extern "C" fn visit(
_heap: *const mimalloc::Heap,
area: *const mimalloc::mi_heap_area_t,
Expand All @@ -285,24 +299,25 @@ impl MimallocArena {
arg: *mut c_void,
) -> bool {
// SAFETY: mimalloc passes a valid `area` for each heap area when
// `visit_all_blocks == false`; `arg` is the `&mut usize` we passed.
// `visit_all_blocks == false`; `arg` is the `&mut ArenaUsage` we passed.
unsafe {
let total = &mut *arg.cast::<usize>();
*total += (*area).used.saturating_mul((*area).full_block_size);
let usage = &mut *arg.cast::<ArenaUsage>();
usage.allocated += (*area).used.saturating_mul((*area).full_block_size);
usage.committed += (*area).committed;
}
true
}
let mut total: usize = 0;
let mut usage = ArenaUsage::default();
// SAFETY: `self.heap` is live; `visit` upholds the callback contract.
unsafe {
mimalloc::mi_heap_visit_blocks(
self.heap_ptr(),
false,
Some(visit),
(&raw mut total).cast(),
(&raw mut usage).cast(),
);
}
total
usage
}

/// `mi_heap_malloc[_aligned]` on this
Expand Down
2 changes: 1 addition & 1 deletion src/bun_alloc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub use baby_vec::BabyVec;
/// through `<&MimallocArena as Allocator>` (= `mi_heap_realloc_aligned` /
/// `mi_free`); reclaimed on arena `reset`/`Drop`.
pub type ArenaVec<'a, T> = BabyVec<'a, T>;
pub use mimalloc_arena::{ArenaString, ArenaVecExt};
pub use mimalloc_arena::{ArenaString, ArenaUsage, ArenaVecExt};

/// `bumpalo::collections::Vec::from_iter_in` parity for [`ArenaVec`].
#[inline]
Expand Down
16 changes: 16 additions & 0 deletions src/jsc/rare_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,8 @@ pub struct RareData {
websocket_inflate_scratch: Option<Vec<u8>>,
/// One libdeflate handle for every JS-thread one-shot inflate; see [`Self::libdeflate_decompressor`].
libdeflate_decompressor: Option<libdeflate::OwnedDecompressor>,
/// Arena of the last finished `Bun.$` script and its `usage()`; see [`Self::take_shell_arena`].
shell_arena: Option<(bun_alloc::Arena, bun_alloc::ArenaUsage)>,

// There is intentionally no `aws_signature_cache` field — storage lives in
// `bun_s3_signing::credentials::AWS_SIGNATURE_CACHE` (process static; it
Expand Down Expand Up @@ -334,6 +336,7 @@ impl Default for RareData {
compression_scratch: None,
websocket_inflate_scratch: None,
libdeflate_decompressor: None,
shell_arena: None,
s3_default_client: Strong::empty(),
node_quic_callbacks: Strong::empty(),
default_csrf_secret: Box::default(),
Expand Down Expand Up @@ -678,6 +681,19 @@ impl RareData {
}
}

/// Arena for one `Bun.$` script's parse, with its `usage()`: an arena is a whole mimalloc heap, too costly per script.
pub fn take_shell_arena(&mut self) -> (bun_alloc::Arena, bun_alloc::ArenaUsage) {
self.shell_arena.take().unwrap_or_default()
}

/// Keeps the first arena returned, and only while its pages (dead ASTs, free blocks) hold at most `KEEP`.
pub fn put_back_shell_arena(&mut self, arena: bun_alloc::Arena, usage: bun_alloc::ArenaUsage) {
const KEEP: usize = 256 * 1024;
if self.shell_arena.is_none() && usage.committed <= KEEP {
self.shell_arena = Some((arena, usage));
}
}

/// libdeflate keeps no state between calls, so one handle serves every one-shot inflate on
/// this thread. `None` when libdeflate cannot allocate it; callers fall back to zlib.
pub fn libdeflate_decompressor(&mut self) -> Option<&mut libdeflate::Decompressor> {
Expand Down
8 changes: 4 additions & 4 deletions src/runtime/shell/ParsedShellScript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,6 @@ fn create_parsed_shell_script_impl(
callframe: &CallFrame,
marked_argument_buffer: &mut MarkedArgumentBuffer,
) -> JsResult<JSValue> {
// Box<ShellArgs> drops automatically on every early `return`/`?` below,
// so no scopeguard is needed.
let mut shargs: Box<ShellArgs> = ShellArgs::init();

let arguments = callframe.arguments();
if arguments.len() < 2 {
return Err(global.throw_not_enough_arguments("Bun.$", 2, arguments.len()));
Expand All @@ -233,6 +229,10 @@ fn create_parsed_shell_script_impl(
marked_argument_buffer,
)?;

// Box<ShellArgs> drops on every early `return` below, so no scopeguard is needed.
let (arena, arena_usage) = global.bun_vm().as_mut().rare_data().take_shell_arena();
let mut shargs: Box<ShellArgs> = ShellArgs::init(arena, arena_usage);

// Reshaped for borrowck — `out_parser`/`out_lex_result` borrow
// `shargs.__arena`, so they're scoped to a block that ends before
// `shargs.script_ast = script` below. The arena reference is taken via raw
Expand Down
82 changes: 46 additions & 36 deletions src/runtime/shell/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,25 +355,40 @@ impl ShellArgs {
/// Heap-allocated (returned as `Box`) because the interpreter stores
/// `Box<ShellArgs>` and state nodes hold `*const ast::*` into the arena;
/// the box must not move once `parse()` has filled `script_ast`.
pub(crate) fn init() -> Box<ShellArgs> {
pub(crate) fn init(
arena: bun_alloc::Arena,
arena_usage: bun_alloc::ArenaUsage,
) -> Box<ShellArgs> {
Box::new(ShellArgs {
__arena: bun_alloc::Arena::new(),
__arena: Some(arena),
arena_usage,
ast_bytes: 0,
// Overwritten by `parse()` before `run()`. An empty stmt list is
// a safe placeholder.
script_ast: ast::Script { stmts: &[] },
})
}

/// For the parse only: [`Self::set_script_ast`] measures the arena, and nothing allocates from it after.
#[inline]
pub(crate) fn arena(&self) -> &bun_alloc::Arena {
&self.__arena
self.__arena
.as_ref()
.expect("the arena is taken after the script has finished")
}

/// Gives up the arena, with its `usage()`, and the AST in it: for a script that has finished.
pub(crate) fn take_arena(&mut self) -> Option<(bun_alloc::Arena, bun_alloc::ArenaUsage)> {
self.script_ast = ast::Script { stmts: &[] };
self.ast_bytes = 0;
let arena = self.__arena.take()?;
debug_assert_eq!(arena.allocated_bytes(), self.arena_usage.allocated);
Some((arena, self.arena_usage))
}

/// Store the parsed AST root alongside its owning arena. This is the single
Comment thread
robobun marked this conversation as resolved.
Resolved
/// self-referential lifetime-erasure point: `script` borrows `self.__arena`
/// for `'a`, but `ShellArgs` is heap-allocated and the arena is never moved
/// or dropped while the interpreter (and thus every state node holding
/// `*const ast::*`) is live. Widening `'a` → `'static` here is therefore
/// for `'a`, and the arena stays here until [`Self::take_arena`] empties `script_ast`, so widening to `'static` is
/// sound — every later dereference happens through raw pointers under
/// `unsafe`, which is where the real invariant is checked.
///
Expand All @@ -385,23 +400,25 @@ impl ShellArgs {
pub(crate) fn set_script_ast(&mut self, script: bun_shell_parser::ast::Script<'_>) {
// `ast::Script` is `bun_shell_parser::ast::Script<'static>` — identical
// type, only the lifetime parameter differs. `self.__arena` owns every
// node `script.stmts` references and is dropped only when this
// `ShellArgs` is, so the widened references remain valid for the
// interpreter's lifetime. Re-construct field-by-field via a slice
// node `script.stmts` references until `take_arena`, after the last state node is done. Re-construct via a slice
// pointer cast (`Stmt<'a>` and `Stmt<'static>` are layout-identical).
let stmts = script.stmts;
// SAFETY: lifetime-only widen; arena outlives `self` (see above).
// SAFETY: lifetime-only widen; `take_arena` empties `script_ast` before the arena leaves `self`.
let stmts: &'static [ast::Stmt] =
unsafe { core::slice::from_raw_parts(stmts.as_ptr().cast::<ast::Stmt>(), stmts.len()) };
self.script_ast = ast::Script { stmts };
// `usage()` walks the heap's pages, so it runs once per script.
let usage = self.arena().usage();
self.ast_bytes = usage.allocated.saturating_sub(self.arena_usage.allocated);
self.arena_usage = usage;
}

/// Reports the arena's `allocated_bytes()` (a superset — tokens + strpool
/// Reports what the parse added to the arena (a superset — tokens + strpool
/// + AST nodes). This is for GC `estimatedSize` reporting only, where
/// over-approximation is preferable to a tree walk on a lifetime-erased
/// AST mirror.
pub(crate) fn memory_cost(&self) -> usize {
core::mem::size_of::<ShellArgs>() + self.__arena.allocated_bytes()
core::mem::size_of::<ShellArgs>() + self.ast_bytes
}
}

Expand Down Expand Up @@ -678,7 +695,7 @@ impl Interpreter {
// We are the script's shell (`bun run <script>`, `bun exec`, `bun x.sh`).
bun_spawn::ctrl_c::install();

let mut shargs = ShellArgs::init();
let mut shargs = ShellArgs::init(bun_alloc::Arena::new(), bun_alloc::ArenaUsage::default());

// ── parse ──────────────────────────────────────────────────────────
// `out_parser`/`out_lex_result` borrow `shargs.__arena`, so they're
Expand Down Expand Up @@ -1484,27 +1501,16 @@ impl Interpreter {
self.root_shell.with_mut(|rs| rs.deinit_embedded(false));
}

// Note: free the parse arena eagerly. `bun_alloc::Arena` is
// a `MimallocArena` (a full `mi_heap_t`): every shell parse pulls
// several fresh 64 KiB pages, and with `MI_DEBUG=3` each page-init
// runs `mi_assert_expensive(mi_mem_is_zero(page, 64 KiB))`. Under the
// shell-load.test.ts fixture (30 000 back-to-back `$\`...\`` calls)
// those pages are held until JSC finalizes the wrapper, so RSS grows
// ~220 KiB per pending interpreter and >50 % of CPU is spent
// re-scanning freshly-mmap'd zero pages. Resetting here returns the
// pages to mimalloc's pool immediately so subsequent parses reuse them
// (`memid.initially_zero == false` → the expensive scan is skipped)
// and memory stays flat. The new heap created by `reset()` allocates
// no pages until first use, so the per-interpreter footprint left for
// the finalizer is just the bare `mi_heap_t`. `script_ast` is cleared
// first because every node it references lives in the arena being
// destroyed; nothing dereferences it after this point (the only
// remaining reader is `memory_cost()`, which queries
// `__arena.allocated_bytes()` and never touches the AST).
self.args.with_mut(|a| {
a.script_ast = ast::Script { stmts: &[] };
a.__arena.reset();
});
// Nothing dereferences the AST after this point: the arena goes to the VM now, not when JSC finalizes the wrapper.
if let Some((arena, usage)) = self.args.with_mut(|a| a.take_arena())
&& let Some(global) = self.global_this_ref()
{
global
.bun_vm()
.as_mut()
.rare_data()
.put_back_shell_arena(arena, usage);
}

self.this_jsvalue.set(crate::jsc::JSValue::ZERO);
self.cleanup_state.set(CleanupState::RuntimeCleaned);
Expand Down Expand Up @@ -2156,8 +2162,12 @@ impl ShellExecEnv {
// ────────────────────────────────────────────────────────────────────────────

pub struct ShellArgs {
/// Arena owning the parsed AST nodes, tokens, and string pool.
pub(crate) __arena: bun_alloc::Arena,
/// Arena owning the parsed AST nodes, tokens, and string pool. `None` after [`Self::take_arena`].
__arena: Option<bun_alloc::Arena>,
/// `__arena.usage()` after the parse; a parked arena comes with the dead ASTs of earlier scripts.
arena_usage: bun_alloc::ArenaUsage,
/// The part of `arena_usage.allocated` that this script's parse added.
ast_bytes: usize,
/// Root AST node. State nodes hold `*const ast::*` into this arena.
pub(crate) script_ast: ast::Script,
}
Expand Down
45 changes: 45 additions & 0 deletions test/js/bun/shell/shell-leak-args.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { $ } from "bun";
import { heapStats } from "bun:jsc";
import { expect, test } from "bun:test";
import { isASAN, rss } from "harness";

// The arena a script is parsed into is a whole mimalloc heap, so this counts
// the scripts that had to create one.
const mimallocHeapsCreated = (): number => heapStats().mimalloc.heaps.total;

test("shell parsing error does not leak emmory", async () => {
const buffer = Buffer.alloc(1024 * 1024, "A").toString();
for (let i = 0; i < 5; i++) {
Expand Down Expand Up @@ -82,3 +87,43 @@ test("non-awaited shell command does not leak argv", async () => {
// original 250 MiB threshold elsewhere.
expect(after - before).toBeLessThan(isASAN ? 450 : 250);
});

test("a finished script gives its parse arena to the next script", async () => {
await $`echo warmup`.quiet();
const before = mimallocHeapsCreated();
for (let i = 0; i < 100; i++) {
expect(await $`echo ${i} && true`.text()).toBe(`${i}\n`);
}
// Two heaps per script when every script creates and destroys its own.
expect(mimallocHeapsCreated() - before).toBeLessThan(10);
});

// The cap is on what the arena's pages hold. The second script's AST is under it,
// but the buffers its lexer grew and freed are not.
test.each([
["with an AST over the cap", `echo ${Buffer.alloc(1024 * 1024, "bun!").toString()}`],
["that leaves large free blocks", `echo '${Buffer.alloc(100 * 1024, "x").toString()}'`],
])("the arena of a script %s is not kept", async (_, script) => {
await $`${{ raw: script }}`.quiet();
const before = mimallocHeapsCreated();
for (let i = 0; i < 10; i++) {
await $`${{ raw: script }}`.quiet();
}
const created = mimallocHeapsCreated() - before;
// Its arena is destroyed at finish, and the next script starts a new one.
expect(created).toBeGreaterThanOrEqual(10);
expect(created).toBeLessThan(20);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
});

test("the arena of a script that fails to parse is not kept", async () => {
await $`echo warmup`.quiet();
const before = mimallocHeapsCreated();
for (let i = 0; i < 10; i++) {
expect(() => $`echo ${i} <!INVALID ==== SYNTAX!>`).toThrow("Redirection with no file");
await $`echo ${i}`.quiet();
}
const created = mimallocHeapsCreated() - before;
// The failed parse destroys the arena it took, and the script after it starts a new one.
expect(created).toBeGreaterThanOrEqual(10);
expect(created).toBeLessThan(20);
});
Loading