Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
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>(r: &T) {
#[cfg(bun_asan)]
__lsan_ignore_object(core::ptr::from_ref(r).cast());
#[cfg(not(bun_asan))]
let _ = r;
}
}

// ────────────────────────────────────────────────────────────────────────────
Expand Down
5 changes: 5 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,11 @@ pub(crate) fn create_binding(global: &JSGlobalObject) -> JSValue {
// trivially un-aliased (sole owner of the fresh `Box`).
module.node_fs.with_mut(|nfs| nfs.vm = NonNull::new(vm));

// Per-VM singleton: freed at `~VM` in workers, process-lifetime on the
// main thread. After `to_js_boxed` the only pointer lives in the GC
// wrapper's `m_ctx` inside the JSC heap, which LSan does not scan.
bun_core::asan::ignore_object::<Binding>(&module);

// `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
22 changes: 22 additions & 0 deletions test/js/node/fs/fs-oom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,25 @@
expect(exitCode).toBe(0);
});
});

// The per-VM node:fs Binding box is anchored only by the GC wrapper's m_ctx
// inside the JSC heap, which LSan does not scan; without __lsan_ignore_object
// the main-thread singleton is reported as a leak.
test.skipIf(!isASAN)(
"require('node:fs') is LSan-clean on the main thread",
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 });
},
60_000,

Check warning on line 200 in test/js/node/fs/fs-oom.test.ts

View check run for this annotation

Claude / Claude Code Review

Per-test 60s timeout violates test/CLAUDE.md

The `60_000` per-test timeout is unnecessary here — the subprocess only runs `require("node:fs")` and exits, which is a few seconds under debug+ASAN+LSan, not an outlier. test/CLAUDE.md:120 says *"Do not set a timeout on tests. Bun already has timeouts"*, and the two heavier ASAN subprocess tests just above (32 MiB allocations, 7 decode paths) don't set one either. Drop the third argument.
Comment thread
robobun marked this conversation as resolved.
Outdated
);
Loading