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
5 changes: 2 additions & 3 deletions src/ast/ast_memory_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,8 @@ impl ASTMemoryAllocator {

/// Per-iteration reset for hot reuse paths (`initialize_mini_store`'s
/// per-workspace-child re-entry). Thin delegate to
/// [`bun_alloc::Arena::reset_retain_with_limit`]; the cold init paths
/// (`bundler::ThreadPool::Worker::init`, `BundleThread::generate_in_new_
/// thread`) keep calling [`Self::reset`].
/// [`bun_alloc::Arena::reset_retain_with_limit`]; the cold init path
/// (`bundler::ThreadPool::Worker::init`) keeps calling [`Self::reset`].
Comment thread
robobun marked this conversation as resolved.
pub fn reset_retain_with_limit(&mut self, limit: usize) {
if self.arena_dirty {
debug_assert!(
Expand Down
67 changes: 15 additions & 52 deletions src/bundler/BundleThread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ pub(crate) struct BundleThread<C: Node> {
/// The trait accessors keep the generic `BundleThread<C>`
/// layout-agnostic. The concrete impl lives in T6 (`bun_bundler_jsc`).
pub trait CompletionStruct: Node + Send + 'static {
/// `bump` is the per-build mimalloc heap that backs `transpiler`, so the
/// two share lifetime `'a` (option fields like `optimize_imports: &'a
/// StringSet` borrow from `bump`).
/// `bump` is the per-build mimalloc heap the transpiler borrows from, so
/// the two share lifetime `'a`.
Comment thread
robobun marked this conversation as resolved.
fn configure_bundler<'a>(
&mut self,
transpiler: &mut Transpiler<'a>,
Expand All @@ -83,18 +82,12 @@ pub trait CompletionStruct: Node + Send + 'static {
/// struct.
fn as_js_bundle_completion_task(&mut self) -> dispatch::CompletionHandle;

/// `Transpiler<'a>` has borrow-carrying fields (`arena: &'a Arena`,
/// `resolver: Resolver<'a>`) that cannot be zero-init'd, so the allocate +
/// configure pair is folded into one trait call returning the
/// arena-allocated, fully-configured transpiler.
// The returned `&'a mut Transpiler<'a>` is arena-allocated via `bump.alloc(...)`
// (bumpalo `Bump`), which hands out `&mut` from `&self` through interior
// mutability — the standard arena pattern `mut_from_ref` cannot see through.
#[allow(clippy::mut_from_ref)]
/// Builds and configures the per-build transpiler. The box drops it on
/// every path.
Comment thread
robobun marked this conversation as resolved.
fn create_and_configure_transpiler<'a>(
&mut self,
bump: &'a Arena,
) -> Result<&'a mut Transpiler<'a>, crate::Error>;
) -> Result<Box<Transpiler<'a>>, crate::Error>;

/// Constructs the `BundleV2`, wires `plugins`/`completion`/`file_map`,
/// and runs the bundle.
Expand Down Expand Up @@ -272,22 +265,18 @@ impl<C: CompletionStruct> BundleThread<C> {
let heap = Arena::new();

let bump = &heap;
let ast_memory_store: &mut bun_ast::ASTMemoryAllocator =
bump.alloc(bun_ast::ASTMemoryAllocator::new(bump));
ast_memory_store.reset();
ast_memory_store.push();
let mut ast_memory_store = bun_ast::ASTMemoryAllocator::default();
let _ast_scope = ast_memory_store.enter();

// Allocate + configure folded — see `create_and_configure_transpiler` doc.
let transpiler = completion.create_and_configure_transpiler(bump)?;
let mut transpiler = completion.create_and_configure_transpiler(bump)?;

transpiler.resolver.generation = generation;

// Construction + run delegated — see
// `init_and_run` doc. Reborrow `transpiler` through a raw ptr so
// `completion` can be borrowed again below.
let transpiler_ptr: *mut Transpiler<'_> = transpiler;
let transpiler_ptr: *mut Transpiler<'_> = &raw mut *transpiler;
let run = completion.init_and_run(
// SAFETY: `transpiler` lives in `bump` for the duration of `heap`.
// SAFETY: `init_and_run` wants `&'a mut Transpiler<'a>`; the box
// outlives the call and is not touched again until it drops.
unsafe { &mut *transpiler_ptr },
bump,
// `WorkPool::get()` returns `&'static ThreadPool`; pass as raw so
Expand All @@ -301,43 +290,17 @@ impl<C: CompletionStruct> BundleThread<C> {
// `deinitWithoutFreeingArena` + wait-group drain live inside `init_and_run`
// (it owns `this`).
let mut out_log = bun_ast::Log::init();
// SAFETY: `transpiler.log` is the arena-allocated `*mut Log` set up by
// `configure_bundler`; valid for the lifetime of `heap`. Raw deref so the
// `&'a mut Transpiler` consumed by `init_and_run` above is not reborrowed.
// SAFETY: `transpiler.log` points at the completion task's own `Log`,
// which its owner keeps alive until `complete_on_bundle_thread`. Raw
// deref so the `&'a mut Transpiler` given to `init_and_run` is not
// reborrowed.
let _ = unsafe { (*(*transpiler_ptr).log).append_to_with_recycled(&mut out_log, true) }; // logger OOM-only
completion.set_log(out_log);

if run.is_ok() {
completion.complete_on_bundle_thread();
}

ast_memory_store.pop();

// `transpiler` / `ast_memory_store` are arena-allocated, but their
// containers (`Resolver` caches, `BundleOptions` strings, the AST
// allocator's own `mi_heap` handle, …) live on the global heap as
// `Vec`/`Box`/`HashMap`, so dropping `heap` (`mi_heap_destroy`) reclaims
// the struct bytes but never runs `Transpiler::drop` /
// `ASTMemoryAllocator::drop` — leaking the resolver's directory/file
// caches and an entire `mi_heap` per `Bun.build()` call. LSan does not
// flag the latter (mimalloc bypasses the ASAN `malloc` interceptor), so
// the symptom is RSS-only: ~32 MB/build linear growth in the
// bun-build-api "does not leak sourcemap JSON" test.
//
// SAFETY: both pointers are the unique `&'a mut` slots returned by
// `bump.alloc(...)` above; nothing else holds a reference to either
// past `init_and_run` (`set_transpiler` was cleared by
// `deinit_without_freeing_arena`, `pop()` restored the AST-allocator
// thread-local). The arena bytes themselves are bulk-freed afterwards
// by `heap`'s `Drop` — `drop_in_place` only releases the *embedded
// global-heap* state, so there is no double free.
unsafe {
core::ptr::drop_in_place(transpiler_ptr);
core::ptr::drop_in_place(std::ptr::from_mut::<bun_ast::ASTMemoryAllocator>(
ast_memory_store,
));
}

run
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/bundler/transpiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,12 @@ pub struct Transpiler<'a> {
}

impl<'a> Transpiler<'a> {
/// Takes `*mut Log` (not `&'a mut`) because the same
/// `*Log` is aliased into `linker.log` / `resolver.log`; the struct
/// field is a raw pointer for that reason.
/// Takes `*mut Log` (not `&'a mut`) because the same `*Log` is aliased
/// into `options.log` / `linker.log` / `resolver.log`.
Comment thread
robobun marked this conversation as resolved.
pub fn set_log(&mut self, log: *mut bun_ast::Log) {
self.log = log;
self.options.log = log;
Comment thread
robobun marked this conversation as resolved.
self.linker.log = log;
// SAFETY: caller (`ThreadPool::Worker::create`) passes the per-worker
// arena-allocated `Log`, which outlives this `Transpiler<'a>`.
self.resolver.log = core::ptr::NonNull::new(log).expect("set_log: log is non-null");
}

Expand Down
22 changes: 6 additions & 16 deletions src/runtime/api/js_bundle_completion_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ impl CompletionStruct for JSBundleCompletionTask {
fn create_and_configure_transpiler<'a>(
&mut self,
bump: &'a Arena,
) -> bun_bundler::Result<&'a mut Transpiler<'a>> {
) -> bun_bundler::Result<Box<Transpiler<'a>>> {
let config = &self.config;
let opts = api::TransformOptions {
define: if config.define.count() > 0 {
Expand Down Expand Up @@ -1277,19 +1277,10 @@ impl CompletionStruct for JSBundleCompletionTask {
};

let log: *mut bun_ast::Log = &raw mut self.log;
let t = Transpiler::init(bump, log, opts, Some(self.env))?;
let transpiler: &'a mut Transpiler<'a> = bump.alloc(t);

// Post-init field wiring.
// Reborrow through a raw ptr so `&mut self` is usable
// again after handing `&'a mut Transpiler` (which is tied to `bump`,
// not `self`) to the trait method.
let tp: *mut Transpiler<'a> = transpiler;
// SAFETY: `tp` aliases nothing in `self`; lives in `bump`.
self.configure_bundler(unsafe { &mut *tp }, bump)?;
// SAFETY: `tp` was the unique `&'a mut` slot from `bump.alloc`; the
// reborrow above has ended.
Ok(unsafe { &mut *tp })
// `configure_linker` stores field addresses: box first, then configure.
let mut transpiler = Box::new(Transpiler::init(bump, log, opts, Some(self.env))?);
self.configure_bundler(&mut transpiler, bump)?;
Ok(transpiler)
}

fn init_and_run<'a>(
Expand Down Expand Up @@ -1342,8 +1333,7 @@ impl CompletionStruct for JSBundleCompletionTask {

let run = bv2.run_from_js_in_new_thread(&entry_points);

// The AST-allocator pop lives in `generate_in_new_thread`; the
// source-map wait-group waits run only on the error path.
// The source-map wait-group waits run only on the error path.
match run {
Ok(build) => {
self.set_result(BundleV2Result::Value(build));
Expand Down
45 changes: 45 additions & 0 deletions test/bundler/bun-build-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,51 @@ export { greeting };`,
});
});

// `Bun.build()` and `new Bun.Transpiler()` create a `Transpiler` before they
// validate every option. When validation fails (here: a `define` value that is
// not valid JSON) the error path must still drop it.
test.skipIf(!isASAN)("Bun.build / Bun.Transpiler option error path does not leak the transpiler", async () => {
using dir = tempDir("bun-build-configure-err-leak", { "e.js": "0;\n" });
const iters = 3;
const script = `
const entry = ${JSON.stringify(join(String(dir), "e.js"))};
const bad = { X: '{"a":' };
let rejected = 0;
for (let i = 0; i < ${iters}; i++) {
try { await Bun.build({ entrypoints: [entry], define: bad }); } catch { rejected++; }
try { new Bun.Transpiler({ define: bad }); } catch { rejected++; }
}
console.log("rejected", rejected);
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script],
env: {
...bunEnv,
// Bun's built-in ASAN defaults turn LSan off. Destructing the VM on exit
// frees what JS still referenced, so only lost allocations get reported.
// abort_on_error=0 so a leak exits 1 instead of raising SIGABRT.
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1", "abort_on_error=0"].filter(Boolean).join(":"),
// verbosity=1 makes the exit-time check announce itself on stderr. The
// binary's built-in suppressions cover the detached bundler thread; the
// shared test/leaksan.supp is not used so a new entry there cannot hide
// this leak.
LSAN_OPTIONS: "verbosity=1",
BUN_DESTRUCT_VM_ON_EXIT: "1",
},
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// Both calls must reach the option error, or the test measures nothing.
expect(stdout.trim()).toBe(`rejected ${iters * 2}`);
expect(stderr).toContain("LeakSanitizer: checking for leaks");
// LSan prints one blank-line-separated block per leaking allocation stack.
// Before the fix each failed `Bun.build` left about 200 of them.
const leaks = stderr.split(/\n\s*\n/).filter(block => /^(?:Direct|Indirect) leak of /.test(block));
expect(leaks).toEqual([]);
expect({ exitCode, signalCode: proc.signalCode }).toEqual({ exitCode: 0, signalCode: null });
});

// On release builds mimalloc's large-allocation arenas make RSS growth too
// non-deterministic to draw a clean line between "leaking" and "not leaking"
// for this path. Under debug/ASAN the allocator behaviour is stable enough to
Expand Down
25 changes: 25 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2534,6 +2534,31 @@ export default <>hi</>
).toThrow();
});

it("invalid define value surfaces the JSON parser diagnostic", () => {
// `configure_defines` writes the parse error into `options.log`, which
// `set_log` used to leave pointing at the constructor's moved-from stack
// slot — so `config.log` stayed empty and the generic `throw_error` path
// was taken instead of `log.to_js`.
let err;
try {
new Bun.Transpiler({ define: { X: '{"a":' } });
} catch (e) {
err = e;
}
expect(err).toBeDefined();
expect(err.message).not.toContain("ParserError");
expect(err.message).toContain("Unexpected end of file");
expect(err.position).toEqual({
lineText: '{"a":',
file: "defines.json",
namespace: "internal",
line: 1,
column: 5,
length: 0,
offset: 5,
});
});

it("define with an empty-string key is ignored without leaving uninitialized slots", async () => {
// `JSPropertyIterator` skips empty-name properties, but `names`/`values` were being
// indexed by the property position instead of a dense counter, leaving garbage in the
Expand Down
Loading