Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 13 additions & 0 deletions src/bun_core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2813,6 +2813,7 @@ pub mod asan {
safe fn __asan_describe_address(ptr: *const c_void);
safe fn __lsan_register_root_region(ptr: *const c_void, size: usize);
safe fn __lsan_unregister_root_region(ptr: *const c_void, size: usize);
safe fn __lsan_ignore_object(ptr: *const c_void);
}

#[inline]
Expand Down Expand Up @@ -2858,6 +2859,18 @@ pub mod asan {
#[cfg(not(bun_asan))]
let _ = (ptr, size);
}
/// Tell LSAN this heap allocation is intentionally process-lifetime and
/// must not be reported as a leak. Use only for per-VM singletons whose
/// sole anchor lives inside the JSC heap (bmalloc/libpas pages are not
/// scanned as roots, so LSAN cannot see the pointer). Freeing the object
/// later is fine; the ignore is per-allocation, not per-address.
#[inline]
pub fn ignore_object<T>(ptr: *const T) {
#[cfg(bun_asan)]
__lsan_ignore_object(ptr.cast());
#[cfg(not(bun_asan))]
let _ = ptr;
}
}

// ────────────────────────────────────────────────────────────────────────────
Expand Down
11 changes: 11 additions & 0 deletions src/runtime/node/node_fs_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,17 @@
// trivially un-aliased (sole owner of the fresh `Box`).
module.node_fs.with_mut(|nfs| nfs.vm = NonNull::new(vm));

// Per-VM singleton: created once for `internal/fs/binding.ts`, freed by
// the GC wrapper's finalizer at `~VM` in workers, and kept for process
// lifetime on the main thread (which does not destroy its JSC VM). After
// `to_js_boxed` the only pointer to this box lives in the wrapper's
// `m_ctx`, inside the JSC heap; LSan does not scan bmalloc/libpas pages
// as roots, so without this it reports the main-thread singleton as a
// leak once the allocation stack is deep enough that the
// `Bun::generateModule` suppression in `test/leaksan.supp` falls outside
// `malloc_context_size`.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
bun_core::asan::ignore_object(&*module as *const Binding);

Check failure on line 428 in src/runtime/node/node_fs_binding.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

borrow as raw pointer

// `module` was `Box::new`-allocated; ownership transfers to the GC
// wrapper, which calls `Binding::finalize` to reclaim it.
Binding::to_js_boxed(module, global)
Expand Down
26 changes: 26 additions & 0 deletions test/js/web/workers/worker-terminate-lifetime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,32 @@
);

// Regression: the per-VM c-ares channel was destroyed in deinit_runtime_state
// The per-VM `node:fs` native binding (a Box<Binding> created once by
// internal/fs/binding.ts) is anchored only by the GC wrapper's m_ctx slot,
// which lives in the JSC heap (bmalloc). LSan does not scan bmalloc pages as
// roots, so without __lsan_ignore_object it reports the main-thread singleton
// as a 4104-byte leak once Rust's allocator internals push Bun::generateModule
// past malloc_context_size. Covered by the dns.lookup test below too, but this
// is the minimal repro and runs in a fraction of the time.

Check warning on line 131 in test/js/web/workers/worker-terminate-lifetime.test.ts

View check run for this annotation

Claude / Claude Code Review

New test inserted mid-comment, splitting the c-ares regression comment block

The new test was inserted one line *into* the pre-existing c-ares regression comment, splitting it: line 124 (`// Regression: the per-VM c-ares channel was destroyed in deinit_runtime_state`) now sits above the unrelated node:fs test, and the dns.lookup test's comment at line 151 begins mid-sentence with `// (RuntimeState drop) AFTER JSC teardown...`. Move the new test + its comment to just above line 124 so the c-ares comment block stays contiguous with its own test.
Comment thread
robobun marked this conversation as resolved.
Outdated
test.skipIf(!isASAN)(
"main-thread node:fs binding is not a false-positive LSan leak",
async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `require("node:fs");`],
env: {
...bunEnv,
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dirname, "../../../leaksan.supp")}`,
},
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toEqual({ stdout: "", stderr: "", exitCode: 0 });
},
timeout,
);

// (RuntimeState drop) AFTER JSC teardown and RareData.file_polls drop.
// ares_destroy() synchronously fires EDESTRUCTION query callbacks and socket-
// state callbacks, which then dereferenced the freed JSGlobalObject and the
Expand Down
Loading