From 3e958586b68ee4903d132d8ca5f4c5ea50f0cceb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 06:14:20 +0000 Subject: [PATCH] lsan: stop suppressing the frames user code runs under; fix the leaks that hid behind them An LSan suppression matches any frame of the allocation stack, so leak:Bun::evaluateCommonJSModuleOnce and leak:JSC::JSModuleLoader::evaluateNonVirtual in test/leaksan.supp hid every leak made by code running at a module's top level (-e scripts, entries, require()d and imported modules, worker eval scripts, test files), which is where most fixtures start their work. JSC__JSModuleLoader__loadAndEvaluateModule names the same kind of frame. Remove all three. The microtask-tick hook installed by Zig__GlobalObject__create was a lambda, so its symbol carried that name and leak:Zig__GlobalObject__create hid everything that ran from a nextTick or promise continuation on the main thread too; install the identical named function that already existed instead. Running every LSan-validated test file with the trimmed file surfaced what those entries were hiding: - process.on("memoryPressure") left registered at exit leaked the watcher box: its RareData slot is an erased pointer and nothing disarmed it at teardown. Disarm it in the teardown stop phase. - crypto.Certificate.exportChallenge() leaked the buffer returned by ASN1_STRING_to_UTF8 on every call. ncrypto::ExportChallenge returns an owning DataPointer, as upstream ncrypto does. - process.dlopen()'s file name for a napi env that an undeleted napi_ref keeps alive forever was never freed, since ~NapiEnv never runs. Free it at the end of the env's cleanup, like its hook set. - A Bun.connect() whose resolve result was already queued when its VM went away leaked the us_connecting_socket_t: closing it could not cancel the queued completion, and the tick that would have run it never came. The teardown drain now runs the dns-ready queue first. - test/regression/issue/26249.test.ts's fixtures never close()d their cc() library, which stays loaded by design; close them. - broadcast-channel-worker-gc.test.ts (EventNames per worker, #38164, and ShadowRealm console objects) and 30205.test.ts (--parallel workers skip napi env cleanup) report leaks owned elsewhere; listed in no-validate-leaksan.txt with the reason. bun:internal-for-testing gains lsanIntentionalLeak() (ASAN builds only) so test/internal/leaksan-suppressions.test.ts can check that a leak made from each of those contexts is reported with test/leaksan.supp applied. --- src/js/internal-for-testing.ts | 11 ++ src/jsc/VirtualMachine.rs | 5 +- src/jsc/bindings/InternalForTesting.cpp | 17 +++ src/jsc/bindings/InternalForTesting.h | 1 + src/jsc/bindings/ZigGlobalObject.cpp | 13 +-- src/jsc/bindings/napi.h | 7 ++ src/jsc/bindings/ncrypto.cpp | 9 +- src/jsc/bindings/ncrypto.h | 3 +- .../node/crypto/node_crypto_binding.cpp | 8 +- src/runtime/jsc_hooks.rs | 4 +- src/runtime/node/memory_pressure.rs | 42 +++++-- src/uws_sys/Loop.rs | 19 +++- test/internal/leaksan-suppressions.test.ts | 107 ++++++++++++++++++ test/js/node/crypto/node-crypto.test.js | 36 +++++- .../process/process-memory-pressure.test.ts | 36 +++++- .../worker-shutdown-post-leak.test.ts | 44 +++++++ test/leaksan.supp | 9 +- test/napi/napi-env-cleanup-leak.test.ts | 56 +++++++++ test/no-validate-leaksan.txt | 13 +++ test/regression/issue/26249.test.ts | 16 +-- 20 files changed, 409 insertions(+), 47 deletions(-) create mode 100644 test/internal/leaksan-suppressions.test.ts create mode 100644 test/napi/napi-env-cleanup-leak.test.ts diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 9b1fed3e91d9..5a5b712037e1 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -667,6 +667,17 @@ export const lsanDoLeakCheck = $newCppFunction("InternalForTesting.cpp", "jsFunc export const isASANEnabled: () => boolean = $newCppFunction("InternalForTesting.cpp", "jsFunction_isASANEnabled", 0); +/** + * Leaks one malloc'd block (ASAN builds only; a no-op elsewhere). LSan attributes it + * to `jsFunction_lsanIntentionalLeak`. Used to check that test/leaksan.supp does not + * hide leaks made by the JS context calling it. + */ +export const lsanIntentionalLeak: () => void = $newCppFunction( + "InternalForTesting.cpp", + "jsFunction_lsanIntentionalLeak", + 0, +); + export const BunString_toThreadSafeRefCountDelta: () => number = $newCppFunction( "InternalForTesting.cpp", "jsFunction_BunString_toThreadSafeRefCountDelta", diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index ac42b0903872..7e20a339589d 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2160,8 +2160,9 @@ pub struct RuntimeHooks { /// right after `close_all_socket_groups`. pub stop_dns_for_vm_teardown: fn() -> SweepResult, /// Stop every registered native handle behind a JS object (servers, - /// listeners, fs watchers) — the stop phase for Rust-side JS classes, run - /// with the VM alive right after the WebCore stop phase. + /// listeners, fs watchers) plus the per-VM `process.on("memoryPressure")` + /// watcher — the stop phase for Rust-side JS classes, run with the VM + /// alive right after the WebCore stop phase. /// /// # Safety /// `vm` is the live per-thread VM on the JS thread; the JSC heap is alive. diff --git a/src/jsc/bindings/InternalForTesting.cpp b/src/jsc/bindings/InternalForTesting.cpp index 69121a0e5b28..aea26f8d51e0 100644 --- a/src/jsc/bindings/InternalForTesting.cpp +++ b/src/jsc/bindings/InternalForTesting.cpp @@ -72,6 +72,23 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_isASANEnabled, (JSC::JSGlobalObject * global #endif } +// Leaks one malloc'd block on purpose so a test can check that LSan, with +// test/leaksan.supp applied, still reports a leak made from the calling JS +// context (test/internal/leaksan-suppressions.test.ts). The report attributes +// the block to this frame. The pointer must not end up anywhere LSan treats +// as a root, so it only ever lives in this local, which is cleared before +// returning; `volatile` keeps the unused allocation from being optimized out. +JSC_DEFINE_HOST_FUNCTION(jsFunction_lsanIntentionalLeak, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ +#if ASAN_ENABLED + char* volatile block = static_cast(malloc(64)); + if (block) + block[0] = 1; + block = nullptr; +#endif + return encodedJSUndefined(); +} + // Returns the net refcount change on the *original* StringImpl after a // BunString owning one ref to it is passed through BunString__toThreadSafe // and then released. A correct implementation must return 0; a positive diff --git a/src/jsc/bindings/InternalForTesting.h b/src/jsc/bindings/InternalForTesting.h index e16a2a98b6c1..19d2d21dc8a3 100644 --- a/src/jsc/bindings/InternalForTesting.h +++ b/src/jsc/bindings/InternalForTesting.h @@ -9,6 +9,7 @@ JSC_DECLARE_HOST_FUNCTION(jsFunction_arrayBufferViewHasBuffer); JSC_DECLARE_HOST_FUNCTION(jsFunction_hasReifiedStatic); JSC_DECLARE_HOST_FUNCTION(jsFunction_lsanDoLeakCheck); JSC_DECLARE_HOST_FUNCTION(jsFunction_isASANEnabled); +JSC_DECLARE_HOST_FUNCTION(jsFunction_lsanIntentionalLeak); JSC_DECLARE_HOST_FUNCTION(jsFunction_BunString_toThreadSafeRefCountDelta); JSC_DECLARE_HOST_FUNCTION(jsFunction_lowercaseHeaderNameSIMD); JSC_DECLARE_HOST_FUNCTION(jsFunction_emitMemoryPressure); diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 3022c09cafe3..ef3e36765873 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -565,15 +565,10 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__create(void* console_client, vm.setOnComputeErrorInfo(computeErrorInfoWrapperToString); vm.setOnComputeErrorInfoJSValue(computeErrorInfoWrapperToJSValue); vm.setComputeLineColumnWithSourcemap(computeLineColumnWithSourcemap); - vm.setOnEachMicrotaskTick([](JSC::VM& vm) -> void { - // if you process.nextTick on a microtask we need this - auto* globalObject = defaultGlobalObject(); - if (auto queue = globalObject->m_nextTickQueue.get()) { - globalObject->resetOnEachMicrotaskTick(); - queue->drain(vm, globalObject); - return; - } - }); + // A named function, not a lambda: microtasks (module evaluation, promise + // jobs) run under this hook's frame, and a lambda's symbol would contain + // this function's name, which test/leaksan.supp suppresses. + vm.setOnEachMicrotaskTick(&checkIfNextTickWasCalledDuringMicrotask); if (executionContextId > -1) { const auto initializeWorker = [&](WebCore::WorkerMessagingProxy& worker) -> void { diff --git a/src/jsc/bindings/napi.h b/src/jsc/bindings/napi.h index 05b95066da16..f61dcc5acb99 100644 --- a/src/jsc/bindings/napi.h +++ b/src/jsc/bindings/napi.h @@ -258,6 +258,13 @@ struct NapiEnv : public WTF::RefCounted { instanceDataFinalizer.call(this, instanceData, true); instanceDataFinalizer.clear(); clearExceptionsBetweenFinalizers(); + + // Same reason as m_cleanupHooks above: a napi_ref the addon never + // deleted keeps the last Ref to this env, so ~NapiEnv may never run. + // Nothing uses the env after cleanup (node_api_get_module_file_name + // answers "" for a null filename). + delete[] std::exchange(filename, nullptr); + m_napiModule.nm_filename = nullptr; } // Threadsafe-function registry. Entries are raw ThreadSafeFunction* owned diff --git a/src/jsc/bindings/ncrypto.cpp b/src/jsc/bindings/ncrypto.cpp index 64e867158ad7..3423c354347b 100644 --- a/src/jsc/bindings/ncrypto.cpp +++ b/src/jsc/bindings/ncrypto.cpp @@ -718,7 +718,7 @@ BIOPointer ExportPublicKey(const char* input, size_t length) return bio; } -Buffer ExportChallenge(const char* input, size_t length) +DataPointer ExportChallenge(const char* input, size_t length) { #ifdef OPENSSL_IS_BORINGSSL // OpenSSL uses EVP_DecodeBlock, which explicitly removes trailing characters, @@ -731,13 +731,12 @@ Buffer ExportChallenge(const char* input, size_t length) NetscapeSPKIPointer sp(NETSCAPE_SPKI_b64_decode(input, length)); if (!sp) return {}; + // ASN1_STRING_to_UTF8 hands back an OPENSSL_malloc'd buffer, which is what + // DataPointer frees. unsigned char* buf = nullptr; int buf_size = ASN1_STRING_to_UTF8(&buf, sp->spkac->challenge); if (buf_size >= 0) { - return { - .data = reinterpret_cast(buf), - .len = static_cast(buf_size), - }; + return DataPointer(buf, static_cast(buf_size)); } return {}; diff --git a/src/jsc/bindings/ncrypto.h b/src/jsc/bindings/ncrypto.h index 3b9224fa24a5..7299d1ed7f50 100644 --- a/src/jsc/bindings/ncrypto.h +++ b/src/jsc/bindings/ncrypto.h @@ -1408,8 +1408,7 @@ bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext); bool VerifySpkac(const char* input, size_t length); BIOPointer ExportPublicKey(const char* input, size_t length); -// The caller takes ownership of the returned Buffer -Buffer ExportChallenge(const char* input, size_t length); +DataPointer ExportChallenge(const char* input, size_t length); // ============================================================================ // KDF diff --git a/src/jsc/bindings/node/crypto/node_crypto_binding.cpp b/src/jsc/bindings/node/crypto/node_crypto_binding.cpp index e8c2d019dad4..4efe262b5c16 100644 --- a/src/jsc/bindings/node/crypto/node_crypto_binding.cpp +++ b/src/jsc/bindings/node/crypto/node_crypto_binding.cpp @@ -187,17 +187,17 @@ JSC_DEFINE_HOST_FUNCTION(jsCertExportChallenge, (JSC::JSGlobalObject * lexicalGl return Bun::ERR::OUT_OF_RANGE(scope, lexicalGlobalObject, "spkac"_s, 0, std::numeric_limits().max(), jsNumber(buffer.size())); } - auto cert = ncrypto::ExportChallenge(reinterpret_cast(buffer.data()), buffer.size()); - if (!cert.data || cert.len == 0) { + auto challenge = ncrypto::ExportChallenge(reinterpret_cast(buffer.data()), buffer.size()); + if (!challenge || challenge.size() == 0) { return JSValue::encode(jsEmptyString(vm)); } - auto result = JSC::ArrayBuffer::tryCreate({ reinterpret_cast(cert.data), cert.len }); + auto result = JSC::ArrayBuffer::tryCreate(challenge.span()); if (!result) { return JSValue::encode(jsEmptyString(vm)); } - auto* bufferResult = JSC::JSUint8Array::create(lexicalGlobalObject, static_cast(lexicalGlobalObject)->JSBufferSubclassStructure(), WTF::move(result), 0, cert.len); + auto* bufferResult = JSC::JSUint8Array::create(lexicalGlobalObject, static_cast(lexicalGlobalObject)->JSBufferSubclassStructure(), WTF::move(result), 0, challenge.size()); RETURN_IF_EXCEPTION(scope, {}); return JSValue::encode(bufferResult); diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index cad7564a5787..4e7b94ccf8fc 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1743,7 +1743,9 @@ pub(crate) fn stop_active_handles_for_test_isolation(vm: &mut VirtualMachine) { } pub(crate) fn stop_active_handles_for_vm_teardown(vm: &mut VirtualMachine) -> SweepResult { - stop_active_handles(vm, StopReason::VmTeardown) + let result = stop_active_handles(vm, StopReason::VmTeardown); + // Not in the registry: one per VM, armed by process.on("memoryPressure"). + result.and(crate::node::memory_pressure::stop_for_vm_teardown(vm)) } #[derive(Clone, Copy, PartialEq, Eq)] diff --git a/src/runtime/node/memory_pressure.rs b/src/runtime/node/memory_pressure.rs index b45c180481d5..15e48dc9ad9e 100644 --- a/src/runtime/node/memory_pressure.rs +++ b/src/runtime/node/memory_pressure.rs @@ -23,12 +23,13 @@ //! //! Armed lazily on the first listener and disarmed on the last removal via //! `onDidChangeListeners` in `BunProcess.cpp`, matching how signal handlers -//! are wired. The watcher does not keep the event loop alive. +//! are wired. The watcher does not keep the event loop alive. A listener that +//! is still registered when its VM goes away is disarmed by +//! [`stop_for_vm_teardown`] instead. use bun_event_loop::ConcurrentTask::{Task, task_tag}; use bun_jsc::JSGlobalObject; -#[cfg(not(windows))] -use bun_jsc::virtual_machine::VirtualMachine; +use bun_jsc::virtual_machine::{SweepResult, VirtualMachine}; #[cfg(not(windows))] use core::ptr::NonNull; @@ -215,8 +216,8 @@ mod posix { *slot(global.bun_vm().as_mut()) = NonNull::new(bun_core::heap::into_raw(watcher).cast()); } - pub(super) fn uninstall(global: &JSGlobalObject) { - let Some(watcher) = take_watcher(global.bun_vm().as_mut()) else { + pub(super) fn uninstall(vm: &mut VirtualMachine) { + let Some(watcher) = take_watcher(vm) else { return; }; if let Some(mut poll) = watcher.poll { @@ -377,8 +378,8 @@ mod windows { *slot(global.bun_vm().as_mut()) = NonNull::new(bun_core::heap::into_raw(watcher).cast()); } - pub(super) fn uninstall(global: &JSGlobalObject) { - let Some(raw) = slot(global.bun_vm().as_mut()).take() else { + pub(super) fn uninstall(vm: &mut VirtualMachine) { + let Some(raw) = slot(vm).take() else { return; }; // SAFETY: slot is populated only by `install` with a `Box`. @@ -406,10 +407,33 @@ pub(crate) extern "C" fn Bun__MemoryPressure__install(global: &JSGlobalObject) { #[unsafe(no_mangle)] pub(crate) extern "C" fn Bun__MemoryPressure__uninstall(global: &JSGlobalObject) { + uninstall(global.bun_vm().as_mut()); +} + +fn uninstall(vm: &mut VirtualMachine) { #[cfg(not(windows))] - posix::uninstall(global); + posix::uninstall(vm); #[cfg(windows)] - windows::uninstall(global); + windows::uninstall(vm); +} + +/// Stop-phase teardown (`stop_active_handles_for_vm_teardown`): a listener +/// still registered when the VM exits never reaches `uninstall` through +/// `onDidChangeListeners`, and the slot in `RareData` is an erased pointer +/// that `RareData`'s drop does not free. Runs while `RareData.file_polls` and +/// the loop are alive, which `uninstall` needs to unregister the poll. +pub(crate) fn stop_for_vm_teardown(vm: &mut VirtualMachine) -> SweepResult { + // Read the raw option: a VM that never armed the watcher has nothing to + // stop and must not lazily allocate a `RareData` here. + let installed = vm + .rare_data + .as_deref_mut() + .is_some_and(|rare| rare.memory_pressure_watcher_slot().is_some()); + if !installed { + return SweepResult::Idle; + } + uninstall(vm); + SweepResult::Stopped } #[unsafe(no_mangle)] diff --git a/src/uws_sys/Loop.rs b/src/uws_sys/Loop.rs index 5120f7d8bb40..328466d85780 100644 --- a/src/uws_sys/Loop.rs +++ b/src/uws_sys/Loop.rs @@ -283,9 +283,19 @@ impl PosixLoop { /// tick; at process/Worker teardown the loop has stopped, so /// `closeAllSocketGroups()` must drain it explicitly or every just-closed /// `us_socket_t` (libc-allocated) shows up as an LSAN leak. + /// + /// Teardown only. A connecting socket closed while its DNS result was + /// already queued on `dns_ready_head` (`us_connecting_socket_close` could + /// not cancel it) is only released by `us_internal_socket_after_resolve`, + /// which the next tick would have run; run that queue first so those land + /// on `closed_connecting_head` too. Every entry is closed by now, so this + /// frees and never starts a connection. pub fn drain_closed_sockets(&mut self) { // SAFETY: self is a valid loop pointer - unsafe { c::us_internal_free_closed_sockets(self) }; + unsafe { + c::us_internal_handle_dns_results(self); + c::us_internal_free_closed_sockets(self); + } } /// `us_socket_group_close_all()` on every group currently linked to this @@ -521,9 +531,13 @@ impl WindowsLoop { self.dec(); } + /// See `PosixLoop::drain_closed_sockets`. pub fn drain_closed_sockets(&mut self) { // SAFETY: self is a valid loop pointer - unsafe { c::us_internal_free_closed_sockets(self) }; + unsafe { + c::us_internal_handle_dns_results(self); + c::us_internal_free_closed_sockets(self); + } } pub fn close_all_groups(&mut self) -> bool { @@ -633,6 +647,7 @@ mod c { now_ns: u64, ); pub(super) fn us_internal_free_closed_sockets(loop_: *mut Loop); + pub(super) fn us_internal_handle_dns_results(loop_: *mut Loop) -> c_int; pub(super) fn us_loop_close_all_groups(loop_: *mut Loop) -> c_int; #[cfg(not(windows))] pub(super) safe fn uws_get_loop() -> *mut Loop; diff --git a/test/internal/leaksan-suppressions.test.ts b/test/internal/leaksan-suppressions.test.ts new file mode 100644 index 000000000000..2e9b71e21096 --- /dev/null +++ b/test/internal/leaksan-suppressions.test.ts @@ -0,0 +1,107 @@ +// CI runs the ASAN build with LeakSanitizer on and test/leaksan.supp applied +// (scripts/runner.node.mjs). An LSan suppression matches when its pattern is a +// substring of ANY frame of an allocation's stack, so an entry naming a frame +// that is on the stack while ordinary code runs hides every leak that code +// makes. The frames that run a module's top level and the VM's microtask-tick +// hook are the important cases: everything a fixture does at its top level +// (-e script, CJS or ESM entry, require()d or imported module, worker eval +// script, the top level of a test file) or from a nextTick/promise +// continuation runs under one of them. +// +// Each row below leaks one malloc'd block (lsanIntentionalLeak, attributed to +// jsFunction_lsanIntentionalLeak) from one of those contexts and expects LSan, +// with the real suppressions file, to report it. The setImmediate row is the +// control: neither kind of frame is on its stack. +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isASAN, isLinux, tempDir } from "harness"; +import { join } from "node:path"; + +const suppressions = join(import.meta.dir, "..", "leaksan.supp"); +const leakCJS = `require("bun:internal-for-testing").lsanIntentionalLeak();`; +const leakESM = `import { lsanIntentionalLeak } from "bun:internal-for-testing";\nlsanIntentionalLeak();`; + +type Row = { + name: string; + files: Record; + cmd: string[]; +}; + +const ROWS: Row[] = [ + { name: "bun -e script", files: {}, cmd: ["-e", leakCJS] }, + { name: "CommonJS entry point", files: { "entry.cjs": leakCJS }, cmd: ["entry.cjs"] }, + { name: "ES module entry point", files: { "entry.mjs": leakESM }, cmd: ["entry.mjs"] }, + { + name: "require()d CommonJS module", + files: { "entry.cjs": `require("./dep.cjs");`, "dep.cjs": leakCJS }, + cmd: ["entry.cjs"], + }, + { + name: "imported ES module", + files: { "entry.mjs": `import "./dep.mjs";`, "dep.mjs": leakESM }, + cmd: ["entry.mjs"], + }, + { + name: "worker_threads eval script", + files: { + "entry.cjs": `new (require("node:worker_threads").Worker)(${JSON.stringify(leakCJS)}, { eval: true });`, + }, + cmd: ["entry.cjs"], + }, + { + name: "top level of a bun test file", + files: { "leak.test.js": `${leakCJS}\nrequire("bun:test").test("registered", () => {});` }, + cmd: ["test", "leak.test.js"], + }, + // Microtasks and nextTick callbacks run under the VM's microtask-tick hook. + { + name: "process.nextTick callback", + files: { "entry.cjs": `process.nextTick(() => { ${leakCJS} });` }, + cmd: ["entry.cjs"], + }, + { + name: "await continuation", + files: { "entry.mjs": `await Promise.resolve();\n${leakESM}` }, + cmd: ["entry.mjs"], + }, + { + name: "setImmediate callback (control)", + files: { "entry.cjs": `setImmediate(() => { ${leakCJS} });` }, + cmd: ["entry.cjs"], + }, +]; + +describe.skipIf(!isASAN || !isLinux)("test/leaksan.supp does not hide leaks made by the code under test", () => { + for (const row of ROWS) { + test.concurrent( + row.name, + async () => { + using dir = tempDir("leaksan-supp", row.files); + await using proc = Bun.spawn({ + cmd: [bunExe(), ...row.cmd], + cwd: String(dir), + env: { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"), + // Same stack depth as CI; a suppression can only match a frame LSan recorded. + // exitcode/abort_on_error pin down how a detected leak ends the process so + // the assertion below does not depend on the ASAN_OPTIONS inherited from CI. + LSAN_OPTIONS: `malloc_context_size=30:print_suppressions=0:abort_on_error=0:exitcode=23:suppressions=${suppressions}`, + }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const reported = + stderr.includes("LeakSanitizer: detected memory leaks") && stderr.includes("jsFunction_lsanIntentionalLeak"); + expect({ reported, exitCode, detail: reported && exitCode === 23 ? "" : stdout + stderr }).toEqual({ + reported: true, + exitCode: 23, + detail: "", + }); + }, + // Symbolizing the report dominates: tens of seconds on a loaded machine with the debug binary. + 90_000, + ); + } +}); diff --git a/test/js/node/crypto/node-crypto.test.js b/test/js/node/crypto/node-crypto.test.js index c48263808fb5..c50a46767808 100644 --- a/test/js/node/crypto/node-crypto.test.js +++ b/test/js/node/crypto/node-crypto.test.js @@ -1,7 +1,8 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isASAN, isLinux } from "harness"; import crypto from "node:crypto"; +import { join } from "node:path"; import { PassThrough, Readable } from "node:stream"; import util from "node:util"; @@ -1222,4 +1223,37 @@ describe("Certificate spkac argument validation", () => { expect(crypto.Certificate.verifySpkac(Buffer.from("not a spkac"))).toBe(false); expect(new crypto.Certificate().verifySpkac("not a spkac", "utf8")).toBe(false); }); + + // exportChallenge copies the decoded challenge into the returned Buffer; the + // OPENSSL_malloc'd buffer it was decoded into used to be dropped. Checked with + // LeakSanitizer the way CI runs the ASAN build. + it.skipIf(!isASAN || !isLinux)( + "Certificate.exportChallenge does not leak the decoded challenge", + async () => { + const spkacPath = join(import.meta.dir, "../test/fixtures/keys/rsa_spkac.spkac"); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Certificate } = require("node:crypto"); + const spkac = require("node:fs").readFileSync(${JSON.stringify(spkacPath)}); + process.stdout.write(Certificate.exportChallenge(spkac).toString()); + `, + ], + env: { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"), + LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dir, "../../../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: "this-is-a-challenge", stderr: "", exitCode: 0 }); + }, + // On a failure LSan symbolizes the report, which takes a while with the debug binary. + 90_000, + ); }); diff --git a/test/js/node/process/process-memory-pressure.test.ts b/test/js/node/process/process-memory-pressure.test.ts index 80d0223d7de1..076d48e3add7 100644 --- a/test/js/node/process/process-memory-pressure.test.ts +++ b/test/js/node/process/process-memory-pressure.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isASAN, isLinux } from "harness"; +import { join } from "node:path"; // process.on("memoryPressure") is a Bun extension. These tests drive the // emit path synthetically via bun:internal-for-testing since real OS memory @@ -108,3 +109,36 @@ describe.concurrent("process.on('memoryPressure')", () => { expect(exitCode).toBe(0); }); }); + +// A listener that is never removed leaves the watcher armed when the process +// exits; VM teardown has to disarm it or the watcher box leaks. Checked with +// LeakSanitizer the way CI runs the ASAN build (the watcher is main-thread +// only, so process exit is the only teardown that can find it armed). +test.skipIf(!isASAN || !isLinux)( + "a listener left registered at exit does not leak the watcher", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { isMemoryPressureWatcherInstalled } = require("bun:internal-for-testing"); + process.on("memoryPressure", () => {}); + process.stdout.write(String(isMemoryPressureWatcherInstalled())); + `, + ], + env: { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"), + LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dir, "../../../leaksan.supp")}`, + }, + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "true", stderr: "", exitCode: 0 }); + }, + // On a failure LSan symbolizes the report, which takes a while with the debug binary. + 90_000, +); diff --git a/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts b/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts index b6b03722c6be..3e7fe673d95c 100644 --- a/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts +++ b/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts @@ -46,4 +46,48 @@ test.skipIf(!isASAN || isWindows)( const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect({ stdout, stderr, exitCode }).toEqual({ stdout: "", stderr: "", exitCode: 0 }); }, + // Spawning a worker VM under debug+ASAN and then running LSan takes several + // seconds on a loaded machine; on a failure symbolizing the report adds more. + 90_000, +); + +// A connect to a hostname with several addresses goes through a +// us_connecting_socket_t whose resolve completion is queued on the loop and +// acted on by the next tick. When the worker exits first, closing the +// connecting socket cannot cancel the already-queued completion and leaves the +// struct to that tick, which never comes, so teardown has to run the queue +// itself. The seeded cache makes the completion queued by the time connect() +// returns, so exiting right away hits this every time. +test.skipIf(!isASAN || isWindows)( + "a worker exiting with a multi-address Bun.connect() in flight does not leak the connecting socket", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("worker_threads"); + const w = new Worker( + 'const { dnsCacheSeed } = require("bun:internal-for-testing");' + + 'dnsCacheSeed("connect-teardown.test", ["127.0.0.1", "::1"]);' + + 'Bun.connect({ hostname: "connect-teardown.test", port: 9, socket: { open() {}, data() {}, close() {}, error() {}, connectError() {} } });' + + 'process.exit(0);', + { eval: true }, + ); + w.on("exit", code => console.log("worker exit " + code)); + `, + ], + env: { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + 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: "worker exit 0\n", stderr: "", exitCode: 0 }); + }, + 90_000, ); diff --git a/test/leaksan.supp b/test/leaksan.supp index 674c921b0629..89543ffbf54b 100644 --- a/test/leaksan.supp +++ b/test/leaksan.supp @@ -1,3 +1,9 @@ +# An entry matches when it is a substring of ANY frame in the allocation stack +# (the innermost malloc_context_size frames, 30 in CI), so an entry naming a +# frame that encloses ordinary code hides every leak that code makes. Never add +# the frames that evaluate a module's top level (the CJS wrapper call, ESM +# evaluation, worker eval scripts): everything a test fixture does at its top +# level runs under them. test/internal/leaksan-suppressions.test.ts checks this. leak:runtime.server.server.ServerAllConnectionsClosedTask.schedule leak:cli.bunfig.Bunfig.parse__anon leak:resolver.resolver.Resolver.parsePackageJSON @@ -28,7 +34,6 @@ leak:Zig::ImportMetaObject::createFromSpecifier leak:Zig::GlobalObject::moduleLoaderResolve leak:JSModuleLoader__import leak:dyld::ThreadLocalVariables -leak:JSC__JSModuleLoader__loadAndEvaluateModule leak:uws_create_app leak:uws_h3_create_app leak:lsquic_global_init @@ -61,7 +66,6 @@ leak:sys_jsc.error_jsc.errorToSystemError leak:runtime.webcore.Blob.getNameString leak:JSC::callIntlDateTimeFormat leak:functionRunProfiler -leak:JSC::JSModuleLoader::evaluateNonVirtual leak:patch.patch.PatchFile.apply leak:jsc.ModuleLoader.RuntimeTranspilerStore.TranspilerJob.runFromJSThread leak:runtime.webcore.blob.Store.initS3WithReferencedCredentials @@ -92,7 +96,6 @@ leak:runtime.node.zlib.NativeZlib.Context.init leak:JSC::intlCollatorAvailableLocales leak:Bun__canonicalizeIP leak:dlopen -leak:Bun::evaluateCommonJSModuleOnce leak:fse_run_loop leak:Zig::NapiClass_ConstructorFunction leak:runtime.webcore.fetch.FetchTasklet.toResponse diff --git a/test/napi/napi-env-cleanup-leak.test.ts b/test/napi/napi-env-cleanup-leak.test.ts new file mode 100644 index 000000000000..6b85df193cf4 --- /dev/null +++ b/test/napi/napi-env-cleanup-leak.test.ts @@ -0,0 +1,56 @@ +import { spawnSync } from "bun"; +import { beforeAll, expect, it } from "bun:test"; +import { existsSync } from "fs"; +import { bunEnv, bunExe, canBuildNodeAddons, isASAN, isWindows } from "harness"; +import { join } from "path"; + +const napiAppDir = join(__dirname, "napi-app"); +const addonName = "napitests"; +const addonPath = join(napiAppDir, `build/Debug/${addonName}.node`); + +// Lives outside napi.test.ts so that it only depends on the one addon it uses. +// Same build strategy as napi-finalizer-delete-ref.test.ts: napi.test.ts (or a +// previous run) has usually built the addon already, and it does not link +// against bun, so an existing binary is still valid; otherwise build just this +// target, falling back to napi-app's full install script. +beforeAll(() => { + if (!canBuildNodeAddons() || !isASAN || isWindows || existsSync(addonPath)) return; + const run = (cmd: string[]) => + spawnSync({ cmd, cwd: napiAppDir, env: bunEnv, stdout: "inherit", stderr: "inherit", stdin: "inherit" }); + if (!existsSync(join(napiAppDir, "node_modules/node-gyp"))) { + run([bunExe(), "install", "--ignore-scripts"]); + } + run([bunExe(), "--bun", "node-gyp", "configure", "build", "--debug", "-j", "max", addonName]); + if (!existsSync(addonPath)) { + run([bunExe(), "install"]); + } + if (!existsSync(addonPath)) { + throw new Error(`building ${addonName} failed`); + } +}, 300_000); + +// create_ref_with_finalizer never deletes the reference napi_wrap handed out, +// as addons commonly do with a constructor reference. That reference keeps the +// env alive past exit, so ~NapiEnv never runs; the env's cleanup has to be what +// releases the module file name process.dlopen() built for it. Checked with +// LeakSanitizer the way CI runs the ASAN build. +it.skipIf(!canBuildNodeAddons() || !isASAN || isWindows)( + "an env kept alive by an undeleted reference does not leak its module file name", + async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `require(${JSON.stringify(addonPath)}).create_ref_with_finalizer(true);`], + env: { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"), + LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(__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 }); + }, + // On a failure LSan symbolizes the report, which takes a while with the debug binary. + 90_000, +); diff --git a/test/no-validate-leaksan.txt b/test/no-validate-leaksan.txt index e4f53b8bfa48..b91d2521894b 100644 --- a/test/no-validate-leaksan.txt +++ b/test/no-validate-leaksan.txt @@ -7,6 +7,19 @@ test/cli/install/bun-security-scanner-matrix-without-node-modules.test.ts # LSAN and no sanitizer report, just slow VM teardown. test/js/node/watch/fs.watch.test.ts +# Its visitChildren stress runs with Malloc=1 (so fastMalloc'd objects are +# visible to LSan) and reports two known leaks: the per-worker EventNames table +# (#38164) and the ConsoleObject of every ShadowRealm global (reported +# separately). Remove once both are fixed. +test/js/web/broadcastchannel/broadcast-channel-worker-gc.test.ts + +# In its `bun test --parallel` case each worker process exits without cleaning +# up the last addon env it loaded, leaking the env's module file name (the +# env's deferred finalizers are the napi_internal_enqueue_finalizer entry in +# leaksan.supp). Reported separately; remove once the worker exit path cleans +# up its envs. +test/regression/issue/30205.test.ts + # error exit root cause unclear test/js/node/test/parallel/test-util-callbackify.js diff --git a/test/regression/issue/26249.test.ts b/test/regression/issue/26249.test.ts index 1235e958c2b5..38a064b72a91 100644 --- a/test/regression/issue/26249.test.ts +++ b/test/regression/issue/26249.test.ts @@ -24,9 +24,9 @@ int get_magic() { import { cc } from "bun:ffi"; import path from "path"; -const { - symbols: { get_magic }, -} = cc({ +// Closed at the end: a library that is never close()d stays loaded on purpose, +// which CI's leak check would otherwise report. +const lib = cc({ source: path.join(import.meta.dir, "test.c"), symbols: { get_magic: { @@ -35,7 +35,8 @@ const { }, }); -console.log(get_magic()); +console.log(lib.symbols.get_magic()); +lib.close(); `, }); @@ -83,9 +84,7 @@ int get_sum() { import { cc } from "bun:ffi"; import path from "path"; -const { - symbols: { get_sum }, -} = cc({ +const lib = cc({ source: path.join(import.meta.dir, "test.c"), symbols: { get_sum: { @@ -94,7 +93,8 @@ const { }, }); -console.log(get_sum()); +console.log(lib.symbols.get_sum()); +lib.close(); `, });