Fix effectively every native-code memory leak in Bun - #30875
Merged
Merged
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR implements widespread ASAN-aware allocator selection, arena-backed allocation migration, and explicit lifetime management across the Bun codebase. The global allocator conditionally selects between mimalloc and system allocator based on ASAN configuration. A new ChangesMonorepo allocator and arena lifetime refactor
Suggested reviewers
|
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/jsc/bindgen.rs`:
- Around line 359-371: The C++-allocated buffer pointed to by `data` can leak if
`Child::convert_from_extern` panics while iterating `unmanaged`; wrap the
`mi_free(data.cast())` call in an RAII guard (implement a small local struct
with `Drop` that calls `bun_alloc::mimalloc::mi_free` on `data.cast()`) created
before the loop so it always runs on unwind, and then explicitly disable/forget
the guard (or set a flag) after the loop when you manually free or transfer
ownership (so you don't double-free); update the block around
`unmanaged.iter_mut()` / `Child::convert_from_extern` to use this guard and
ensure `data` is treated as owned by the guard until success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 082cc08c-a39d-4f76-ad55-5ae92575c402
📒 Files selected for processing (4)
src/bun_alloc/lib.rssrc/bun_bin/lib.rssrc/bun_core/lib.rssrc/jsc/bindgen.rs
Comment on lines
359
to
+371
| let mut result = bun_core::handle_oom(Self::ZigType::init_capacity(length)); | ||
| for item in unmanaged { | ||
| // PERF(port): was appendAssumeCapacity — profile in Phase B | ||
| result.append_assume_capacity(Child::convert_from_extern(item)); | ||
| let mut unmanaged = ManuallyDrop::new(unmanaged); | ||
| for item in unmanaged.iter_mut() { | ||
| // SAFETY: each slot `i < length` holds a C++-initialized `ExternType`; | ||
| // `ManuallyDrop` ensures it isn't read twice. | ||
| result.append_assume_capacity(Child::convert_from_extern(unsafe { | ||
| core::ptr::read(item) | ||
| })); | ||
| } | ||
| // SAFETY: `data` is the live `mi_malloc`'d block handed over by C++ | ||
| // `ExternVectorTraits::convertToExtern` (it never returns a null `data` | ||
| // here — the null case is handled at the top of this function). | ||
| unsafe { bun_alloc::mimalloc::mi_free(data.cast()) }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
wc -l src/jsc/bindgen.rsRepository: oven-sh/bun
Length of output: 78
🏁 Script executed:
sed -n '350,380p' src/jsc/bindgen.rsRepository: oven-sh/bun
Length of output: 1629
🏁 Script executed:
# Get broader context around the function to understand error handling
sed -n '330,400p' src/jsc/bindgen.rsRepository: oven-sh/bun
Length of output: 3565
🏁 Script executed:
# Search for Child::convert_from_extern to understand if it can panic
rg "convert_from_extern" --max-count 20Repository: oven-sh/bun
Length of output: 3098
🏁 Script executed:
# Check for similar RAII/Drop-based cleanup patterns in the codebase
rg "impl Drop" src/ -A 3 | head -40Repository: oven-sh/bun
Length of output: 2391
🏁 Script executed:
# Look at specific convert_from_extern implementations in bindgen.rs
sed -n '150,250p' src/jsc/bindgen.rs | grep -A 5 "fn convert_from_extern"Repository: oven-sh/bun
Length of output: 908
🏁 Script executed:
# Check if any implementations use unwrap/expect that could panic
rg "convert_from_extern" src/jsc/bindgen.rs -A 3 | grep -E "(unwrap|expect|panic|unreachable)" | head -20Repository: oven-sh/bun
Length of output: 442
🏁 Script executed:
# Look for the trait definition and how it's used in the loop context
sed -n '1,150p' src/jsc/bindgen.rs | grep -A 10 "trait Bindgen\|convert_from_extern"Repository: oven-sh/bun
Length of output: 3627
🏁 Script executed:
# Check the exact line numbers for the fallback path to confirm context
sed -n '355,375p' src/jsc/bindgen.rsRepository: oven-sh/bun
Length of output: 1174
🏁 Script executed:
# Verify if there are any error paths that would cause convert_from_extern to return/panic
rg "BindgenArray" src/jsc/bindgen.rs -B 2 -A 15 | grep -E "(impl Bindgen|convert_from_extern)" | head -30Repository: oven-sh/bun
Length of output: 290
🏁 Script executed:
# Check for similar guard patterns in this specific file
rg "struct.*Guard" src/jsc/bindgen.rs -A 5Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look at the full BindgenArray impl to understand the full context
sed -n '290,375p' src/jsc/bindgen.rsRepository: oven-sh/bun
Length of output: 4726
Guard the fallback mi_free with Drop to prevent memory leaks on panic.
If Child::convert_from_extern unwinds during loop iteration, the C++-allocated mi_malloc buffer leaks because mi_free only runs after the loop completes. A Drop guard ensures cleanup happens regardless of panics. This aligns with the coding guideline: "Prefer RAII (Drop) over manual cleanup in Rust code".
♻️ Proposed fix
let mut result = bun_core::handle_oom(Self::ZigType::init_capacity(length));
let mut unmanaged = ManuallyDrop::new(unmanaged);
+ struct MiFreeGuard(*mut core::ffi::c_void);
+ impl Drop for MiFreeGuard {
+ fn drop(&mut self) {
+ unsafe { bun_alloc::mimalloc::mi_free(self.0) };
+ }
+ }
+ let free_guard = MiFreeGuard(data.cast());
for item in unmanaged.iter_mut() {
// SAFETY: each slot `i < length` holds a C++-initialized `ExternType`;
// `ManuallyDrop` ensures it isn't read twice.
result.append_assume_capacity(Child::convert_from_extern(unsafe {
core::ptr::read(item)
}));
}
- // SAFETY: `data` is the live `mi_malloc`'d block handed over by C++
- // `ExternVectorTraits::convertToExtern` (it never returns a null `data`
- // here — the null case is handled at the top of this function).
- unsafe { bun_alloc::mimalloc::mi_free(data.cast()) };
+ drop(free_guard);
result🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jsc/bindgen.rs` around lines 359 - 371, The C++-allocated buffer pointed
to by `data` can leak if `Child::convert_from_extern` panics while iterating
`unmanaged`; wrap the `mi_free(data.cast())` call in an RAII guard (implement a
small local struct with `Drop` that calls `bun_alloc::mimalloc::mi_free` on
`data.cast()`) created before the loop so it always runs on unwind, and then
explicitly disable/forget the guard (or set a flag) after the loop when you
manually free or transfer ownership (so you don't double-free); update the block
around `unmanaged.iter_mut()` / `Child::convert_from_extern` to use this guard
and ensure `data` is treated as owned by the guard until success.
![claude[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Jarred-Sumner
marked this pull request as draft
May 16, 2026 06:10
…nder ASAN Switching `#[global_allocator]` to `std::alloc::System` under ASAN broke every site that hands a `Vec`/`Box`-allocated pointer to `mi_free()` or a `mi_*`-backed vtable: JSC `ArrayBuffer` deallocators (`mi_free_bytes`/ `mi_free_ctx`), `bun_alloc::basic::C_ALLOCATOR`/`Z_ALLOCATOR`, the `GLOBAL_MIMALLOC_VTABLE`, `ZigString__free`, `BunString::deinit_global`, `MarkedArrayBuffer::destroy`, and the C++ finalizers in `BunString.cpp`, `Uint8Array.cpp`, `ZigSourceProvider.cpp`, and `MiString.h`. Under ASAN those buffers now come from libc malloc, so `mi_free` reports an invalid pointer and the bundler/dev-server tests SEGV. Add `bun_alloc::default_alloc` (Rust) and `Bun::defaultAllocatorFree` (C++) as the layout-agnostic raw alloc/free that always agree with the `#[global_allocator]` — `mi_*` normally, `libc::*` when `cfg(bun_asan)` — and rewire the conflation sites to use them. `MimallocArena` keeps real `mi_heap_*`/`mi_free` (its allocations never cross into the global allocator), and `dupe_z`/`free_sensitive_cstr`/`free_without_size` stay mimalloc-backed since they pair with `mi_malloc`'d (Rust or C++) buffers regardless of the global allocator.
…cfg(not(bun_asan))
…helpers The aligned alloc/realloc helpers used `if cfg!(bun_asan)` to pick between `mi_*_aligned` and a `posix_memalign`-backed libc path, but `if cfg!()` still type-checks both branches — and `libc::posix_memalign`/`malloc_usable_size` don't exist on Windows. Split the function bodies on `#[cfg(bun_asan)]` / `#[cfg(not(bun_asan))]` so the libc path is never compiled on targets where `bun_asan` cannot be set.
The CLI `bun build` path drops `BundleV2` without calling `deinit_without_freeing_arena()`, so `MultiArrayList<JSAst>`'s slab-only `Drop` strands the global-heap `Vec<Symbol>`/`Vec<Part>`/`Vec<ImportRecord>`/ `HashMap<Scope>` columns inside `graph.ast` and `linker.graph.meta` on every build. The `Bun.build()` runtime path already calls it (`js_bundle_completion_task.rs:1148`); this matches it for the CLI. Found by LeakSanitizer once the ASAN build switched the global allocator to `std::alloc::System` (mimalloc previously hid these allocations from LSan).
The "unmanaged" suffix is a Zig-ism — Zig's std bit set has no destructor and needs an explicit `deinit(allocator)`. In Rust, `Drop` is the right idiom for an owned heap allocation. The only reason it was held back was that `DynamicBitSetList::at()` hands out non-owning views whose `masks` pointer is interior to the list's shared buffer; running `deinit()` on those would free mid-allocation. Wrap those views in `ManuallyDrop` instead so the rest of the codebase (`LinkerGraph::files_live`/`is_scb_bitset`, `find_reachable_files` locals, `IncrementalGraph::stale_files`, …) gets the masks freed automatically on drop. Removes the now-redundant explicit `Drop` on `DynamicBitSet`.
… allocator With `std::alloc::System` as the global allocator under ASAN, LeakSanitizer now sees Rust allocations that mimalloc previously hid. Several entries in `__lsan_default_suppressions` had module paths that never matched actual demangled symbols — the resolver lives in `bun_resolver::__phase_a_body`, not `bun_resolver::resolver`, and the runtime transpiler store is `bun_jsc::runtime_transpiler_store`, not `bun_jsc::module_loader`. Pin those on the stable `Resolver>::<fn>` tail so a porting-artifact module rename doesn't silently re-break them. Also add a small batch of Rust-only entries for allocations reachable from process-lifetime structures that are intentionally never dropped (the arena-allocated `Transpiler` and `Resolver`, `RealFS::dir_cache`, the test scanner's directory walk, the runtime VM's `SavedSourceMap`, the bundler worker pool's deferred-free `Box<Worker>`s, and the parser's thread-local `StringVoidMap` pool — TLS roots LSan doesn't scan).
…rdown `TSNamespaceScope` and `TSNamespaceMemberMap` are `arena.alloc()`'d in `get_or_create_exported_namespace_members`, but their `StringArrayHashMap`s allocate column `Vec`s, `Box<[u8]>` keys, and the `Box<HashTable>` index from the global heap. The parser arena bulk-frees the structs without running `Drop`, stranding those allocations on every TypeScript namespace/enum. Two-part fix: - Route the column `Vec`s and key boxes through `AstAlloc` (no-op `deallocate`, reclaimed by the arena's `mi_heap_destroy`) — same pattern `ast_alloc.rs` introduced for `BabyList<T>` → `Vec<T>` arena leaks. - Track every arena-allocated scope/map in `P` and `mem::take` their `StringArrayHashMap` fields in `P::Drop` (covers the `index: Box<HashTable>` accelerator, which stays on `Global` regardless of `A`, and any path that bails before `to_ast`).
OutputFile::init Box::leak'd input_path because fs::Path<'static> only carries borrowed slices and the Zig deinit() that freed them wasn't ported. Hold the Box<[u8]> in a sibling owned_src_path_text field and have src_path.text borrow it — the Box's heap buffer has a stable address so the borrow stays valid across moves, and the field's Drop reclaims the bytes when the OutputFile drops. derive(Clone) → manual Clone so src_path's slices re-borrow the cloned buffer instead of pointing at the original's (which would dangle once it dropped). Also drop the BuildArtifact/TimeoutObject LSan suppressions — they weren't real false positives: the test runner sets BUN_DESTRUCT_VM_ON_EXIT=1, the VM is destructed, finalizers do run.
…hrough PackageJSONEditor)
…ditor, MultiArrayList::to_owned_slice Switching the global allocator to std::alloc::System under ASAN exposed several real leaks that the previous mimalloc backing had been hiding. Fix the reachable ones and drop the corresponding LSan suppressions. WTFTimer (src/jsc/bindings/ZigGlobalObject.cpp) Zig__GlobalObject__destructOnExit now grabs a Ref to the VM's RunLoop before tearing the VM down and calls runLoop->threadWillExit() after the final deref. ~VM enqueues the JSRunLoopTimer's RunLoop::Timer (which owns a Bun WTFTimer) onto the RunLoop's dispatch queue so it can be freed on its home thread, but Bun's RunLoop never drains that queue (RunLoop::wakeUp is a no-op for Kind::Bun). Draining pending dispatches at exit frees the Timer and its Box<WTFTimer>. dupe_resolved_path (src/jsc/VirtualMachine.rs) The _resolve fast-path duplicates were allocated via heap::into_raw and never freed. Store each Box<[u8]> in a new VirtualMachine::resolved_path_dups Vec so the bytes have a stable address for the life of the VM and get reclaimed in destroy() (e.g. under BUN_DESTRUCT_VM_ON_EXIT=1). The handed-out borrows are still erased to 'static to match the existing ResolveFunctionResult.path contract. PackageJSONEditor (src/install/PackageManager/PackageJSONEditor.rs) leak_str/leak_dup Box::leak'd EString backing storage through the global allocator. Replace them with arena_str/arena_dup backed by manager.ast_arena, the process-lifetime arena that is the Rust port's stand-in for Zig's manager.allocator. edit_patched_dependencies now uses the same arena instead of a frame-local bump arena. The edit_trusted_dependencies path drops the dupe entirely and borrows the caller-owned name, matching the Zig source. MultiArrayList::to_owned_slice (src/collections/multi_array_list.rs, src/install/lockfile/Tree.rs) to_owned_slice() transfers slab ownership to the caller but Slice<T> is Copy with no Drop, so the slab leaked. Add Slice::deinit_owned() which delegates to to_multi_array_list() and lets the reconstructed list's Drop free the slab. Tree::clean() now mem::take's each per-tree dependency Vec out of the SoA column (so it drops) and then calls slice.deinit_owned() to free the slab. Suppression cleanup (src/bun_bin/lib.rs) Remove the now-unneeded suppressions for bun_install::package_manager_real::package_json_editor and bun_install::lockfile_real::tree::Tree>::process_subtree, and tighten the remaining UpdateRequest comment. The CSS deep_clone leak is not addressed here; it needs follow-up in src/bundler/.
bundle_v2.rs: - Add a ResolveDeinitGuard mirroring Zig's `defer resolve.deinit()`. The Resolve is arena-allocated so its Drop never runs when the arena resets; the guard takes the owned import_record Box<[u8]> fields and any unconsumed ResolveValue on every exit path of on_resolve. - In the InputFile teardown loop, also drain the remaining heap-owning MultiArrayList columns (secondary_path, additional_files, unique_key_for_additional_file). MultiArrayList::drop is slab-only, so these Box/Vec payloads strand without an explicit take. computeCrossChunkDependencies.rs: - Build the ESM export ClauseItem buffer in c.arena() (ArenaVec) instead of a global-heap Vec that was mem::forget-ed into a raw fat ptr. The arena is bulk-reclaimed with the link pass, so LSan no longer sees a stranded allocation. resolver/fs.rs: - Add a LEAK NOTE on read_file_with_handle_impl explaining the intentional process-lifetime caches that retain the Cow::Owned contents (tsconfig intern, DirInfo arena, transpiler source cache), all already in the LSan suppression list. A plain drop frees the Vec; only a caller that mem::forget/into_raw-s PathContentsPair::contents would actually leak. runtime/webcore/blob/read_file.rs: - Add a LEAK NOTE on ReadFile::then enumerating both InternalReadFileFn consumers (Adapter<H> via heap::take, LoadFileAdapter via Box::<[u8]>::from_raw) plus the NewReadFileHandler path, documenting that ownership of the boxed slice always transfers to cb.
…_arena The `css: Option<StoreRef<BundlerStyleSheet>>` AST column points at a `StyleSheet` `bump.alloc()`'d in the worker arena (`ParseTask.rs`); the arena bulk-free won't run its `Drop`, stranding the global-heap fields inside (`sources: Vec<Box<[u8]>>`, `source_map_urls`, `layer_names`, `local_scope`, `composes`, `rules` and every owned `Vec`/`HashMap` within). LSan reported these via `css_parser::parse_with` and `parse_comma_separated_internal` once the global allocator switched to `std::alloc::System` under ASAN. Add a `drop_in_place` to the `take_ast_cols!` macro alongside the other arena-stored columns.
… annotations FileReader.rs / SubprocessPipeReader.rs: after a read error the BufferedReader never reaches done(), so the matching on_reader_done() that would balance the waiting_for_on_reader_done source ref never fires and the whole NewSource leaks. Release the in-flight start ref directly in the error path (skipping when on_cancel already drove a close that will release it). Verified against the streams test suites; the two streams.test.js timeouts and the streams-leak.test.ts timeout are pre-existing in debug builds (identical with the change stashed). extract_tarball.rs, shell_parser/parse.rs, test_runner/Order.rs, server/mod.rs, bundler/LinkerContext.rs, timer/mod.rs: comment-only. Add or expand LEAK(LSan) annotations documenting known intentional or cross-file leaks (arena-stranded Vecs, Box::into_raw'd execution entries, etc.) and the proper fix when it requires changes outside this batch. No behavior change. npm.rs and io/PipeReader.rs were analyzed and need no change at this layer. The CSS selector AstVec migration was reverted: it traded one leak for another in the bun:internal-for-testing path (css_internals.rs runs without an AST_HEAP scope), and the proper fix lives in a file outside this batch.
…Task union arm When a resolve task finishes, the manifest used to be deep-cloned into PackageManifestMap while the original stayed in task.data.package_manifest. The resolve-task pool reclaims that slot without running drop, so the boxed slices inside the original PackageManifest were leaked on every fetched package — and we paid a redundant clone of hundreds of KB to get there. Take ownership instead: ManuallyDrop::take the manifest out of the union arm (tag-guarded), write a default placeholder back so the slot stays well-formed for HiveArray::put's drop_in_place and reuse, and pass the owned manifest straight into PackageManifestMap::insert. The progress-bar display name is captured before the move since it was the only later use. Removes the data_package_manifest accessor from extern_union_accessors since the only call site now reads the union arm directly.
`CssChunk::asts: Box<[BundlerStyleSheet]>` holds bitwise-shallow copies of the per-file CSS ASTs (`prepareCssAstsForChunk` `ptr::read`s them so multiple chunk slots can alias the same heap buffers). The previous `Drop` `mem::forget`'d the whole slice to avoid the element-wise double-free — but that also strands the `Box<[T]>` slab itself (~480 B/element). LSan reported it via `compute_chunks::compute_chunks` once the global allocator switched to `std::alloc::System` under ASAN. Free the slab via `Vec::drop` after `set_len(0)`, which deallocates the buffer without running `BundlerStyleSheet::drop` on the aliased elements.
…buffers, SubprocessPipeReader, sourcemap Chunk clone, test runner Order Switching the global allocator to std::alloc::System under ASAN exposed several real leaks the previous mimalloc backing had been hiding: ParseTask (src/bundler/ParseTask.rs) CSS module path bump-allocates a CssAstRef that strands when the worker arena resets. Track and free it. postProcessJSChunk (src/bundler/linker_context/postProcessJSChunk.rs) Post-processed JS chunk buffers stranded in the chunk slab. Free them in the drop path. SubprocessPipeReader / Readable (src/runtime/api/bun/subprocess/Readable.rs) Pipe readers leaked when subprocesses were spawned and reaped before the JS Readable wrapper was GC'd. Tighten the refcount/free chain. sourcemap Chunk (src/sourcemap/Chunk.rs) Chunk clone leaked the mappings buffer. Add a Drop. test runner Order (src/runtime/test_runner/bun_test.rs) Test ordering data leaked. Free in teardown.
- postProcessCSSChunk: replace deep MutableString clone of source map chunk with an unsafe alias mirroring postProcessJSChunk, with SAFETY notes on lifetime and the no-realloc/set_capacity ordering invariant. The MultiArrayList only Drops the slab, so a deep clone stranded the inner heap buffer. - bun_bin: add LSan suppressions for BuildMessage::create, ResolveMessage::create, and TextDecoder::decode_slice, which hand ownership to GC-managed objects that LSan scans before the final GC sweep runs. - ast/lib.rs, MutableString.rs, unicode.rs: trim hardcoded line-number references in leak/lifetime comments, fix the drain-function name (Log::append_to_maybe_recycled), document the to_external_u16 too-long-branch caveat, and collapse the MutableString LEAK NOTE to a cross-reference now that the postProcessCSSChunk offender is fixed. - HTMLScanner.rs, options.rs: keep prior batch-6 changes as approved.
ArrayBuffer::to_js() registers MarkedArrayBuffer_deallocator only when mi_is_in_heap_region(ptr) is true. Under ASAN the global allocator is std::alloc::System (libc malloc), so the probe returns false for every Vec<u8>/Box<[u8]> buffer — including the file-read buffers that NewReadFileHandler hands to JSC. The buffer gets no deallocator and strands when the JS ArrayBuffer is GC'd at VM teardown (LSan reported 596 MB leaks in test/bake/deinitialization.test.ts: the fixture reads the bun executable itself for the embedded HMR runtime). Use to_js_unchecked() (which always registers the deallocator) in the three call sites that provably wrap an owned default-allocator Box<[u8]>: Blob::to_array_buffer_view_with_bytes::<Lifetime::Temporary>, and both ArrayBufferSink::end / ::flush. SubprocessPipeReader::on_reader_done / on_reader_error released the start() "in-flight read" ref only when self.process was still Some. Under BUN_DESTRUCT_VM_ON_EXIT=1, Readable::pipe_detach (run from the Subprocess GC finalizer) clears self.process first, so on_reader_done silently drops on the floor — the start ref strands and PipeReader (plus the PosixBufferedReader::_buffer accumulator, ~196 KB) is never freed. Always release the start ref.
… part compaction
- generateCompileResultForHtmlChunk: return Vec<Vec<u8>> instead of
BoundedArray<Vec<u8>, 2> from get_head_tags. BoundedArray is
[MaybeUninit<T>; N] with no Drop impl, so the inner Vec<u8>s for
<link>/<script>/<style> head tags were leaked on every HTML chunk.
- bundle_v2 on_load: add LoadDeinitGuard mirroring Zig's
`defer load.deinit()`. JSBundler::Load is arena-allocated so Drop
never runs when the arena resets; the guard frees the owned heap
fields (path, namespace, value) on every exit path including the
early returns.
- js_parser part compaction (timeout_object_p_init multi-pass loop):
- On the kept path, wipe the source slot's symbol_uses /
import_symbol_property_uses before the bitwise overwrite of
parts[parts_end], so a stale source slot re-scanned in a later
pass cannot alias a compacted survivor's heap maps.
- On the filtered path, drop the heap-backed maps via the
duplicate, then clear the alias in parts[idx] so the re-scan and
final set_len never observe freed handles. Previously the
filtered slot was silently abandoned by set_len with its maps
still allocated.
…r/FileReader Five fixes for heap allocations stranded by arena resets or skipped Drop paths, all surfaced by LeakSanitizer once Rust allocations became visible: - bundle_v2.rs: add LoadDeinitGuard so JSBundler::Load's owned Box<[u8]> path/namespace and unconsumed value are freed on every on_load exit path (Load is arena-allocated; Drop never runs). - generateCompileResultForHtmlChunk.rs: HTMLLoader::get_head_tags returned BoundedArray<Vec<u8>, 2>, which has no Drop impl over its MaybeUninit storage and stranded the Vec<u8> head tags. Switch to Vec<Vec<u8>>. - ParseTask.rs: bump-allocate the Framework byte slices (Cow::Borrowed) instead of Cow::Owned(Vec), since the containing Framework is bump- allocated and Drop never runs. - p.rs: in P::to_ast's part-filtering loop, free the global-heap symbol_uses/import_symbol_property_uses maps from filtered-out parts before abandoning the slot via set_len. - FileReader.rs: release the start() ref when reader.start() fails — otherwise the Source refcount never reaches zero and the buffered data leaks.
robobun
added a commit
that referenced
this pull request
Jun 6, 2026
Main's #30875 replaced the auto-derived Default impl for InputFile with an explicit one, which I missed in the last rebase — CI caught the missing-field on every build-rust lane. Add the None init. Fixes the build-rust failure in build #56341.
Jarred-Sumner
pushed a commit
that referenced
this pull request
Jun 6, 2026
…ion (#31905) Fixes #31903 ### Repro Debug builds panic during a dev server hot reload when a CSS file parses but fails import resolution (for example an unterminated `url(` that tokenizes to a URL, or a `url(./missing.png)` pointing at a file that does not exist): ``` panic: assertion failed: !chunk.content.is_css() bun_bundler::linker_context::generate_chunks_in_parallel::generate_chunks_in_parallel::<true> src/bundler/linker_context/generateChunksInParallel.rs:132 <bun_bundler::bundle_v2::BundleV2>::finish_from_bake_dev_server ``` Both of these existing tests crash on main with `DevServer crashed while waiting for hot reload`: ```bash bun bd test test/bake/dev/css.test.ts -t "syntax error crash" bun bd test test/bake/dev/css.test.ts -t "css import before create project relative" ``` ### Cause `graph.css_file_count` is only incremented when a parse task completes successfully with a CSS AST. When resolution fails after a successful CSS parse, `run_resolution_for_parse_task` converts the result to an error, and (since the leak fixes in #30875) moved the parsed stylesheet onto the graph row so teardown could free it. That comment assumed the linker never runs after such an error, but the dev server intentionally proceeds with failed files, and `finish_from_bake_dev_server` treats a populated `css` slot as "successfully parsed CSS" when discovering CSS entry points. The failed file was therefore re-added as a CSS entry point and got a CSS chunk while `css_file_count` stayed 0, tripping the `debug_assert!(!chunk.content.is_css())` in `generate_chunks_in_parallel`. In release builds the assert compiles out and the CSS dedup/prepare pass is silently skipped for that chunk. The Zig implementation never stored the CSS AST on this path, so the dev server's invariant held there. The parked AST also diverged from the reference in `find_imported_files_in_css_order` (a failed file's stale rules could be included in another chunk's import order) and `scan_css_imports`. ### Fix Drop the stylesheet at the failure site in `run_resolution_for_parse_task` instead of parking it on the graph row. The graph row stays `None` for failed files, restoring the invariant every dev server consumer of the `css` column relies on, and the stylesheet's non-arena allocations are still freed (same `drop_in_place` the teardown pass uses). ### Verification - New test `test/bake/dev/css.test.ts` "css url resolve error on hot reload is recoverable" fails on the unfixed build with `DevServer crashed while waiting for hot reload` and passes with the fix: a connected client asserts the exact error overlay text and the route returns 500, then recovery back to a 200 is checked after the error is fixed. Exercising the recovery with the client still connected hits a separate, pre-existing HMR patch bug (the HTML route module is shipped without the route-reload flag), tracked in #31908. - Full `test/bake/dev/css.test.ts` (14 tests, including the two previously-crashing ones), `test/bake/dev/html.test.ts`, `test/bake/dev/bundle.test.ts`, `test/bake/dev/hot.test.ts`, and `test/bake/dev/plugins.test.ts` pass on the debug/ASAN build. - Non-dev path (`Bun.build` with a CSS entry whose `url()` fails to resolve) still reports `Could not resolve` and is clean under ASAN.
robobun
added a commit
that referenced
this pull request
Jun 27, 2026
Main's #30875 replaced the auto-derived Default impl for InputFile with an explicit one, which I missed in the last rebase — CI caught the missing-field on every build-rust lane. Add the None init. Fixes the build-rust failure in build #56341.
Collaborator
|
Heads up: the |
robobun
added a commit
that referenced
this pull request
Jul 23, 2026
…n write completes On Windows, Bun.spawn/spawnSync with an ArrayBuffer/Blob stdin leaked one StaticPipeWriter per spawn in the common ordering where the uv_write completes before the child exits. StaticPipeWriter::start() takes a +1 and sets started=true. #30875 added a release_start_ref block to on_write() that clears started and derefs once the buffer drains, but gated it behind cfg(not(windows)). On Windows the write-complete path instead reaches on_close_io (via writer.close -> on_close_source -> StaticPipeWriter::on_close), which replaces stdin with Writable::Ignore and derefs create()'s +1 only. The other release site, take_pending_start_writer (called from on_process_exit / close_io), then finds stdin == Ignore and matches nothing, so start()'s +1 is stranded. Remove the cfg gate so on_write releases start()'s +1 on Windows too, and make take_pending_start_writer clear started when it claims the ref. The two sites are then mutually exclusive via started: whichever runs first clears the flag so the other is a no-op, covering the ordering where on_process_exit closes the pipe while a uv_write completion that already succeeded (so its callback reports status 0, not ECANCELED) is still queued.
robobun
added a commit
that referenced
this pull request
Jul 23, 2026
…l write path On Windows, Bun.spawn/spawnSync with an ArrayBuffer/Blob stdin leaked one StaticPipeWriter per spawn. StaticPipeWriter::start() takes a +1 and sets started=true. #30875 added a release_start_ref block to on_write() that clears started and derefs once the buffer drains, but gated it behind cfg(not(windows)). On Windows both terminal arms of WindowsBufferedWriter::on_write_complete reach on_close_io (via close() -> on_close_source -> StaticPipeWriter::on_close) with started still set, flipping stdin to Writable::Ignore so the other release site (take_pending_start_writer, called from on_process_exit / close_io) matches nothing and start()'s +1 is stranded. - on_write: drop the cfg(not(windows)) gates so release_start_ref runs on every platform. - on_close (Windows only): claim started for paths that reach close() without going through on_write (the on_write_complete error arm). write()'s +1, held by that callback's scopeguard, keeps self live past the deref. Gated to Windows because POSIX drain_buffered_data may call on_error() -> close() -> on_close and then on_write() on the same object with no extra ref held. - take_pending_start_writer: clear started when it claims the pointer so a uv_write whose I/O already completed before uv_close (delivered with its real status, not ECANCELED) does not double-release. - WindowsBufferedWriter::on_write_complete: saturating_sub for has_pending_data so the late-success-after-close ordering does not underflow in debug builds.
robobun
added a commit
that referenced
this pull request
Jul 23, 2026
…l write path On Windows, Bun.spawn/spawnSync with an ArrayBuffer/Blob stdin leaked one StaticPipeWriter per spawn. StaticPipeWriter::start() takes a +1 and sets started=true. #30875 added a release_start_ref block to on_write() that clears started and derefs once the buffer drains, but gated it behind cfg(not(windows)). On Windows both terminal arms of WindowsBufferedWriter::on_write_complete reach on_close_io (via close() -> on_close_source -> StaticPipeWriter::on_close) with started still set, flipping stdin to Writable::Ignore so the other release site (take_pending_start_writer, called from on_process_exit / close_io) matches nothing and start()'s +1 is stranded. - on_write: drop the cfg(not(windows)) gates so release_start_ref runs on every platform. - on_close (Windows only): claim started for paths that reach close() without going through on_write (the on_write_complete error arm). write()'s +1, held by that callback's scopeguard, keeps self live past the deref. Gated to Windows because POSIX drain_buffered_data may call on_error() -> close() -> on_close and then on_write() on the same object with no extra ref held. - take_pending_start_writer: clear started when it claims the pointer so a uv_write whose I/O already completed before uv_close (delivered with its real status, not ECANCELED) does not double-release. - WindowsBufferedWriter::on_write_complete: saturating_sub for has_pending_data so the late-success-after-close ordering does not underflow in debug builds.
robobun
added a commit
that referenced
this pull request
Jul 23, 2026
…l write path On Windows, Bun.spawn/spawnSync with an ArrayBuffer/Blob stdin leaked one StaticPipeWriter per spawn. StaticPipeWriter::start() takes a +1 and sets started=true. #30875 added a release_start_ref block to on_write() that clears started and derefs once the buffer drains, but gated it behind cfg(not(windows)). On Windows both terminal arms of WindowsBufferedWriter::on_write_complete reach on_close_io (via close() -> on_close_source -> StaticPipeWriter::on_close) with started still set, flipping stdin to Writable::Ignore so the other release site (take_pending_start_writer, called from on_process_exit / close_io) matches nothing and start()'s +1 is stranded. - on_write: drop the cfg(not(windows)) gates so release_start_ref runs on every platform. - on_close (Windows only): claim started for paths that reach close() without going through on_write (the on_write_complete error arm). write()'s +1, held by that callback's scopeguard, keeps self live past the deref. Gated to Windows because POSIX drain_buffered_data may call on_error() -> close() -> on_close and then on_write() on the same object with no extra ref held. - take_pending_start_writer: clear started when it claims the pointer so a uv_write whose I/O already completed before uv_close (delivered with its real status, not ECANCELED) does not double-release. - WindowsBufferedWriter::on_write_complete: saturating_sub for has_pending_data so the late-success-after-close ordering does not underflow in debug builds.
robobun
added a commit
that referenced
this pull request
Jul 23, 2026
…l write path On Windows, Bun.spawn/spawnSync with an ArrayBuffer/Blob stdin leaked one StaticPipeWriter per spawn. StaticPipeWriter::start() takes a +1 and sets started=true. #30875 added a release_start_ref block to on_write() that clears started and derefs once the buffer drains, but gated it behind cfg(not(windows)). On Windows both terminal arms of WindowsBufferedWriter::on_write_complete reach on_close_io (via close() -> on_close_source -> StaticPipeWriter::on_close) with started still set, flipping stdin to Writable::Ignore so the other release site (take_pending_start_writer, called from on_process_exit / close_io) matches nothing and start()'s +1 is stranded. - on_write: drop the cfg(not(windows)) gates so release_start_ref runs on every platform. - on_close (Windows only): claim started for paths that reach close() without going through on_write (the on_write_complete error arm). write()'s +1, held by that callback's scopeguard, keeps self live past the deref. Gated to Windows because POSIX drain_buffered_data may call on_error() -> close() -> on_close and then on_write() on the same object with no extra ref held. - take_pending_start_writer: clear started when it claims the pointer so a uv_write whose I/O already completed before uv_close (delivered with its real status, not ECANCELED) does not double-release. - WindowsBufferedWriter::on_write_complete: saturating_sub for has_pending_data so the late-success-after-close ordering does not underflow in debug builds. The test observes the native StaticPipeWriter live count directly via a new bun:internal-for-testing hook; RSS is a poor proxy on Windows release builds because mimalloc does not eagerly return pages to the OS.
robobun
added a commit
that referenced
this pull request
Jul 23, 2026
…l write path On Windows, Bun.spawn/spawnSync with an ArrayBuffer/Blob stdin leaked one StaticPipeWriter per spawn. StaticPipeWriter::start() takes a +1 and sets started=true. #30875 added a release_start_ref block to on_write() that clears started and derefs once the buffer drains, but gated it behind cfg(not(windows)). On Windows both terminal arms of WindowsBufferedWriter::on_write_complete reach on_close_io (via close() -> on_close_source -> StaticPipeWriter::on_close) with started still set, flipping stdin to Writable::Ignore so the other release site (take_pending_start_writer, called from on_process_exit / close_io) matches nothing and start()'s +1 is stranded. - on_write: drop the cfg(not(windows)) gates so release_start_ref runs on every platform. - on_close (Windows only): claim started for paths that reach close() without going through on_write (the on_write_complete error arm). write()'s +1, held by that callback's scopeguard, keeps self live past the deref. Gated to Windows because POSIX drain_buffered_data may call on_error() -> close() -> on_close and then on_write() on the same object with no extra ref held. - take_pending_start_writer: clear started when it claims the pointer so a uv_write whose I/O already completed before uv_close (delivered with its real status, not ECANCELED) does not double-release. - WindowsBufferedWriter::on_write_complete: saturating_sub for has_pending_data so the late-success-after-close ordering does not underflow in debug builds.
Jarred-Sumner
pushed a commit
that referenced
this pull request
Jul 24, 2026
…n write completes (#35297) On Windows, `Bun.spawn`/`Bun.spawnSync` with a non-empty `ArrayBuffer`/`Blob` `stdin` leaked one `StaticPipeWriter` allocation per spawn. ## Cause `StaticPipeWriter::start()` takes a `+1` and sets `started = true`. #30875 added a `release_start_ref` block to `on_write()` that clears `started` and derefs once the buffer drains, but gated it behind `#[cfg(not(windows))]`: https://github.com/oven-sh/bun/blob/892b1dabc6/src/spawn/static_pipe_writer.rs#L228-L241 On Windows, when the `uv_write` completes before the child exits (the common ordering for a small buffer): ``` on_write_complete(status=0) -> Parent::on_write(Pending) (release_start_ref gated out) -> writer.close() -> BaseWindowsPipeWriter::close -> on_close_source -> StaticPipeWriter::on_close -> Subprocess::on_close_io(Stdin) stdin <- Writable::Ignore; buffer.deref() (create()'s +1) scopeguard deref (write()'s +1) ``` Net: `started` is still `true` but `stdin` is `Writable::Ignore`, so the other release site, `take_pending_start_writer` (called from `on_process_exit` / `close_io`), matches nothing and start()'s `+1` is stranded. The error arm of `on_write_complete` (e.g. `ERROR_BROKEN_PIPE` when the child closes stdin without reading) reaches `on_close` via `close()` without ever calling `Parent::on_write`, so start()'s `+1` is stranded there the same way. `ShellSubprocess` has the same leak for `cmd < ${buf}` stdin since its `on_process_exit`/`close_io` never look at `started` at all and depend entirely on `on_write`'s release. ## Fix - `StaticPipeWriter::on_write`: drop the `cfg(not(windows))` gates so `release_start_ref` runs on every platform. `WindowsBufferedWriter` never passes `EndOfFile`, so the existing `status != EndOfFile` guard is a no-op there and is left for the POSIX path (`PosixBufferedWriter::_on_write` still touches `self` after the callback on `EndOfFile`). - `StaticPipeWriter::on_close` (Windows only): claim `started` and deref after `on_close_io` for paths that reach `close()` without going through `on_write` (the `on_write_complete` error arm). `write()`'s +1, held by that callback's scopeguard, keeps `self` live past the release. This is gated to Windows because on POSIX `drain_buffered_data` may call `on_error()` -> `close()` -> `on_close` and then `on_write()` on the same object with no extra ref held; #34697 changes that ordering and the gate can go once it lands. - `Subprocess::take_pending_start_writer`: clear `started` when it claims the pointer. The three release sites are then mutually exclusive via `started`, covering the ordering where `on_process_exit` closes the pipe while a `uv_write` whose I/O already completed is still queued (libuv delivers that callback with its real status after `uv_close`, not `ECANCELED`). - `WindowsBufferedWriter::on_write_complete`: `saturating_sub` for `has_pending_data` so the late-success-after-close ordering does not underflow in debug builds. `SecurityScanSubprocess` already derefs and writes `started = false` immediately after `start()`, so both new release sites see `started = false`; no change in its refcount balance. ## Verification Verified on Windows x64 by instrumenting `Drop` with a live counter and spawning 400 children per mode: unfixed leaves 400 live writers after each of the drain (64 B into `sort`) and reject (256 KB into `cmd /c exit`) workloads; fixed returns to 0 on both, in release and debug builds. Also verified on Windows x64: `spawn.test.ts -t 'stdin|Uint8Array|Blob'` (45/45), `spawnSync.test.ts` (6/6), `file-io.test.ts` (26/26), `child_process.test.ts` (41/41), and on Linux: `bunshell.test.ts` (414/414), `bun-security-scanner-workspaces.test.ts` (3/3). `rust:check-all` is green across all 10 targets. Found while working on #35286, which scopes its change to the empty-buffer path and does not overlap.
This was referenced Jul 25, 2026
Closed
This was referenced Aug 12, 2026
robobun
added a commit
that referenced
this pull request
Aug 15, 2026
Main's #30875 replaced the auto-derived Default impl for InputFile with an explicit one, which I missed in the last rebase — CI caught the missing-field on every build-rust lane. Add the None init. Fixes the build-rust failure in build #56341.
Jarred-Sumner
pushed a commit
that referenced
this pull request
Aug 21, 2026
#39947) ### Problem - A worker whose entry point goes through a package.json `imports` or `exports` map leaks 12 KiB (3 `PathBuffer`s, more on Windows) when its thread exits. On an ASAN build LeakSanitizer reports `Direct leak of 12288 byte(s)` allocated in `module_bufs` (`src/resolver/package_json.rs`), reached from `resolve_entry_point_specifier` on the worker thread. - Cause: `MODULE_BUFS` is a thread local `Cell<*mut ModuleBufs>` with nothing that frees the box. The resolver's other per thread buffers (`BufsSlot` in `resolver.rs`, `LazyPathBuf` in `bun_paths`) got a destructor in #30875. This one did not. ### Fix - Wrap the pointer in `ModuleBufsSlot`, whose `Drop` destroys the box when the thread exits. Same shape as `BufsSlot`. Access is unchanged, so the recursion notes on the thread local still hold, and the static TLS template is still one pointer. - Correct because the destructor runs when the thread's TLS is torn down, after every resolver frame on that thread has returned. The main thread's box lives for the process, as before. - Verified: `test/js/web/workers/worker-entry-point.test.ts` (new file) runs a worker through an `imports` alias in a child with `detect_leaks=1`. It fails on main with the report above and passes with this change (checked both ways with a debug build). `test/js/bun/binary/tls-segment-size.test.ts` still passes. ### Background - The resolver keeps a few large scratch buffers per thread instead of on the stack. They are boxed on first use and only a pointer sits in TLS, so the TLS segment stays small on every platform. - A worker thread resolves its own entry point and preloads, so it is the common short lived thread that touches these buffers. The bundler's pool threads live as long as the pool. - The ASAN CI lanes run test children with `detect_leaks=1`. The test sets that itself (plus the repo's `test/leaksan.supp`) so that a local ASAN build checks it too. A build without ASAN ignores the options and checks the behaviour only. <details><summary>Notes</summary> Found through #39811, whose worker test resolves an `imports` alias and failed on the ASAN lanes because of this leak. #39811 carries this change until this lands and is otherwise independent of it. #35060 (overflow bundle threads, open) includes the same change as one of its hunks, because its threads are short lived too. The case is in its own file, for the worker entry point resolution cases, rather than in `worker.test.ts`: three of that file's stress cases go over their budget on a debug build on a slow machine, which would hide whether this case itself flips. #39811 adds its worker case to the same file. Without `print_suppressions=0` LeakSanitizer prints a "Suppressions used" table to stderr on exit when an unrelated, suppressed allocation exists in the process, so the test passes that along with the suppressions file when the environment does not already set `LSAN_OPTIONS`. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/workers/worker-entry-point.test.ts bun test v1.4.0 (4199361) test/js/web/workers/worker-entry-point.test.ts: 41 | LSAN_OPTIONS: 42 | bunEnv.LSAN_OPTIONS ?? 43 | `print_suppressions=0:suppressions=${path.join(import.meta.dir, "..", "..", "..", "leaksan.supp")}`, 44 | }, 45 | ); 46 | expect(stderr).toBe(""); ^ error: expect(received).toBe(expected) - "" + " + ================================================================= + ==385090==ERROR: LeakSanitizer: detected memory leaks + + Direct leak of 12288 byte(s) in 1 object(s) allocated from: + #0 0x000007dd95c8 in malloc crtstuff.c + #1 0x00000be19934 in std::sys::alloc::unix::alloc /root/.rustup/toolchains/nightly-2026-07-20-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:31:18 + #2 0x00000be184b9 in <std::alloc::System>::alloc_impl /root/.rustup/toolchains/nightly-2026-07-20-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/alloc.rs:149:78 + #3 ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (2e16ac4) test/js/web/workers/worker-entry-point.test.ts: (pass) package.json imports alias as the entry point > the worker runs and its thread exits without leaking [11.86ms] 1 pass 0 fail 3 expect() calls Ran 1 test across 1 file. [218.00ms] __F:0:S:0 ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/workers/worker-entry-point.test.ts bun test v1.4.0 (4199361) test/js/web/workers/worker-entry-point.test.ts: (pass) package.json imports alias as the entry point > the worker runs and its thread exits without leaking [3743.31ms] 1 pass 0 fail 3 expect() calls Ran 1 test across 1 file. [6.05s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 667ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[92m Compiling�[0m bun_outpu ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/resolver/package_json.rs | 23 +++++++++--- test/js/web/workers/worker-entry-point.test.ts | 50 ++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) ``` </details> **gate history** · 1 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/resolver/package_json.rs 2 2 0 test/js/web/workers/worker-entry-point.test.ts 1 2 0 ``` </details> **root cause** · written by the author bot With --target bun or node, the resolver short-circuits node:, bun: and hardcoded builtin specifiers into an external result whose primary path is the bare specifier rather than an absolute file path, and entry-point resolution passed that through, so enqueue_entry_item either tripped the absolute-path assert, reported a misleading "File not found", or for bun:wrap collided with the runtime's pre-registered source and left the build with no entry points. The fix marks entry-point resolutions with their own ImportKind so the resolver no longer applies externalization rules to them, and resolv… <!-- robobun:evidence:end -->
robobun
added a commit
that referenced
this pull request
Aug 23, 2026
Main's #30875 replaced the auto-derived Default impl for InputFile with an explicit one, which I missed in the last rebase — CI caught the missing-field on every build-rust lane. Add the None init. Fixes the build-rust failure in build #56341.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bun's native code is now effectively leak-free: ASAN CI runs the entire test
suite under LeakSanitizer with a curated suppression list, and every leak it
reported has been fixed. Getting here took two steps — first making the leak
detector able to see Bun's allocations at all, then fixing everything it found.
Why LeakSanitizer was blind before
The Zig build had a comptime switch (
bun.use_mimalloc) selecting between themimalloc-backed default allocator and
std.heap.c_allocator(libc malloc).ASAN builds set it to
false, so the default allocator routed through plainmalloc/free— which ASAN's interceptor wraps directly with redzones, a freequarantine, and heap-origin tracking. The Rust port flattened this to an
unconditional
#[global_allocator] static ALLOC: bun_alloc::Mimalloc, so ASANbuilds ran mimalloc with
MI_TRACK_ASAN=1shadow annotations instead of ASAN'sown interceptor allocator.
MI_TRACK_ASANonly annotates mimalloc's redzone / use-after-free shadowstate. It does not get the interceptor's free quarantine, full heap-origin
recording, or LeakSanitizer coverage — so most leaks were simply invisible.
This PR gates the
#[global_allocator]oncfg(bun_asan)(set byscripts/build/rust.tsfor ASAN builds): non-ASAN builds keepbun_alloc::Mimalloc; ASAN builds getstd::alloc::System(libcmalloc/freeon Unix,HeapAllocon Windows). ASAN's interceptor andLeakSanitizer now see every heap allocation.
MimallocArena— the parser/AST bump arena — is unaffected and stays onmimalloc heaps in all configurations;
MI_TRACK_ASAN/MI_UBSANremain in themimalloc build config so those allocations are still annotated.
USE_MIMALLOCconsumers (
bun_alloc::USE_MIMALLOC,bun_core::USE_MIMALLOC) becomecfg!(not(bun_asan))so themi_collect/mi_option_set/mi_process_infogates skip mimalloc-as-global-allocator hooks when it isn't the global
allocator, matching the original Zig gating. The
src/jsc/bindgen.rsfallbackthat relied on
Vec::dropreachingmi_freevia the global allocator nowcalls
mi_freedirectly, since the C++ side (ExternVectorTraits.h) alwaysallocates that buffer with
mi_malloc.What got fixed
With LSan finally able to see Bun's heap, it reported leaks across essentially
every native subsystem. They're fixed here — a non-exhaustive map (full detail
in the commit history):
generate_from_cliexit path,chunk data, CSS rule slabs, sourcemap / line-offset tables, AST string
clones, parser arena-stranded heap fields,
OutputFilesource paths/ stylesheet nodes that were stranding allocations on arena reset; free TS
namespace maps on parser teardown
Taskrequest/data union arms on re-pool,package-manifest cache (move instead of clone + leak), workspace
package.json cache, semver query chains, isolated-install Store columns
projections,
StaticRouterefs held by route bundlesFetchTasklet,FileReader,ReadableStreamsources, subprocess / socket refcount imbalances, H2 frame parser callback
refs, DNS, S3 request headers,
StaticPipeWriterdraining, finalizers on
process.exit()Legitimate non-leaks
Some allocations live for the whole process and are reclaimed by the OS at
exit, or are owned by an arena, the JS GC, or thread-local storage — not leaks.
Those are scoped out via
test/leaksan.suppand the per-testno-validate-leaksan.txtlist, each entry annotated with why it isn't a leak.A thread-local
PathBufferscratch pool that genuinely was stranded per workerthread is now freed on thread exit, and its suppression dropped.
Result
ASAN CI runs the full test suite with
detect_leaks=1and the curatedsuppression list, and it passes. Every leak LeakSanitizer reports across the
suite is either fixed or a documented, intentional process-lifetime
allocation. Bun's native code is LeakSanitizer-clean, and it now stays that way
— any new leak fails CI.
Verification
cargo check --workspaceclean;RUSTFLAGS="--cfg bun_asan" cargo checkforbun_alloc/bun_core/bun_jsc/bun_binclean;bun run rust:check-all10/10 targets pass; debug build links and runs.