From 23e67495780a63e577cf79da806cf8d9a8c20a4d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 04:23:33 +0000 Subject: [PATCH 1/8] ci: add linux aarch64 ASAN build and test lanes to PR pipelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the existing linux x64 ASAN lane: release-asan build on amazonlinux 2023 (aarch64) plus a 20-shard test step on debian 13. Like x64-asan, the lane is PR-only — filtered out on the main branch. Skip baseline verification for asan profiles: asan link steps upload ${triplet}-asan.zip, not the ${triplet}-profile.zip that step downloads, and asan artifacts never ship in releases. --- .buildkite/ci.mjs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.buildkite/ci.mjs b/.buildkite/ci.mjs index 275420c99834..8d8e680a6cf1 100755 --- a/.buildkite/ci.mjs +++ b/.buildkite/ci.mjs @@ -140,6 +140,9 @@ const buildPlatforms = [ { os: "linux", arch: "aarch64", distro: "amazonlinux", release: "2023", features: ["docker"] }, { os: "linux", arch: "x64", distro: "amazonlinux", release: "2023", features: ["docker"] }, { os: "linux", arch: "x64", baseline: true, distro: "amazonlinux", release: "2023", features: ["docker"] }, + // ASAN lanes are PR-only (filtered out on main — see getPipelineOptions and + // includeASAN in getPipeline) and never ship in releases. + { os: "linux", arch: "aarch64", profile: "asan", distro: "amazonlinux", release: "2023", features: ["docker"] }, { os: "linux", arch: "x64", profile: "asan", distro: "amazonlinux", release: "2023", features: ["docker"] }, { os: "linux", arch: "aarch64", abi: "musl", distro: "alpine", release: "3.23" }, { os: "linux", arch: "x64", abi: "musl", distro: "alpine", release: "3.23" }, @@ -194,6 +197,7 @@ const testPlatforms = [ { os: "linux", arch: "aarch64", distro: "debian", release: "13", tier: "latest" }, { os: "linux", arch: "x64", distro: "debian", release: "13", tier: "latest" }, { os: "linux", arch: "x64", baseline: true, distro: "debian", release: "13", tier: "latest" }, + { os: "linux", arch: "aarch64", profile: "asan", distro: "debian", release: "13", tier: "latest" }, { os: "linux", arch: "x64", profile: "asan", distro: "debian", release: "13", tier: "latest" }, { os: "linux", arch: "aarch64", distro: "ubuntu", release: "25.04", tier: "latest" }, { os: "linux", arch: "x64", distro: "ubuntu", release: "25.04", tier: "latest" }, @@ -688,7 +692,12 @@ function getTargetTriplet(platform) { * @returns {boolean} */ function needsBaselineVerification(platform) { - const { os, arch, baseline } = platform; + const { os, arch, baseline, profile } = platform; + // ASAN builds are PR-only test artifacts that never ship, and they don't + // produce the ${triplet}-profile.zip this step downloads (the asan link + // uploads ${triplet}-asan.zip instead — see scripts/build/ci.ts). The + // release lanes already cover instruction-policy verification. + if (profile === "asan") return false; if (os === "linux") return (arch === "x64" && baseline) || arch === "aarch64"; if (os === "windows") return arch === "x64" && baseline; return false; From daf118df2c89f547842a491f82ddf74e30c28315 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 06:53:22 +0000 Subject: [PATCH 2/8] Bump WEBKIT_VERSION to f18cf9c267c7 Picks up oven-sh/WebKit#247: FreeList::forEach asserted a hardcoded 16 KB interval bound, but MarkedBlock::blockSize is 64 KB on Linux ARM64 (CeilingOnPageSize), so every ASSERT_ENABLED arm64-linux artifact (-asan, -debug, -debug-asan) crashed at the first GC stop. The new linux-aarch64-asan lane needs the fixed -asan prebuilt. Also picks up from oven-sh/WebKit main: xwin 0.9.0, UB fix in double-to-int conversions, JIT disassembler compiled out of release, windows amd64-baseline ThinLTO variant. --- scripts/build/deps/webkit.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/build/deps/webkit.ts b/scripts/build/deps/webkit.ts index c49031a453f7..cac0cb25d228 100644 --- a/scripts/build/deps/webkit.ts +++ b/scripts/build/deps/webkit.ts @@ -7,7 +7,7 @@ // -lto variants built with ThinLTO (per-module summaries for cross-language // importing), and the Windows ICU data table filtered + per-item zstd // compressed (lazily decompressed via bun_icu_decompress.cpp). -export const WEBKIT_VERSION = "963f8758c29e965471c191668d5776a1a1b014b6"; +export const WEBKIT_VERSION = "f18cf9c267c72ff3d1ca31eec864ebcbeefd29b5"; /** * WebKit (JavaScriptCore) — the JS engine. From b86e11672ea0e4ff514678c1d208a4ad4803685f Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 2 Jun 2026 08:17:02 +0000 Subject: [PATCH 3/8] Fix memory leaks found by LeakSanitizer on the linux aarch64 ASAN lane Covers four leak groups across the runtime: - Bytecode cache (transpiler/module loader): the sidecar .jsc buffer read for '// @bun @bytecode' modules was never freed. The CachedBytecode destructor was selected off ResolvedSource.needsDeref, which tracks the source_code string and is cleared (and externally mutated) before the selection ran, so the no-op destructor was always chosen. Add a dedicated bytecode_cache_needs_deref flag (Rust + C++ struct mirror), set it at the two heap::into_raw producer sites, key the destructor off it, clear it once CachedBytecode adopts the buffer, and free the buffer in OwnedResolvedSource::drop when ownership never crossed FFI. Also fixes the async RuntimeTranspilerStore path passing a pointer borrowed from a local parse_result (dangling by the time the JS thread used it) by transferring ownership instead. Embedded bun build --compile bytecode keeps the no-op destructor (flag stays false). - Bun.serve teardown: a server finalized during VM shutdown enqueued its App.close + deinit task pair on an event loop that never ticks again; EventLoop::deinit freed the tasks unrun and the entire server graph (NewServer box, ServerConfig strings, routes, HTMLBundle routes) leaked. Run close + deinit synchronously when the VM is shutting down, preserving close-before-destroy ordering. Guard JSNodeHTTPServerSocket::onData against allocating JS cells during shutdown, mirroring onClose. - Dev server route bundles: DevServer::drop released client_bundle and cached_response but not the intrusive ref RouteBundle holds on its html_bundle route (raw pointer, so Vec drop could not release it), leaking one HTMLBundleRoute per bundled route. Mirror Zig's RouteBundle.deinit and deref it. - Watcher: WatchItem.file_path is an owning Cow column in a MultiArrayList whose Drop is slab-only, so owned paths leaked at watcher teardown and on every eviction (swap_remove is a bitwise overwrite). Drop the rows before freeing the slab and take the evicted path out before swap_remove. - Cron teardown ordering: VirtualMachine::destroy took rare_data before calling cron_clear_all_teardown, which early-returns when rare_data is None - the hook was a no-op, leaking still-registered CronJobs and tripping the cron_jobs.is_empty() debug assert when a job survived finalization (process.exit from inside a cron callback under --hot). Call the hook before taking rare_data. --- src/jsc/ResolvedSource.rs | 21 ++++++++++++ src/jsc/RuntimeTranspilerStore.rs | 32 ++++++++++++++----- src/jsc/VirtualMachine.rs | 11 ++++++- src/jsc/bindings/ZigSourceProvider.cpp | 11 ++++++- src/jsc/bindings/headers-handwritten.h | 4 +++ .../bindings/node/JSNodeHTTPServerSocket.cpp | 5 ++- src/runtime/bake/DevServer.rs | 6 ++++ src/runtime/jsc_hooks.rs | 3 ++ src/runtime/server/mod.rs | 32 +++++++++++++++++++ src/watcher/Watcher.rs | 26 +++++++++++++-- 10 files changed, 138 insertions(+), 13 deletions(-) diff --git a/src/jsc/ResolvedSource.rs b/src/jsc/ResolvedSource.rs index 0d64d35f0f6b..a26c32358f7c 100644 --- a/src/jsc/ResolvedSource.rs +++ b/src/jsc/ResolvedSource.rs @@ -45,6 +45,13 @@ pub struct ResolvedSource { // -- Bytecode cache fields -- pub bytecode_cache: *mut u8, pub bytecode_cache_size: usize, + /// Whether `bytecode_cache` is a heap allocation (`heap::into_raw`'d + /// `Box<[u8]>`) that the C++ `JSC::CachedBytecode` destructor must free. + /// `false` when the bytes point into the standalone executable's embedded + /// section (`bun build --compile`). Deliberately separate from + /// `source_code_needs_deref`, which C++ mutates while consuming the + /// source string. + pub bytecode_cache_needs_deref: bool, pub module_info: *mut c_void, /// The file path used as the source origin for bytecode cache validation. /// JSC validates bytecode by checking if the origin URL matches exactly what @@ -68,6 +75,7 @@ impl Default for ResolvedSource { already_bundled: false, bytecode_cache: core::ptr::null_mut(), bytecode_cache_size: 0, + bytecode_cache_needs_deref: false, module_info: core::ptr::null_mut(), bytecode_origin_path: BunString::empty(), } @@ -139,6 +147,19 @@ impl Drop for OwnedResolvedSource { if self.0.source_code_needs_deref { self.0.source_code.deref(); } + if self.0.bytecode_cache_needs_deref && !self.0.bytecode_cache.is_null() { + // SAFETY: `bytecode_cache_needs_deref` marks the pointer as the + // `heap::into_raw`'d `Box<[u8]>` built on the runtime + // `// @bun @bytecode` path. `Drop` only runs when `into_ffi()` was + // never called, so Rust is still the sole owner of the buffer + // (the C++ `CachedBytecode` destructor frees it otherwise). + unsafe { + drop(bun_core::heap::take(core::ptr::slice_from_raw_parts_mut( + self.0.bytecode_cache, + self.0.bytecode_cache_size, + ))); + } + } self.0.specifier.deref(); self.0.source_url.deref(); self.0.bytecode_origin_path.deref(); diff --git a/src/jsc/RuntimeTranspilerStore.rs b/src/jsc/RuntimeTranspilerStore.rs index 465d44b9bd60..39c266a30861 100644 --- a/src/jsc/RuntimeTranspilerStore.rs +++ b/src/jsc/RuntimeTranspilerStore.rs @@ -1032,17 +1032,33 @@ impl TranspilerJob { } if !matches!(parse_result.already_bundled, AlreadyBundled::None) { - let bytecode_slice = parse_result.already_bundled.bytecode_slice(); + // Move the bytecode out of `parse_result` and transfer ownership + // to C++ (matches `transpile_source_code_inner` in jsc_hooks.rs and + // the Zig default_allocator semantics): `parse_result` is dropped + // when this function returns, so a borrowed `bytecode_slice()` + // pointer would dangle by the time the JS thread builds the + // `JSC::CachedBytecode` over it. `bytecode_cache_needs_deref` + // tells the C++ CachedBytecode destructor to free the buffer. + let already_bundled = core::mem::take(&mut parse_result.already_bundled); + let is_commonjs_module = already_bundled.is_common_js(); + let (bytecode_cache, bytecode_cache_size) = match already_bundled { + AlreadyBundled::Bytecode(bytes) | AlreadyBundled::BytecodeCjs(bytes) => { + let len = bytes.len(); + if len == 0 { + (ptr::null_mut(), 0) + } else { + (bun_core::heap::into_raw(bytes).cast::(), len) + } + } + _ => (ptr::null_mut(), 0), + }; self.resolved_source = OwnedResolvedSource::from(ResolvedSource { source_code: String::clone_latin1(&parse_result.source.contents), already_bundled: true, - bytecode_cache: if !bytecode_slice.is_empty() { - bytecode_slice.as_ptr().cast_mut() - } else { - ptr::null_mut() - }, - bytecode_cache_size: bytecode_slice.len(), - is_commonjs_module: parse_result.already_bundled.is_common_js(), + bytecode_cache, + bytecode_cache_size, + bytecode_cache_needs_deref: !bytecode_cache.is_null(), + is_commonjs_module, tag: this_tag, ..Default::default() }); diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 6dfa9baa7086..0b1ef2873759 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -4495,10 +4495,19 @@ impl VirtualMachine { // each stored map and `deinit()`s the sibling `saved_source_map_table`. drop(core::mem::take(&mut self.source_mappings)); - if let Some(rare) = self.rare_data.take() { + if self.rare_data.is_some() { + // Must run BEFORE `rare_data.take()`: `CronJob::clear_all_for_vm` + // reads `vm.rare_data` to drain the job list and early-returns on + // `None`. Calling it after the take() made it a no-op, leaking + // every still-registered CronJob (and tripping the + // `cron_jobs.is_empty()` assert in `RareData::drop` when a job + // survived finalization, e.g. `process.exit()` from inside a cron + // callback). if let Some(hooks) = runtime_hooks() { (hooks.cron_clear_all_teardown)(self); } + } + if let Some(rare) = self.rare_data.take() { // Paired with `rare_data()`'s register_root_region. Without this, // every terminated Worker leaves a stale LSAN root entry pointing // into a freed arena. diff --git a/src/jsc/bindings/ZigSourceProvider.cpp b/src/jsc/bindings/ZigSourceProvider.cpp index e7b3f6e47ab7..e9793aa08fa2 100644 --- a/src/jsc/bindings/ZigSourceProvider.cpp +++ b/src/jsc/bindings/ZigSourceProvider.cpp @@ -124,11 +124,20 @@ Ref SourceProvider::create( const auto destructorNoOp = [](const void* ptr) { // no-op, for bun build --compile. }; - const auto destructor = resolvedSource.needsDeref ? destructorPtr : destructorNoOp; + // NOTE: do not gate this on `needsDeref` — that flag tracks the + // `source_code` string +1 and is cleared above (and mutated by + // `JSCommonJSModule::evaluate`'s wrapper-override path) before we + // get here, which both leaked runtime `// @bun @bytecode` buffers + // and could free embedded standalone-binary bytecode. + const auto destructor = resolvedSource.bytecode_cache_needs_deref ? destructorPtr : destructorNoOp; auto origin = getSourceOrigin(); Ref bytecode = JSC::CachedBytecode::create(std::span(resolvedSource.bytecode_cache, resolvedSource.bytecode_cache_size), destructor, {}); + // The CachedBytecode now owns the buffer; clear the ownership + // marker (mirroring the `needsDeref` clearing above) so the copy + // stored in m_resolvedSource cannot be misread as still owning it. + resolvedSource.bytecode_cache_needs_deref = false; auto provider = adoptRef(*new SourceProvider( globalObject->bunVM(), resolvedSource, diff --git a/src/jsc/bindings/headers-handwritten.h b/src/jsc/bindings/headers-handwritten.h index 3d2cdd525e8b..548879ff6b6b 100644 --- a/src/jsc/bindings/headers-handwritten.h +++ b/src/jsc/bindings/headers-handwritten.h @@ -130,6 +130,10 @@ typedef struct ResolvedSource { // -- Bytecode cache fields -- uint8_t* bytecode_cache; size_t bytecode_cache_size; + // Whether `bytecode_cache` is heap-owned (Rust `Box<[u8]>` via + // `heap::into_raw`) and must be freed by the CachedBytecode destructor. + // False for `bun build --compile`, where it points into the executable. + bool bytecode_cache_needs_deref; void* module_info; // File path used as source origin for bytecode cache validation. // Converted to file:// URL. If empty, origin is derived from source_url. diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index e498d98fc36a..bdbd9d114e1b 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -247,7 +247,10 @@ void JSNodeHTTPServerSocket::onData(const char* data, int length, bool last) WebCore::ScriptExecutionContext* scriptExecutionContext = globalObject->scriptExecutionContext(); - if (scriptExecutionContext) { + // Mirror onClose: never allocate JS cells or post tasks while the VM is + // shutting down (this can be reached from uws close callbacks during the + // final GC, when allocation is forbidden and posted tasks never run). + if (scriptExecutionContext && !globalObject->isShuttingDown()) { auto scope = DECLARE_TOP_EXCEPTION_SCOPE(globalObject->vm()); JSC::JSUint8Array* buffer = WebCore::createBuffer(globalObject, std::span(reinterpret_cast(data), length)); auto chunk = JSC::JSValue(buffer); diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index 701a448d2ff7..d7000bc87052 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -1208,6 +1208,12 @@ impl Drop for DevServer { // SAFETY: stored ref from `init_from_any_blob`; no live borrow. unsafe { StaticRoute::deref_(cached.as_ptr()) }; } + // Zig `RouteBundle.deinit`: `html.html_bundle.deref()` — release + // the intrusive ref taken by `get_or_put_route_bundle` when the + // bundle was stored. `html_bundle` is a raw `*mut`, so dropping + // the Vec would otherwise leak the route (and its HTMLBundle). + // SAFETY: the slot holds a counted ref taken at store time. + unsafe { bun_ptr::RefCount::::deref(html.html_bundle) }; } } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 49d4e9610c21..d661b03bc946 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -2732,6 +2732,9 @@ fn transpile_source_code_inner( already_bundled: true, bytecode_cache, bytecode_cache_size, + // C++ owns the buffer (heap::into_raw above); the + // CachedBytecode destructor frees it. + bytecode_cache_needs_deref: !bytecode_cache.is_null(), is_commonjs_module, ..Default::default() })); diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index f5ea45032423..0735ff48debf 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1715,6 +1715,38 @@ impl NewServer { // for the server's lifetime); single-threaded JS context, no aliasing `&mut`. let vm = unsafe { &mut *self.vm_mut() }; + if vm.is_shutting_down() { + // The VM is shutting down (`is_shutting_down` is set in `on_exit`, + // before `Zig__GlobalObject__destructOnExit`'s final GC / + // `lastChanceToFinalize()` — typically we get here from a server + // wrapper finalizer during that last collection): the event loop + // will never tick again, so the App.close + deinit task pair below would + // be freed *unrun* by `EventLoop::deinit()` and the entire server + // graph (config, user_routes, HTMLBundle routes, …) would leak. + // Run both steps synchronously instead, preserving the + // close-before-destroy ordering (`~TemplatedApp` requires `close()` + // to have run — destroying without closing leaks the listen-socket + // polls). The re-entrancy hazard the task split guards against + // (App.close firing handlers mid-GC) is moot here: this branch is + // only reachable with no listener, no pending requests, and no + // active websockets, and `Listener` finalizers already + // `closeAll()` synchronously on this same path (their closed + // sockets are drained right after `destructOnExit`). + if !self.flags.contains(ServerFlags::TERMINATED) { + self.flags.insert(ServerFlags::TERMINATED); + if let Some(app) = self.app { + // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. + bun_opaque::opaque_deref_mut(app).close(); + } + } + // SAFETY: `self` is the unique heap server pointer (its JS wrapper + // has been finalized — `schedule_deinit` is only reached with + // `js_value == Finalized`). `deinit` frees it; `self` must not be + // touched after this call, so return immediately. + Self::deinit(std::ptr::from_mut::(self)); + return; + } + if !self.flags.contains(ServerFlags::TERMINATED) { // App.close can cause finalizers to run. // scheduleDeinit can be called inside a finalizer. diff --git a/src/watcher/Watcher.rs b/src/watcher/Watcher.rs index 51796ecd1ab7..661d6f50ccea 100644 --- a/src/watcher/Watcher.rs +++ b/src/watcher/Watcher.rs @@ -267,7 +267,12 @@ impl Watcher { let _ = bun_sys::close(fd); } } - // watchlist freed by Drop on Box + // `MultiArrayList`'s `Drop` is slab-only and never runs column + // destructors; drop the rows first or every `Cow::Owned` + // `file_path` leaks (matches Zig, where the watchlist allocator + // owned the dupes). + me.watchlist.drop_elements(); + // watchlist slab freed by Drop on Box // SAFETY: this was heap-allocated by caller of init() drop(unsafe { bun_core::heap::take(this) }); } @@ -318,7 +323,11 @@ impl Watcher { let _ = bun_sys::close(fd); } } - // watchlist freed by Drop below + // `MultiArrayList`'s `Drop` is slab-only and never runs column + // destructors; drop the rows first or every `Cow::Owned` + // `file_path` leaks. + me.watchlist.drop_elements(); + // watchlist slab freed by Drop below } // Close trace file if open @@ -397,6 +406,12 @@ impl Watcher { if item == last_item || self.watchlist.len() <= item as usize { continue; } + // `swap_remove` is a bitwise row overwrite that never runs column + // destructors — take the `Cow::Owned` file_path out first or it + // leaks (matches Zig, where the watchlist allocator owned the dupe). + drop(core::mem::take( + &mut self.watchlist.items_file_path_mut()[item as usize], + )); self.watchlist.swap_remove(item as usize); // swapRemove put a different entry at `item`, but its kqueue registration still @@ -1093,6 +1108,7 @@ pub enum WatchItemKind { /// the unsafe generic `Slice::items::(field)`. pub trait WatchItemColumns { fn items_file_path(&self) -> &[Cow<'static, [u8]>]; + fn items_file_path_mut(&mut self) -> &mut [Cow<'static, [u8]>]; fn items_hash(&self) -> &[u32]; fn items_fd(&self) -> &[Fd]; fn items_fd_mut(&mut self) -> &mut [Fd]; @@ -1106,6 +1122,9 @@ impl WatchItemColumns for WatchList { fn items_file_path(&self) -> &[Cow<'static, [u8]>] { self.items::<"file_path", Cow<'static, [u8]>>() } + fn items_file_path_mut(&mut self) -> &mut [Cow<'static, [u8]>] { + self.items_mut::<"file_path", Cow<'static, [u8]>>() + } fn items_hash(&self) -> &[u32] { self.items::<"hash", u32>() } @@ -1131,6 +1150,9 @@ impl WatchItemColumns for bun_collections::multi_array_list::Slice { fn items_file_path(&self) -> &[Cow<'static, [u8]>] { self.items::<"file_path", Cow<'static, [u8]>>() } + fn items_file_path_mut(&mut self) -> &mut [Cow<'static, [u8]>] { + self.items_mut::<"file_path", Cow<'static, [u8]>>() + } fn items_hash(&self) -> &[u32] { self.items::<"hash", u32>() } From 7f1803a094eb9a54333e90a839d66f7c453cccde Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 2 Jun 2026 15:02:32 +0000 Subject: [PATCH 4/8] Fix remaining LeakSanitizer leaks; widen windows baseline allowlist - http: send_sync returned a picohttp::Response that borrowed an intentionally leaked HTTPResponseMetadata (status text, header buffer, and the boxed headers slice leaked once per sync CLI request). Return the owning HTTPResponseMetadata instead; a Deref to the inner Response keeps all call sites unchanged and Drop reclaims the buffers. Also stop copying the now-droppable Response back into the caller's AsyncHTTP in the sync handoff so no stale alias survives the metadata's drop. - server: Route::init takes a +1 ref on its HTMLBundle, but Route's Drop never released it (RefPtr has no Drop glue), stranding every bundle attached to a route past VM teardown. Release it explicitly, matching the original deinit order. - test: html-rewriter-leak's RSS workload exceeds its 15s timeout under ASAN instrumentation; raise the timeout to 90s on ASAN builds only, keeping the workload and threshold unchanged. - verify-baseline: the strpbrk flag on windows x64-baseline is a data-in-text artifact (an in-text switch jump-table entry decoding as xabort after a layout shift), not a real RTM instruction; widen the existing ceiling to [AVX, AVX2, RTM] with a comment documenting the evidence. --- .../allowlist-x64-windows.txt | 11 +++++++++- src/http/AsyncHTTP.rs | 20 +++++++++++-------- src/http/lib.rs | 17 ++++++++++++++++ src/runtime/server/HTMLBundle.rs | 5 ++++- src/runtime/server/server_body.rs | 5 +++-- test/js/workerd/html-rewriter-leak.test.ts | 6 ++++-- 6 files changed, 50 insertions(+), 14 deletions(-) diff --git a/scripts/verify-baseline-static/allowlist-x64-windows.txt b/scripts/verify-baseline-static/allowlist-x64-windows.txt index 9078bd9e2aad..33b86b2a3a41 100644 --- a/scripts/verify-baseline-static/allowlist-x64-windows.txt +++ b/scripts/verify-baseline-static/allowlist-x64-windows.txt @@ -846,7 +846,16 @@ sinf [AVX, FMA] sinh_fma [AVX, FMA] sinhf_fma [AVX, FMA] strnlen [AVX, AVX2] -strpbrk [AVX, AVX2] # UCRT vectorized str routine, runtime-gated on __isa_available +# strpbrk: UCRT vectorized str routine, runtime-gated on __isa_available. +# RTM is a disassembly artifact, not real code: MSVC puts the routine's +# switch jump table in .text right after the body's ret. The 4-byte table +# entries (verified by disassembling bun-profile.exe at the flagged VA: +# `c6 f8 1a 03` = target offset 0x031af8c6, one of a +7-stride run matching +# the pslldq/jmp ladder dispatched via `jmp *table(%rsi,%r9,4)`) happen to +# decode as `xabort imm8` (C6 F8 ib). Layout shifts (e.g. a WebKit bump) +# change the offsets stored in the table, so which junk instruction the +# linear sweep sees here varies build to build. +strpbrk [AVX, AVX2, RTM] tan [AVX, FMA] tanf [AVX, FMA] tanh_fma [AVX, FMA] diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index c2e9d6a2b7e2..28e7b1f29d27 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -657,7 +657,12 @@ fn send_sync_callback( if let Some(mut real) = async_http.real { // SAFETY: `real` outlives the HTTP-thread copy by construction. let real = unsafe { real.as_mut() }; - real.response = async_http.response; + // Don't copy `response` back: it's a bitwise alias of + // `metadata.response`, whose buffers are freed when the caller drops + // the `HTTPResponseMetadata` returned by `send_sync`, so a stored copy + // would dangle. (Zig copied it, but Zig also leaked the metadata.) + // No sync caller reads `real.response`; they use the returned value. + real.response = None; real.request = async_http.request.take(); real.response_headers = core::mem::take(&mut async_http.response_headers); real.response_encoding = async_http.response_encoding; @@ -679,7 +684,7 @@ fn send_sync_callback( } impl<'a> AsyncHTTP<'a> { - pub fn send_sync(&mut self) -> Result, bun_core::Error> { + pub fn send_sync(&mut self) -> Result { crate::http_thread::init(&Default::default()); // PORT NOTE: Zig leaked `ctx` (never destroyed). `Box::leak` is forbidden @@ -705,12 +710,11 @@ impl<'a> AsyncHTTP<'a> { return Err(err); } debug_assert!(result.metadata.is_some()); - // The returned `Response` borrows `metadata.owned_buf` (status text + - // header slices). Zig's `sendSync` returns `result.metadata.?.response` - // and never `deinit`s the metadata; mirror that by suppressing Drop so - // the borrowed buffer outlives the call. `send_sync` is one-shot CLI. - let metadata = core::mem::ManuallyDrop::new(result.metadata.unwrap()); - Ok(metadata.response) + // `metadata.response` borrows `metadata.owned_buf` (status text + + // header slices), so hand the whole metadata to the caller — its + // `Deref` exposes the `Response`, and `Drop` reclaims the buffers + // (Zig's `sendSync` returned the bare `Response` and leaked them). + Ok(result.metadata.unwrap()) } // ────────────────────────────────────────────────────────────────────── diff --git a/src/http/lib.rs b/src/http/lib.rs index bce6e59856c2..a6946bdf5535 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -133,6 +133,23 @@ pub struct HTTPResponseMetadata { pub response: bun_picohttp::Response<'static>, } +// `send_sync` returns the whole metadata so the borrowed `Response` cannot +// outlive its backing buffers; callers only read `Response` fields, so a +// `Deref` keeps the call sites unchanged. +// +// CAUTION: the `Response` is typed `'static`, but its status text and header +// slices actually borrow the sibling `owned_buf` (and the heap headers slice) +// that `Drop` frees. The borrow checker cannot catch a slice from +// `res.headers.get(..)` escaping the metadata binding's scope — keep all such +// slices strictly inside it. +impl core::ops::Deref for HTTPResponseMetadata { + type Target = bun_picohttp::Response<'static>; + + fn deref(&self) -> &Self::Target { + &self.response + } +} + impl Default for HTTPResponseMetadata { fn default() -> Self { Self { diff --git a/src/runtime/server/HTMLBundle.rs b/src/runtime/server/HTMLBundle.rs index af1500746fdd..29a91b6e19f9 100644 --- a/src/runtime/server/HTMLBundle.rs +++ b/src/runtime/server/HTMLBundle.rs @@ -788,7 +788,10 @@ impl Drop for Route { fn drop(&mut self) { // pending responses keep a ref to the route debug_assert!(self.pending_responses.get().is_empty()); - // `pending_responses` (Vec) and `bundle` (IntrusiveRc) auto-drop. + // `pending_responses` (Vec) auto-drops. `RefPtr`/`IntrusiveRc` has no + // `Drop`, so release the bundle ref taken in `Route::init` explicitly + // (mirrors Zig `Route.deinit` calling `this.bundle.deref()`). + self.bundle.deref(); // `state` has no `Drop` glue for the intrusive-pointer variants — release // them explicitly (mirrors Zig `Route.deinit` calling `this.state.deinit()`). // `with_mut` is fine here — refcount==0 so no other `&Route` exists. diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index b882c9ecfac2..c8c864f1a31c 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -717,8 +717,9 @@ impl AnyRoute { // *without* deref). `RefPtr` has no `Drop`, so a bit-copy // here keeps the net refcount at 1 — bumping for the map // slot would leak +1 per first-seen HTMLBundle. - // SAFETY: `html_bundle` is the live `RefPtr` from the - // route map; `init` consumes its +1 ref into the new `Route`. + // SAFETY: `html_bundle` is a borrowed pointer to the live + // JS-wrapped `HTMLBundle`; `init` takes its own +1 ref on + // it (released by `Route`'s `Drop`). let route = html_bundle::Route::init(html_bundle); // SAFETY: `route.data` is the just-allocated NonNull (rc=1); // wrap without bumping so the map slot stays non-owning diff --git a/test/js/workerd/html-rewriter-leak.test.ts b/test/js/workerd/html-rewriter-leak.test.ts index 18c362862b03..0f770a5128ee 100644 --- a/test/js/workerd/html-rewriter-leak.test.ts +++ b/test/js/workerd/html-rewriter-leak.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isDebug } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug } from "harness"; // Each .on() / .onDocument() call heap-allocates an ElementHandler / DocumentHandler // struct via bun.default_allocator. When the HTMLRewriter is garbage-collected, @@ -77,5 +77,7 @@ test.skipIf(isDebug)( expect(deltaMB).toBeLessThan(25); expect(exitCode).toBe(0); }, - 15_000, + // ASAN instrumentation makes each pass several times slower (observed >15s + // total on aarch64-asan); same workload, just more wall-clock headroom. + isASAN ? 90_000 : 15_000, ); From 1014e9e319949821c941eb5070aad128940de9c5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 21:17:51 +0000 Subject: [PATCH 5/8] server: thread `*mut Self` through the teardown chain `schedule_deinit`'s shutdown branch frees the server synchronously, which is UB under Stacked Borrows while any `&mut self` argument up the stack is still protected. Convert `deinit_if_we_can`, `schedule_deinit`, `on_request_complete`, `on_static_request_complete` (and the `ServerLike` trait method / `AnyServer` dispatch) to the `borrow = ptr` shape: raw `*mut Self` parameters with scoped reborrows that end before the freeing call, matching `Watcher::thread_main` and `NewServer::deinit`. --- src/runtime/server/RequestContext.rs | 9 +- src/runtime/server/mod.rs | 362 +++++++++++++++++---------- src/runtime/server/server_body.rs | 16 +- test/js/bun/http/serve.test.ts | 84 +++++++ 4 files changed, 325 insertions(+), 146 deletions(-) diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index d5d8c9e8b19b..037d7fea696d 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -884,9 +884,12 @@ where // server is a BACKREF; pool put + onRequestComplete server .release_request_context(std::ptr::from_mut::(self).cast::(), HTTP3); - // SAFETY: `&mut` through the backref — the server outlives this - // context and no other borrow of it is live here. - unsafe { (*server.as_ptr()).on_request_complete() }; + // Raw `*mut S` end-to-end: completing the last pending request + // can free the server synchronously during VM shutdown + // (`deinit_if_we_can` → `schedule_deinit` → `deinit`), which must + // not happen while a `&mut` into the server allocation is + // protected (Stacked Borrows; `borrow = ptr` in src/CLAUDE.md). + ThisServer::on_request_complete(server.as_ptr()); } } diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 0735ff48debf..38900f9aa752 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1470,19 +1470,32 @@ impl NewServer { // same crate, separate file. Kept there alongside `on_reload`/`reload_static_routes` // so the Zig diff stays side-by-side. - pub fn on_static_request_complete(&mut self) { - self.pending_requests -= 1; - self.deinit_if_we_can(); - } - + /// Takes `this: *mut Self`, not `&mut self`: completing the last pending + /// request may free the server synchronously (`deinit_if_we_can` → + /// `schedule_deinit` → `deinit` when the VM is shutting down), and + /// deallocating while a `&mut self` argument is still protected is UB + /// under Stacked Borrows / Tree Borrows — see the `borrow = ptr` rule in + /// src/CLAUDE.md. + pub(crate) fn on_static_request_complete(this: *mut Self) { + // SAFETY: `this` is the live heap server pointer; the raw field + // access ends before `deinit_if_we_can`, which may free `*this`. + unsafe { (*this).pending_requests -= 1 }; + Self::deinit_if_we_can(this); + } + + /// Raw-pointer shape for the same reason as [`Self::on_static_request_complete`]. #[inline] - pub fn on_request_complete(&mut self) { - // SAFETY: `vm_mut()` is the process-static `*mut VirtualMachine` (non-null - // for the server's lifetime); `.event_loop()` returns the VM-owned - // `*mut EventLoop`. Single-threaded JS context, no aliasing `&mut`. - unsafe { (*(*self.vm_mut()).event_loop()).process_gc_timer() }; - self.pending_requests -= 1; - self.deinit_if_we_can(); + pub(crate) fn on_request_complete(this: *mut Self) { + // SAFETY: `this` is the live heap server pointer. `vm_mut()` is the + // process-static `*mut VirtualMachine` (non-null for the server's + // lifetime); `.event_loop()` returns the VM-owned `*mut EventLoop`. + // Single-threaded JS context, no aliasing `&mut`. All access into + // `*this` ends before `deinit_if_we_can`, which may free `*this`. + unsafe { + (*(*(*this).vm_mut()).event_loop()).process_gc_timer(); + (*this).pending_requests -= 1; + } + Self::deinit_if_we_can(this); } pub fn active_sockets_count(&self) -> u32 { @@ -1615,112 +1628,167 @@ impl NewServer { } self.stop_listening(abrupt); - self.deinit_if_we_can(); - } - + // `stop` is only reachable through a live JS wrapper (`server.stop()` + // / `Symbol.asyncDispose`), so `js_value` is never `Finalized` here + // and `deinit_if_we_can` cannot take the synchronous-free path while + // this frame's `&mut self` is protected. + Self::deinit_if_we_can(std::ptr::from_mut::(self)); + } + + /// Takes `this: *mut Self`, not `&mut self`: when the JS wrapper has + /// already been finalized this may free the server synchronously (via + /// `schedule_deinit` while the VM is shutting down). Deallocating while a + /// `&mut self` argument is still protected is UB under Stacked Borrows / + /// Tree Borrows, so all field access is scoped to end before the + /// potentially-freeing call — the `borrow = ptr` rule in src/CLAUDE.md; + /// same shape as `Watcher::thread_main`. #[inline] - pub fn deinit_if_we_can(&mut self) { - httplog!( - "deinitIfWeCan. requests={}, listener={}, websockets={}, has_handled_all_closed_promise={}, all_closed_promise={}, has_js_deinited={}", - self.pending_requests, - if self.listener.is_none() { - "null" - } else { - "some" - }, - if self.has_active_web_sockets() { - "active" - } else { - "no" - }, - self.flags - .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE), - if self.all_closed_promise.has_value() { - "has" - } else { - "no" - }, - matches!(self.js_value, jsc::JsRef::Finalized), - ); - - if self.pending_requests == 0 - && !self.has_listener() - && !self.has_active_web_sockets() - && !self - .flags - .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE) - && self.all_closed_promise.has_value() - // `ServerAllConnectionsClosedTask::run_from_js_thread` early-returns - // (without resolving the promise) when the VM is shutting down — - // see the `if !vm.is_shutting_down()` gate there. Skip the - // allocation entirely so a `Server::finalize()` that fires during - // `lastChanceToFinalize()` doesn't strand a `Box` (and its - // `JSPromiseStrong`) that no event-loop tick will ever drain. - && !self.vm().is_shutting_down() - { - httplog!("schedule other promise"); - // use a flag here instead of `this.all_closed_promise.get().isHandled(vm)` to prevent the race condition of this block being called - // again before the task has run. - self.flags - .insert(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE); - - let global = self.global_this(); - let vm_ref = jsc::VirtualMachine::get_mut(); - ServerAllConnectionsClosedTask::schedule( - ServerAllConnectionsClosedTask { - global_object: self.global_this, - // Duplicate the Strong handle so that we can hold two independent strong references to it. - promise: jsc::JSPromiseStrong::from_value( - self.all_closed_promise.value(), - global, - ), - tracker: jsc::AsyncTaskTracker::init(vm_ref), + pub(crate) fn deinit_if_we_can(this: *mut Self) { + let can_deinit = { + // SAFETY: `this` is the live heap server pointer and we are on + // the JS thread with exclusive access; this is the only live + // reference into `*this` and it ends with the enclosing block. + let me = unsafe { &mut *this }; + httplog!( + "deinitIfWeCan. requests={}, listener={}, websockets={}, has_handled_all_closed_promise={}, all_closed_promise={}, has_js_deinited={}", + me.pending_requests, + if me.listener.is_none() { + "null" + } else { + "some" }, - vm_ref, + if me.has_active_web_sockets() { + "active" + } else { + "no" + }, + me.flags + .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE), + if me.all_closed_promise.has_value() { + "has" + } else { + "no" + }, + matches!(me.js_value, jsc::JsRef::Finalized), ); - } - if self.pending_requests == 0 && !self.has_listener() && !self.has_active_web_sockets() { - if let Some(ws) = self.config.websocket.as_mut() { - ws.handler.app = None; - } - self.unref(); - // Detach DevServer. This is needed because there are aggressive - // tests that check for DevServer memory soundness. Keeping the JS - // binding alive should not pin `dev.memory_cost()` bytes. - if let Some(dev) = self.dev_server.take() { - if let Some(app) = self.app { - // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. - bun_opaque::opaque_deref_mut(app).clear_routes(); - } - drop(dev); // dev.deinit() + if me.pending_requests == 0 + && !me.has_listener() + && !me.has_active_web_sockets() + && !me + .flags + .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE) + && me.all_closed_promise.has_value() + // `ServerAllConnectionsClosedTask::run_from_js_thread` early-returns + // (without resolving the promise) when the VM is shutting down — + // see the `if !vm.is_shutting_down()` gate there. Skip the + // allocation entirely so a `Server::finalize()` that fires during + // `lastChanceToFinalize()` doesn't strand a `Box` (and its + // `JSPromiseStrong`) that no event-loop tick will ever drain. + && !me.vm().is_shutting_down() + { + httplog!("schedule other promise"); + // use a flag here instead of `this.all_closed_promise.get().isHandled(vm)` to prevent the race condition of this block being called + // again before the task has run. + me.flags.insert(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE); + + let global = me.global_this(); + let vm_ref = jsc::VirtualMachine::get_mut(); + ServerAllConnectionsClosedTask::schedule( + ServerAllConnectionsClosedTask { + global_object: me.global_this, + // Duplicate the Strong handle so that we can hold two independent strong references to it. + promise: jsc::JSPromiseStrong::from_value( + me.all_closed_promise.value(), + global, + ), + tracker: jsc::AsyncTaskTracker::init(vm_ref), + }, + vm_ref, + ); } + if me.pending_requests == 0 && !me.has_listener() && !me.has_active_web_sockets() { + if let Some(ws) = me.config.websocket.as_mut() { + ws.handler.app = None; + } + me.unref(); + + // Detach DevServer. This is needed because there are aggressive + // tests that check for DevServer memory soundness. Keeping the JS + // binding alive should not pin `dev.memory_cost()` bytes. + if let Some(dev) = me.dev_server.take() { + if let Some(app) = me.app { + // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. + bun_opaque::opaque_deref_mut(app).clear_routes(); + } + drop(dev); // dev.deinit() + } - // Only free the memory if the JS reference has been freed too. - if matches!(self.js_value, jsc::JsRef::Finalized) { - self.schedule_deinit(); + // Only free the memory if the JS reference has been freed too. + matches!(me.js_value, jsc::JsRef::Finalized) + } else { + false } + }; + if can_deinit { + Self::schedule_deinit(this); } } - pub fn schedule_deinit(&mut self) { - if self.flags.contains(ServerFlags::DEINIT_SCHEDULED) { - httplog!("scheduleDeinit (again)"); - return; - } - self.flags.insert(ServerFlags::DEINIT_SCHEDULED); - httplog!("scheduleDeinit"); + /// Takes `this: *mut Self`, not `&mut self`: when the VM is shutting down + /// this frees the server synchronously via [`Self::deinit`], and + /// deallocating while a `&mut self` argument is still protected is UB + /// under Stacked Borrows / Tree Borrows (the `borrow = ptr` rule in + /// src/CLAUDE.md). All field access happens through a scoped reborrow + /// that ends before the freeing call. + pub(crate) fn schedule_deinit(this: *mut Self) { + { + // SAFETY: `this` is the live heap server pointer and we are on + // the JS thread with exclusive access; this is the only live + // reference into `*this` and it ends with the enclosing block. + let me = unsafe { &mut *this }; + if me.flags.contains(ServerFlags::DEINIT_SCHEDULED) { + httplog!("scheduleDeinit (again)"); + return; + } + me.flags.insert(ServerFlags::DEINIT_SCHEDULED); + httplog!("scheduleDeinit"); + + // SAFETY: `vm_mut()` is the process-static `*mut VirtualMachine` (non-null + // for the server's lifetime); single-threaded JS context, no aliasing `&mut`. + let vm = unsafe { &mut *me.vm_mut() }; + + if !vm.is_shutting_down() { + if !me.flags.contains(ServerFlags::TERMINATED) { + // App.close can cause finalizers to run. + // scheduleDeinit can be called inside a finalizer. + // Therefore, we split it into two tasks. + me.flags.insert(ServerFlags::TERMINATED); + let app = me.app.unwrap(); + vm.enqueue_task(bun_event_loop::ManagedTask::ManagedTask::new(app, |app| { + // S008: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. + bun_opaque::opaque_deref_mut(app).close(); + Ok(()) + })); + } - // SAFETY: `vm_mut()` is the process-static `*mut VirtualMachine` (non-null - // for the server's lifetime); single-threaded JS context, no aliasing `&mut`. - let vm = unsafe { &mut *self.vm_mut() }; + vm.enqueue_task(bun_event_loop::ManagedTask::ManagedTask::new( + this, + |this| { + // SAFETY: `this` is the unique owning server pointer enqueued + // above; the task runs once on the JS thread. + Self::deinit(this); + Ok(()) + }, + )); + return; + } - if vm.is_shutting_down() { // The VM is shutting down (`is_shutting_down` is set in `on_exit`, // before `Zig__GlobalObject__destructOnExit`'s final GC / // `lastChanceToFinalize()` — typically we get here from a server // wrapper finalizer during that last collection): the event loop - // will never tick again, so the App.close + deinit task pair below would + // will never tick again, so the App.close + deinit task pair above would // be freed *unrun* by `EventLoop::deinit()` and the entire server // graph (config, user_routes, HTMLBundle routes, …) would leak. // Run both steps synchronously instead, preserving the @@ -1732,43 +1800,20 @@ impl NewServer { // active websockets, and `Listener` finalizers already // `closeAll()` synchronously on this same path (their closed // sockets are drained right after `destructOnExit`). - if !self.flags.contains(ServerFlags::TERMINATED) { - self.flags.insert(ServerFlags::TERMINATED); - if let Some(app) = self.app { + if !me.flags.contains(ServerFlags::TERMINATED) { + me.flags.insert(ServerFlags::TERMINATED); + if let Some(app) = me.app { // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. bun_opaque::opaque_deref_mut(app).close(); } } - // SAFETY: `self` is the unique heap server pointer (its JS wrapper - // has been finalized — `schedule_deinit` is only reached with - // `js_value == Finalized`). `deinit` frees it; `self` must not be - // touched after this call, so return immediately. - Self::deinit(std::ptr::from_mut::(self)); - return; } - - if !self.flags.contains(ServerFlags::TERMINATED) { - // App.close can cause finalizers to run. - // scheduleDeinit can be called inside a finalizer. - // Therefore, we split it into two tasks. - self.flags.insert(ServerFlags::TERMINATED); - let app = self.app.unwrap(); - vm.enqueue_task(bun_event_loop::ManagedTask::ManagedTask::new(app, |app| { - // S008: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. - bun_opaque::opaque_deref_mut(app).close(); - Ok(()) - })); - } - - vm.enqueue_task(bun_event_loop::ManagedTask::ManagedTask::new( - std::ptr::from_mut::(self), - |this| { - // SAFETY: `this` is the unique owning server pointer enqueued - // above; the task runs once on the JS thread. - Self::deinit(this); - Ok(()) - }, - )); + // SAFETY: `this` is the unique heap server pointer (its JS wrapper + // has been finalized — `schedule_deinit` is only reached with + // `js_value == Finalized`) and the scoped reborrow above has ended, + // so no reference into `*this` is live. `deinit` frees it; `this` + // must not be touched after this call. + Self::deinit(this); } pub fn on_listen(&mut self, socket: Option<*mut uws_sys::app::ListenSocket>) { @@ -3183,7 +3228,12 @@ pub trait ServerLike { /// trips `invalid_reference_casting`. fn vm_mut(&self) -> *mut jsc::VirtualMachine; fn config(&self) -> &ServerConfig; - fn on_request_complete(&mut self); + /// Raw `*mut Self`, not `&mut self`: completing the last pending request + /// can free the server synchronously during VM shutdown + /// (`deinit_if_we_can` → `schedule_deinit` → `deinit`), and no `&mut` + /// into the allocation may be protected when that happens — the + /// `borrow = ptr` rule in src/CLAUDE.md. + fn on_request_complete(this: *mut Self); fn dev_server(&self) -> Option<&crate::bake::DevServer::DevServer>; fn js_value(&self) -> &jsc::JsRef; fn h3_alt_svc(&self) -> Option<&[u8]>; @@ -3220,8 +3270,8 @@ impl ServerLike for NewServer { &self.config } #[inline] - fn on_request_complete(&mut self) { - Self::on_request_complete(self) + fn on_request_complete(this: *mut Self) { + Self::on_request_complete(this) } #[inline] fn dev_server(&self) -> Option<&crate::bake::DevServer::DevServer> { @@ -3398,6 +3448,40 @@ macro_rules! any_server_dispatch_mut { }}; } +/// Dispatch over the four `NewServer` monomorphizations as the typed raw +/// pointer (`$s: *mut NewServer`, no reference materialized). +/// For calls that may free the server (`on_request_complete` → +/// `deinit_if_we_can` → `schedule_deinit` → `deinit` during VM shutdown): +/// deallocating while any `&`/`&mut` into the allocation is protected is +/// Stacked-Borrows UB, so these paths stay raw end-to-end (the `borrow = ptr` +/// rule in src/CLAUDE.md). The body is monomorphized four times, so +/// `NewServer::method($s, …)` infers `` from `$s`. +macro_rules! any_server_dispatch_ptr { + ($self:expr, |$s:ident| $body:expr) => {{ + let this = $self; + // ptr was produced by `AnyServer::from` for the matching tag and is + // non-null while the server is alive; the callee derefs it. + match this.tag { + AnyServerTag::HTTPServer => { + let $s = this.ptr.cast::(); + $body + } + AnyServerTag::HTTPSServer => { + let $s = this.ptr.cast::(); + $body + } + AnyServerTag::DebugHTTPServer => { + let $s = this.ptr.cast::(); + $body + } + AnyServerTag::DebugHTTPSServer => { + let $s = this.ptr.cast::(); + $body + } + } + }}; +} + /// Dispatch over the four `NewServer` monomorphizations, simultaneously /// downcasting an [`uws::AnyResponse`] to the matching `*mut Response`. /// @@ -3541,11 +3625,11 @@ impl AnyServer { } pub fn on_request_complete(&mut self) { - any_server_dispatch_mut!(self, |s| s.on_request_complete()) + any_server_dispatch_ptr!(self, |s| NewServer::on_request_complete(s)) } pub fn on_static_request_complete(&mut self) { - any_server_dispatch_mut!(self, |s| s.on_static_request_complete()) + any_server_dispatch_ptr!(self, |s| NewServer::on_static_request_complete(s)) } pub fn dev_server(&self) -> Option<&crate::bake::DevServer::DevServer> { diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index c8c864f1a31c..e56807994181 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2712,10 +2712,18 @@ where pub fn finalize(self: Box) { httplog!("finalize"); // `deinit_if_we_can` may defer the actual free (pending requests still - // hold a ref), so hand ownership back to the raw teardown path. - let this = bun_core::heap::release(self); - this.js_value.finalize(); - this.deinit_if_we_can(); + // hold a ref), so hand ownership back to the raw teardown path. Stay + // on the raw pointer: when the VM is shutting down, + // `deinit_if_we_can` frees `*this` synchronously, which must not + // happen while any `&`/`&mut` into the allocation is protected + // (Stacked Borrows; `borrow = ptr` in src/CLAUDE.md). The `Box` + // argument itself is fine — by-value boxes carry only a weak + // protector, which permits deallocation. + let this = bun_core::heap::into_raw(self); + // SAFETY: `this` is the heap pointer just unboxed above; the field + // borrow ends before `deinit_if_we_can`. + unsafe { (*this).js_value.finalize() }; + Self::deinit_if_we_can(this); } pub fn get_all_closed_promise(&mut self, global: &JSGlobalObject) -> JSValue { diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index b31e07eeeaa6..76abdfeafc19 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -2396,3 +2396,87 @@ it.if(isPosix)("serves /bun:info over a unix socket in development mode", async expect(text).toContain("bun_version"); expect(res.status).toBe(200); }); + +// Server teardown during VM shutdown: when the server wrapper is finalized by +// the exit-time GC (`lastChanceToFinalize`), the event loop never ticks again, +// so the uws App close + server free run synchronously instead of as deferred +// event-loop tasks. +describe.concurrent("server deinit during process exit", () => { + it("stopped server finalized at exit", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const server = Bun.serve({ port: 0, fetch: () => new Response("ok") }); + const res = await fetch("http://localhost:" + server.port + "/"); + console.log(await res.text()); + server.stop(true); + process.exit(0);`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("ok\n"); + expect(exitCode).toBe(0); + }); + + it("exit with a request still in flight", async () => { + // The request context still holds a pending-request ref when the server + // wrapper is finalized, so the free is deferred until the request itself + // is torn down later in shutdown. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const server = Bun.serve({ + port: 0, + fetch() { + // Exit while this request is still pending. + queueMicrotask(() => { + console.log("exiting"); + process.exit(0); + }); + return new Promise(() => {}); + }, + }); + fetch("http://localhost:" + server.port + "/").catch(() => {});`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("exiting\n"); + expect(exitCode).toBe(0); + }); + + it("stopped server collected before exit", async () => { + // Normal (non-shutdown) path: the wrapper is GC'd while the loop is still + // ticking, so close + free go through the deferred task pair. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `{ + let server = Bun.serve({ port: 0, fetch: () => new Response("x") }); + await fetch("http://localhost:" + server.port + "/"); + server.stop(true); + server = null; + } + Bun.gc(true); + await Bun.sleep(0); + Bun.gc(true); + await Bun.sleep(0); + console.log("done");`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("done\n"); + expect(exitCode).toBe(0); + }); +}); From 12f5956382ddcfd95695e2da352371c244bca69b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 21:18:03 +0000 Subject: [PATCH 6/8] ci: detect WebKit bumps via scripts/build/deps/webkit.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasWebKitChanges() still looked for SetupWebKit.cmake, which no longer exists — the WEBKIT_VERSION pin moved to scripts/build/deps/webkit.ts — so WebKit bumps never got --jit-stress or the longer verify-baseline timeout. --- .buildkite/ci.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.buildkite/ci.mjs b/.buildkite/ci.mjs index 8d8e680a6cf1..beed6ee31047 100755 --- a/.buildkite/ci.mjs +++ b/.buildkite/ci.mjs @@ -726,7 +726,9 @@ const SDE_URL = `https://downloadmirror.intel.com/859732/sde-external-${SDE_VERS */ function hasWebKitChanges(options) { const { changedFiles = [] } = options; - return changedFiles.some(file => file.includes("SetupWebKit.cmake")); + // The WebKit pin (WEBKIT_VERSION) lives in scripts/build/deps/webkit.ts; + // it was previously in cmake/tools/SetupWebKit.cmake, which no longer exists. + return changedFiles.some(file => file.includes("scripts/build/deps/webkit.ts")); } /** From 4625f57f43cf70b3bbf780e1f6695ced25b17e9f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 23:09:17 +0000 Subject: [PATCH 7/8] ci: retrigger From 212daf8ce8ab360ad6548e73aad20d15ee5d5fb1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 2 Jun 2026 23:47:51 +0000 Subject: [PATCH 8/8] server: take AnyServer by value in request-complete dispatchers `AnyServer::{on_request_complete, on_static_request_complete}` reach `deinit_if_we_can`, which drops the server-owned `Box`; the DevServer call sites passed `&mut self` pointing into that same box (`dev.server.as_mut()`), leaving a protected reference live across the free. `AnyServer` is `Copy`, so take `self` by value and copy the handle out at the call sites. --- src/runtime/bake/DevServer.rs | 19 ++++++++++--------- .../bake/DevServer/ErrorReportRequest.rs | 12 +++++------- src/runtime/server/FileRoute.rs | 2 +- src/runtime/server/NodeHTTPResponse.rs | 2 +- src/runtime/server/StaticRoute.rs | 2 +- src/runtime/server/mod.rs | 12 ++++++++++-- 6 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index d7000bc87052..397ef3d79b08 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -3925,8 +3925,11 @@ pub(super) fn finalize_bundle( dev.start_next_bundle_if_present(); - // Unref the ref added in `start_async_bundle` - if let Some(server) = dev.server.as_mut() { + // Unref the ref added in `start_async_bundle`. Copy the `AnyServer` + // handle out (no `as_mut()`): the call can drop this very + // `Box` via `deinit_if_we_can`, so no reference into it + // may be live across the call. + if let Some(server) = dev.server { server.on_static_request_complete(); } }; @@ -6683,14 +6686,12 @@ impl UnrefSourceMapRequest { // SAFETY: caller contract — ctx is the original Box allocation; no // live borrow of *ctx exists. let ctx = unsafe { bun_core::heap::take(ctx) }; + // Copy the `AnyServer` handle out of the DevServer (no `as_mut()`): + // the call can drop the `Box` via `deinit_if_we_can`, so + // no reference into it may be live across the call. // SAFETY: dev outlives the request - unsafe { - (*ctx.dev) - .server - .as_mut() - .unwrap() - .on_static_request_complete() - }; + let server = unsafe { (*ctx.dev).server.unwrap() }; + server.on_static_request_complete(); drop(ctx); } diff --git a/src/runtime/bake/DevServer/ErrorReportRequest.rs b/src/runtime/bake/DevServer/ErrorReportRequest.rs index 1a252de49a9d..7d251d1dd42a 100644 --- a/src/runtime/bake/DevServer/ErrorReportRequest.rs +++ b/src/runtime/bake/DevServer/ErrorReportRequest.rs @@ -85,18 +85,16 @@ impl ErrorReportRequest { /// `ctx` must be the pointer returned by `heap::alloc` in `run`; called /// exactly once (success path here, or via `on_error` on abort/error). pub(crate) fn finalize(ctx: *mut ErrorReportRequest) { + // Copy the `AnyServer` handle out of the DevServer (no `as_mut()`): + // the call can drop the `Box` via `deinit_if_we_can`, so + // no reference into it may be live across the call. // SAFETY: `ctx` is the original Box allocation produced by `run`; no // live borrow of `*ctx` exists (BodyReaderHandler hands us the raw // pointer, never `&mut self`). Only reachable via `on_body`/`on_error`, // both of which uphold this contract. unsafe { - (*ctx) - .dev - .get_mut() - .server - .as_mut() - .unwrap() - .on_static_request_complete(); + let server = (*ctx).dev.get_mut().server.unwrap(); + server.on_static_request_complete(); drop(bun_core::heap::take(ctx)); } } diff --git a/src/runtime/server/FileRoute.rs b/src/runtime/server/FileRoute.rs index 39764eaa190d..9ece06ce0b47 100644 --- a/src/runtime/server/FileRoute.rs +++ b/src/runtime/server/FileRoute.rs @@ -582,7 +582,7 @@ impl FileRoute { resp.clear_timeout(); // SAFETY: `this` is live (ref held by caller); `deref()` may free it. unsafe { - if let Some(mut server) = (*this).server.get() { + if let Some(server) = (*this).server.get() { server.on_static_request_complete(); } Self::deref(this); diff --git a/src/runtime/server/NodeHTTPResponse.rs b/src/runtime/server/NodeHTTPResponse.rs index 01e5c2e6b297..58f4edbf49a2 100644 --- a/src/runtime/server/NodeHTTPResponse.rs +++ b/src/runtime/server/NodeHTTPResponse.rs @@ -574,7 +574,7 @@ impl NodeHTTPResponse { self.buffered_request_body_data_during_pause .with_mut(|b| b.clear_and_free()); - let mut server = self.server; + let server = self.server; self.poll_ref.with_mut(|r| r.unref(vm)); self.unregister_auto_flush(); diff --git a/src/runtime/server/StaticRoute.rs b/src/runtime/server/StaticRoute.rs index d1a142ebcbc0..f0f79f3ceb34 100644 --- a/src/runtime/server/StaticRoute.rs +++ b/src/runtime/server/StaticRoute.rs @@ -388,7 +388,7 @@ impl StaticRoute { resp.clear_aborted(); resp.clear_on_writable(); resp.clear_timeout(); - if let Some(mut server) = (*this).server.get() { + if let Some(server) = (*this).server.get() { server.on_static_request_complete(); } Self::deref_(this); diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 38900f9aa752..e795726d51d6 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -3624,11 +3624,19 @@ impl AnyServer { any_server_dispatch_resp!(self, resp, |s, r| NewServer::on_request(s, req, r)) } - pub fn on_request_complete(&mut self) { + /// By-value `self` (`AnyServer` is `Copy`), not `&mut self`: these reach + /// `deinit_if_we_can`, which drops the server-owned `Box` — + /// and the DevServer call sites store their `AnyServer` handle *inside* + /// that box (`dev.server`), so a `&mut self` argument would be a + /// protected reference into the very allocation being freed (Stacked + /// Borrows; `borrow = ptr` in src/CLAUDE.md). Callers copy the handle + /// out first. + pub fn on_request_complete(self) { any_server_dispatch_ptr!(self, |s| NewServer::on_request_complete(s)) } - pub fn on_static_request_complete(&mut self) { + /// By-value `self` for the same reason as [`Self::on_request_complete`]. + pub fn on_static_request_complete(self) { any_server_dispatch_ptr!(self, |s| NewServer::on_static_request_complete(s)) }