diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index a04e0c5148d1..1b688b9ac5b7 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -97,6 +97,7 @@ function getNodeParallelTestTimeout(testPath) { if (testPath.includes("test-cluster-")) return 60_000; // cluster IPC + socket-handle passing is process-heavy under runner concurrency if (testPath.includes("-docker-")) return 60_000; if (testPath.includes("test-stdin-pipe-large")) return 60_000; // pipes 1MB stdin->stdout through an extra child process; slow under runner concurrency + if (testPath.includes("test-require-builtins")) return 120_000; // requires every builtin module; ~60s alone under local ASAN debug builds // test-fs-read-stream-pos.js exit condition is a pure timing race (writer must append // between two consecutive ReadStream preads) with a 90s upstream safety timer; solo // runtimes are ~1s on linux-x64 but 1-40s on Windows since #34834 raised its timer @@ -896,6 +897,15 @@ async function runTests() { // (test-child-process-*-detached.js), which this flag defeats. env.BUN_FEATURE_FLAG_NO_ORPHANS = "1"; } + if (isMacOS && basename(execPath).includes("asan")) { + // ASAN debug builds resolve asan-dyld-shim.dylib via @rpath + // relative to the binary. Tests that copy process.execPath + // elsewhere (fork-exec-path, stdin-from-file-spawn, ...) lose + // that anchor; give dyld a last-resort search path (prepending + // rather than clobbering any inherited value). + const dir = dirname(realpathSync(execPath)); + env.DYLD_FALLBACK_LIBRARY_PATH = [dir, process.env.DYLD_FALLBACK_LIBRARY_PATH].filter(Boolean).join(":"); + } if ((basename(execPath).includes("asan") || !isCI) && shouldValidateExceptions(testPath)) { env.BUN_JSC_validateExceptionChecks = "1"; env.BUN_JSC_dumpSimulatedThrows = "1"; @@ -1789,6 +1799,12 @@ async function spawnBun(execPath, { args, cwd, timeout, gracefulTimeout, idleTim BUN_RUNTIME_TRANSPILER_CACHE_PATH: "0", BUN_INSTALL_CACHE_DIR: tmpdirPath, SHELLOPTS: isWindows ? "igncr" : undefined, // ignore "\r" on Windows + // common/tmpdir.js reads NODE_TEST_DIR — point it at the per-test tmpdir + // so its `.tmp.` subdir is swept by the finally-rmSync below even + // when the test aborts (ASAN abort_on_error skips its exit handler). + // POSIX-only: there is no Windows ASAN lane, and relocating testRoot to + // realpath(%TEMP%) breaks path-shape assumptions in a few Windows tests. + NODE_TEST_DIR: isWindows ? undefined : tmpdirPath, TEST_TMPDIR: tmpdirPath, // Used in Node.js tests. ...(typeof remapPort == "number" ? { BUN_CRASH_REPORT_URL: `http://localhost:${remapPort}` } diff --git a/src/CLAUDE.md b/src/CLAUDE.md index 53c8159a6cae..625c09bd91df 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -159,14 +159,16 @@ let url = URL::from_utf8(href)?; // Option> // caller owns the C++ object — destroy it when done: // unsafe { URL::destroy(url.as_ptr()) } -url.protocol() // bun_core::String -url.pathname() // bun_core::String -url.host() // bun_core::String — the hostname WITHOUT the port (opposite of JS `host`!) +url.protocol() // bun_core::OwnedString (+1; Drop derefs) +url.pathname() // bun_core::OwnedString +url.host() // bun_core::OwnedString — the hostname WITHOUT the port (opposite of JS `host`!) url.port() // u32 (u32::MAX = unset; otherwise u16 range) ``` `URL::href_from_js`, `URL::file_url_from_string`, `URL::path_from_file_url` -do whole-string conversions. The JSC-free shim `bun_url::whatwg::URL` exposes +do whole-string conversions. Every string getter returns `OwnedString` — use +`.into_inner()` only when you must transfer the +1 out (e.g. into a struct +field that will deref later). The JSC-free shim `bun_url::whatwg::URL` exposes `hostname()`, which returns the host WITH the port (also the opposite of JS `hostname`) — so `bun_jsc::URL::host` and `bun_url::whatwg::URL::hostname` are effectively swapped relative to their JS namesakes. diff --git a/src/http/lib.rs b/src/http/lib.rs index b5edd7cad4eb..c028b8ac96c3 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -981,7 +981,7 @@ use bun_boringssl as boringssl; use bun_collections::{ArrayHashMap, VecExt}; use bun_core::StringBuilder; use bun_core::{FeatureFlags, Global, Output}; -use bun_core::{OwnedString, String as BunString, Tag as BunStringTag, strings}; +use bun_core::{String as BunString, Tag as BunStringTag, strings}; use bun_http_types::ETag::StringPointer; use bun_uws as uws; // the std Wyhash algorithm, not Wyhash11. @@ -5018,7 +5018,7 @@ impl<'a> HTTPClient<'a> { debug_assert!(string_builder.cap == string_builder.len); let input = BunString::borrow_utf8(string_builder.allocated_slice()); - let normalized_url = OwnedString::new(bun_url::href_from_string(&input)); + let normalized_url = bun_url::href_from_string(&input); if normalized_url.tag() == BunStringTag::Dead { // URL__getHref failed, dont pass dead tagged string to toOwnedSlice. return Err(crate::Error::RedirectURLInvalid); @@ -5074,7 +5074,7 @@ impl<'a> HTTPClient<'a> { debug_assert!(string_builder.cap == string_builder.len); let input = BunString::borrow_utf8(string_builder.allocated_slice()); - let normalized_url = OwnedString::new(bun_url::href_from_string(&input)); + let normalized_url = bun_url::href_from_string(&input); if normalized_url.tag() == BunStringTag::Dead { return Err(crate::Error::RedirectURLInvalid); } @@ -5098,7 +5098,7 @@ impl<'a> HTTPClient<'a> { let base = BunString::borrow_utf8(original_url.href); let rel = BunString::borrow_utf8(location); - let new_url_ = OwnedString::new(bun_url::join(&base, &rel)); + let new_url_ = bun_url::join(&base, &rel); if new_url_.is_empty() { return Err(crate::Error::InvalidRedirectURL); diff --git a/src/install/NetworkTask.rs b/src/install/NetworkTask.rs index 9f1edc07278d..aa6456da7d6f 100644 --- a/src/install/NetworkTask.rs +++ b/src/install/NetworkTask.rs @@ -466,10 +466,10 @@ impl NetworkTask { // `OwnedString` derefs the WTF-backed result on scope exit — // covers both the // success path and the InvalidURL early returns below. - let tmp = bun_core::OwnedString::new(bun_url::join( + let tmp = bun_url::join( &bun_core::String::borrow_utf8(scope.url.href()), &bun_core::String::borrow_utf8(encoded_name), - )); + ); if tmp.tag() == bun_core::Tag::Dead { if !is_optional { diff --git a/src/install/hosted_git_info.rs b/src/install/hosted_git_info.rs index 66e1afac0993..e71d20fba5a4 100644 --- a/src/install/hosted_git_info.rs +++ b/src/install/hosted_git_info.rs @@ -54,7 +54,7 @@ use core::ptr::NonNull; use bun_alloc::AllocError; use bun_core::StringBuilder; -use bun_core::{OwnedString, strings}; +use bun_core::strings; use bun_url::PercentEncoding; use bun_url::whatwg::URL as JscUrl; use enum_map::{Enum, EnumMap}; @@ -973,7 +973,7 @@ impl HostProvider { /// Parse a URL and return the appropriate host provider, if any. fn from_url(url: &JscUrl) -> Option { - let proto_str = OwnedString::new(url.protocol()); + let proto_str = url.protocol(); // Try shortcut first (github:, gitlab:, etc.) if let Some(provider) = HostProvider::from_shortcut(proto_str.byte_slice(), false) { @@ -985,7 +985,7 @@ impl HostProvider { /// Given a URL, use the domain in the URL to find the appropriate host provider. fn from_url_domain(url: &JscUrl) -> Option { - let hostname_str = OwnedString::new(url.hostname()); + let hostname_str = url.hostname(); let hostname_utf8 = hostname_str.to_utf8(); let hostname = strings::without_prefix(hostname_utf8.slice(), b"www."); @@ -1074,7 +1074,7 @@ pub(crate) mod formatters { // valid until it's copied into the StringBuilder. let fragment_utf8; let committish: Option<&[u8]> = if type_part.is_none() { - let fragment_str = OwnedString::new(url.fragment_identifier()); + let fragment_str = url.fragment_identifier(); fragment_utf8 = fragment_str.to_utf8(); let fragment = fragment_utf8.slice(); if !fragment.is_empty() { @@ -1135,7 +1135,7 @@ pub(crate) mod formatters { return Ok(None); } - let fragment_str = OwnedString::new(url.fragment_identifier()); + let fragment_str = url.fragment_identifier(); let fragment_utf8 = fragment_str.to_utf8(); let fragment = fragment_utf8.slice(); let committish: Option<&[u8]> = if !fragment.is_empty() { @@ -1190,7 +1190,7 @@ pub(crate) mod formatters { return Ok(None); } - let fragment_str = OwnedString::new(url.fragment_identifier()); + let fragment_str = url.fragment_identifier(); let fragment_utf8 = fragment_str.to_utf8(); let committish = fragment_utf8.slice(); @@ -1255,7 +1255,7 @@ pub(crate) mod formatters { return Ok(None); } - let fragment_str = OwnedString::new(url.fragment_identifier()); + let fragment_str = url.fragment_identifier(); let fragment_utf8 = fragment_str.to_utf8(); let fragment = fragment_utf8.slice(); let committish: Option<&[u8]> = if !fragment.is_empty() { @@ -1332,7 +1332,7 @@ pub(crate) mod formatters { return Ok(None); } - let fragment_str = OwnedString::new(url.fragment_identifier()); + let fragment_str = url.fragment_identifier(); let fragment_utf8 = fragment_str.to_utf8(); let fragment = fragment_utf8.slice(); let committish: Option<&[u8]> = if !fragment.is_empty() { diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index 4af87463bd8f..74c843d00a2b 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -1382,8 +1382,6 @@ class ChildProcess extends EventEmitter { const detachedOption = options.detached; this.#stdioOptions = bunStdio; - const stdioCount = stdio.length; - const hasSocketsToEagerlyLoad = stdioCount >= 3; validateString(options.file, "options.file"); var file; @@ -1415,12 +1413,10 @@ class ChildProcess extends EventEmitter { this.pid = this.#handle.pid; $debug("ChildProcess: onExit", exitCode, signalCode, err, this.pid); - if (hasSocketsToEagerlyLoad) { - process.nextTick(() => { - void this.stdio; - $debug("ChildProcess: onExit", exitCode, signalCode, err, this.pid); - }); - } + process.nextTick(() => { + void this.stdio; + $debug("ChildProcess: onExit", exitCode, signalCode, err, this.pid); + }); process.nextTick( (exitCode, signalCode, err) => this.#handleOnExit(exitCode, signalCode, err), @@ -1460,10 +1456,8 @@ class ChildProcess extends EventEmitter { if (options[kFromNode]) this.#closesNeeded += 1; } - if (hasSocketsToEagerlyLoad) { - for (let item of this.stdio) { - item?.ref?.(); - } + for (let item of this.stdio) { + item?.ref?.(); } } catch (ex) { const exCode = ex != null && typeof ex === "object" && Object.hasOwn(ex, "code") ? ex.code : undefined; diff --git a/src/jsc/URL.rs b/src/jsc/URL.rs index 36c2dc07e26d..ca44b3d1e834 100644 --- a/src/jsc/URL.rs +++ b/src/jsc/URL.rs @@ -1,6 +1,6 @@ use core::ptr::NonNull; -use bun_core::String; +use bun_core::{OwnedString, String}; use bun_jsc::{JSGlobalObject, JSValue, JsResult}; bun_opaque::opaque_ffi! { @@ -8,32 +8,31 @@ bun_opaque::opaque_ffi! { pub struct URL; } -// Getters take `&URL` (non-null `*const URL` at the C ABI; BunString.cpp never -// mutates the WTF::URL on read). `&mut String` for the in/out params is -// ABI-identical to non-null `*mut String`. `URL__deinit` consumes the C++ -// allocation, so it keeps a raw pointer and stays `unsafe fn`. +// Getters take `&URL` (BunString.cpp never mutates on read); `URL__deinit` +// consumes the C++ allocation so it stays `unsafe fn`. String returns are +1 +// (`Bun::toStringRef`) → `OwnedString` (repr(transparent)) for scope-exit deref. unsafe extern "C" { safe fn URL__fromJS(value: JSValue, global: &JSGlobalObject) -> *mut URL; safe fn URL__fromString(input: &mut String) -> *mut URL; - safe fn URL__protocol(url: &URL) -> String; - safe fn URL__username(url: &URL) -> String; - safe fn URL__password(url: &URL) -> String; - safe fn URL__host(url: &URL) -> String; + safe fn URL__protocol(url: &URL) -> OwnedString; + safe fn URL__username(url: &URL) -> OwnedString; + safe fn URL__password(url: &URL) -> OwnedString; + safe fn URL__host(url: &URL) -> OwnedString; safe fn URL__port(url: &URL) -> u32; fn URL__deinit(url: *mut URL); - safe fn URL__pathname(url: &URL) -> String; - safe fn URL__getHrefFromJS(value: JSValue, global: &JSGlobalObject) -> String; - safe fn URL__getFileURLString(input: &mut String) -> String; - safe fn URL__pathFromFileURL(input: &mut String) -> String; + safe fn URL__pathname(url: &URL) -> OwnedString; + safe fn URL__getHrefFromJS(value: JSValue, global: &JSGlobalObject) -> OwnedString; + safe fn URL__getFileURLString(input: &mut String) -> OwnedString; + safe fn URL__pathFromFileURL(input: &mut String) -> OwnedString; } impl URL { - pub fn file_url_from_string(str: String) -> String { + pub fn file_url_from_string(str: String) -> OwnedString { let mut input = str; URL__getFileURLString(&mut input) } - pub fn path_from_file_url(str: String) -> String { + pub fn path_from_file_url(str: String) -> OwnedString { let mut input = str; URL__pathFromFileURL(&mut input) } @@ -41,7 +40,7 @@ impl URL { /// This percent-encodes the URL, punycode-encodes the hostname, and returns the result /// If it fails, the tag is marked Dead #[track_caller] - pub fn href_from_js(value: JSValue, global: &JSGlobalObject) -> JsResult { + pub fn href_from_js(value: JSValue, global: &JSGlobalObject) -> JsResult { crate::call_check_slow(global, || URL__getHrefFromJS(value, global)) } @@ -61,27 +60,21 @@ impl URL { // from_js/from_string/from_utf8 return an owned C++ heap pointer that the // caller must destroy(). - pub fn protocol(&self) -> String { + pub fn protocol(&self) -> OwnedString { URL__protocol(self) } - pub fn username(&self) -> String { + pub fn username(&self) -> OwnedString { URL__username(self) } - pub fn password(&self) -> String { + pub fn password(&self) -> OwnedString { URL__password(self) } - /// Returns the host WITHOUT the port. - /// - /// Note that this does NOT match JS behavior, which returns the host with the port. The - /// with-port form lives on the JSC-free shim as `bun_url::whatwg::URL::hostname`. - /// - /// ```text - /// URL("http://example.com:8080").host() => "example.com" - /// ``` - pub fn host(&self) -> String { + /// Host WITHOUT the port — opposite of JS `url.host` (https://url.spec.whatwg.org/#dom-url-host). + /// The with-port form is `bun_url::whatwg::URL::hostname`. + pub fn host(&self) -> OwnedString { URL__host(self) } @@ -98,7 +91,7 @@ impl URL { unsafe { URL__deinit(this) } } - pub fn pathname(&self) -> String { + pub fn pathname(&self) -> OwnedString { URL__pathname(self) } } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 1182f24abe2f..7bb197efdff0 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1601,9 +1601,7 @@ impl VirtualMachine { // JSC `Strong`/`Weak` handles against a live heap. self.event_loop_mut().release_queued_tasks_for_shutdown(); - if let Some(rare) = self.rare_data.as_deref_mut() { - rare.release_js_handles(); - } + self.release_strong_refs_before_teardown(); Zig__GlobalObject__destructOnExit(self.global()); @@ -1619,6 +1617,22 @@ impl VirtualMachine { } bun_core::Global::exit(u32::from(self.exit_handler.exit_code)) } + + /// Release every Rust-side JSC `Strong` (fields, `RareData`, `RuntimeState`) + /// while the HandleSet is live — dropping after `destructOnExit`/teardownJSCVM + /// is an ASAN UAF in `Bun__StrongRef__delete`. Idempotent. + pub fn release_strong_refs_before_teardown(&mut self) { + self.overridden_main.deinit(); + self.entry_point_result.value.deinit(); + if let Some(rare) = self.rare_data.as_deref_mut() { + rare.release_js_handles(); + } + if let Some(hooks) = runtime_hooks() { + // SAFETY: JS thread, live VM; the hook only touches the + // per-thread RuntimeState it owns. + unsafe { (hooks.release_runtime_state_js_handles)(core::ptr::from_mut(self)) }; + } + } } extern crate alloc; @@ -1659,6 +1673,9 @@ pub struct RuntimeHooks { /// `heap::take`s it and clears its thread-local cache. Without this slot /// every worker leaked one box. pub deinit_runtime_state: unsafe fn(vm: *mut VirtualMachine, state: RuntimeState), + /// Release `RuntimeState`'s JSC `Strong` handles (SQL on_query callbacks) + /// before `destructOnExit` — dropping later UAFs the freed HandleSet. + pub release_runtime_state_js_handles: unsafe fn(vm: *mut VirtualMachine), /// `ServerEntryPoint.generate(watch, entry_path)` — produces the synthetic /// `bun:main` module body for `entry_path`. Returns `false` on error /// (error already logged into `vm.log`). @@ -4315,6 +4332,11 @@ impl VirtualMachine { } /// Worker-thread teardown. pub fn destroy(&mut self) { + // No-op on `global_exit`/worker paths (already released, idempotent); + // `bake::production`'s unwind guard reaches here with the JSC VM still + // live and no prior release, so this is its reclaim point. + self.release_strong_refs_before_teardown(); + self.regular_event_loop.deinit(); self.macro_event_loop.deinit(); @@ -4370,8 +4392,6 @@ impl VirtualMachine { drop(core::mem::take(&mut self.resolved_path_dups)); - self.overridden_main.deinit(); - // `timer`/`entry_point` live in the high-tier `RuntimeState` box, so // dispatch the reclaim through the hook. if let Some(hooks) = runtime_hooks() { diff --git a/src/jsc/bindings/ConsoleObject.h b/src/jsc/bindings/ConsoleObject.h index 9628d40c0053..5c2cc22043c9 100644 --- a/src/jsc/bindings/ConsoleObject.h +++ b/src/jsc/bindings/ConsoleObject.h @@ -10,6 +10,9 @@ using namespace JSC; class ConsoleObject final : public JSC::ConsoleClient { WTF_DEPRECATED_MAKE_FAST_ALLOCATED(ConsoleObject); + // FAST_ALLOCATED shadows CanMakeThreadSafeCheckedPtr's destroying-delete, + // so redeclare it (matches JSC::JSGlobalObjectConsoleClient). + WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(ConsoleObject); public: ~ConsoleObject() final {} diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index a3aa700f0fbe..95c67bac75a9 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -1117,7 +1117,10 @@ void GlobalObject::promiseRejectionTracker(JSGlobalObject* obj, JSC::JSPromise* void GlobalObject::setConsole(void* console) { - this->setConsoleClient(new Bun::ConsoleObject(console)); + // JSGlobalObject::setConsoleClient only stores a WeakPtr — own it here so + // per-ShadowRealm globals free their ConsoleObject + buffered messages. + m_ownedConsoleClient = makeUnique(console); + this->setConsoleClient(m_ownedConsoleClient.get()); } JSC_DEFINE_CUSTOM_GETTER(errorConstructorPrepareStackTraceGetter, diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index 628b1db8e66b..29ee851c940c 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -779,6 +779,10 @@ class GlobalObject : public Bun::GlobalScope { Lock m_gcLock; Ref m_world; RefPtr m_performance { nullptr }; + // Owns the ConsoleClient installed by setConsole(). JSGlobalObject only + // keeps a WeakPtr, so without an owner every global (notably each + // ShadowRealm-derived one) leaks its ConsoleObject and buffered messages. + std::unique_ptr m_ownedConsoleClient; public: // De-optimization once `require("module")._resolveFilename` is written to diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 7f30e0f913e2..63e0c9c7a5b0 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1200,6 +1200,8 @@ impl WebWorker { /// null and skips wakeup() instead of touching /// memory freed in step 5. /// 2. `vm.onExit()` — user 'exit' handlers run; needs the JSC VM. + /// `release_strong_refs_before_teardown()` — drop every Rust-side + /// `Strong` while the HandleSet is live. /// 3. `teardownJSCVM()` — collectNow + vm.deref (single — the /// API-lock path takes no extra /// `RefPtr`, see the `thread_main` @@ -1311,9 +1313,7 @@ impl WebWorker { // worker VM is dealloc'd-without-Drop so anything still in // self.tasks leaks. Mirrors the global_exit() ordering. vm.event_loop_mut().release_queued_tasks_for_shutdown(); - if let Some(rare) = vm.rare_data.as_deref_mut() { - rare.release_js_handles(); - } + vm.release_strong_refs_before_teardown(); exit_code = i32::from(vm.exit_handler.exit_code); global_object = Some(vm.global); } diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index f3727e048041..28fd9318eec2 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1176,7 +1176,7 @@ fn do_resolve_with_args( // by value without `dupe_ref()`/`deref()` refcount churn. Only the // URL-decoded branch produces a string we must release. let specifier_for_resolve = if specifier.has_prefix_comptime(b"file://") { - owned.decoded_specifier = jsc::URL::path_from_file_url(specifier); + owned.decoded_specifier = jsc::URL::path_from_file_url(specifier).into_inner(); owned.decoded_specifier } else { specifier diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 1a5f86b7ec84..d74db595c57b 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1448,11 +1448,25 @@ mod vm_loader_ctx { } } +/// Hook: drop the JSC `Strong` handles held inside `RuntimeState` while the +/// JSC VM (and its HandleSet) is still alive. Idempotent — `deinit()` leaves +/// the `StrongOptional`s empty so the later `RuntimeState` drop is a no-op. +unsafe fn release_runtime_state_js_handles(_vm: *mut VirtualMachine) { + let state = runtime_state(); + if state.is_null() { + return; + } + // SAFETY: `state` is the live per-thread `RuntimeState`; this runs on the + // JS thread before any teardown frees it. + unsafe { &mut *state }.sql_rare.release_js_handles(); +} + /// The static `RuntimeHooks` instance handed to `bun_jsc`. #[unsafe(no_mangle)] static __BUN_RUNTIME_HOOKS: RuntimeHooks = RuntimeHooks { init_runtime_state, deinit_runtime_state, + release_runtime_state_js_handles, generate_entry_point, load_preloads, ensure_debugger, diff --git a/src/runtime/socket/SocketAddress.rs b/src/runtime/socket/SocketAddress.rs index f87c39caef4f..ca0aa943ee74 100644 --- a/src/runtime/socket/SocketAddress.rs +++ b/src/runtime/socket/SocketAddress.rs @@ -218,7 +218,7 @@ impl SocketAddress { // `BackRef` liveness invariant holds; `Deref` encapsulates the single // `NonNull::as_ref` site. let url = bun_ptr::BackRef::from(url_ptr); - let host: BunString = url.host(); + let host = url.host(); let port_: u16 = { let port32 = url.port(); if port32 > u32::from(u16::MAX) { diff --git a/src/runtime/test_runner/ScopeFunctions.rs b/src/runtime/test_runner/ScopeFunctions.rs index a30dd6f486b5..935d011c6040 100644 --- a/src/runtime/test_runner/ScopeFunctions.rs +++ b/src/runtime/test_runner/ScopeFunctions.rs @@ -807,10 +807,10 @@ fn create_unbound(global: &JSGlobalObject, mode: Mode, each: JSValue, cfg: BaseS } fn bind(value: JSValue, global: &JSGlobalObject, name: BunString) -> JsResult { - // `#[bun_jsc::host_fn]` on `call_as_function` emits the C-ABI thunk - // `__jsc_host_call_as_function`; `JSFunction::create` wants the raw - // `JSHostFn` shape, not the safe Rust signature. - let call_fn = bun_jsc::JSFunction::create(global, name.clone(), __jsc_host_call_as_function, 1, Default::default()); + // `#[bun_jsc::host_fn]` emits the raw `JSHostFn` thunk `__jsc_host_call_as_function`. + // `name` is borrowed (bit-copy); `JSFunction__createFromZig` only reads it, + // so a `clone()` would leak the StringImpl. + let call_fn = bun_jsc::JSFunction::create(global, name, __jsc_host_call_as_function, 1, Default::default()); let bound = JSValueTestExt::bind(call_fn, global, value, &name, 1.0, &[])?; set_prototype_direct(bound, value.get_prototype(global), global)?; Ok(bound) diff --git a/src/runtime/webcore/Request.rs b/src/runtime/webcore/Request.rs index 075d91942968..61b4f3a61b2f 100644 --- a/src/runtime/webcore/Request.rs +++ b/src/runtime/webcore/Request.rs @@ -942,9 +942,8 @@ impl Request { if !href.is_empty() { if core::ptr::eq(href.byte_slice().as_ptr(), url.as_ptr()) { self.url.set(BunString::clone_latin1(&url[..href.length()])); - href.deref(); } else { - self.url.set(href); + self.url.set(href.into_inner()); } } else { // TODO: what is the right thing to do for invalid URLS? @@ -977,7 +976,7 @@ impl Request { let href = bun_url::href_from_string(&self.url.get()); // TODO: what is the right thing to do for invalid URLS? if !href.is_empty() { - self.url.set(href); + self.url.set(href.into_inner()); } return Ok(()); @@ -1483,7 +1482,7 @@ impl Request { // we increment the reference count on usage above, so we must // decrement it to be perfectly balanced. - req.url.set(href); + req.url.set(href.into_inner()); if matches!(req.body_value(), BodyValue::Blob(_)) && req.headers.get().is_some() { if let BodyValue::Blob(blob) = req.body_value() { diff --git a/src/runtime/webcore/Response.rs b/src/runtime/webcore/Response.rs index 1cb48ba9333a..03e7b2b074e8 100644 --- a/src/runtime/webcore/Response.rs +++ b/src/runtime/webcore/Response.rs @@ -1180,7 +1180,7 @@ impl Response { // https://fetch.spec.whatwg.org/#dom-response-redirect steps 1 & 6: `Location` // gets the serialization of the parsed url, not the raw input. Non-absolute // input keeps the raw string: relative redirects are documented Bun behavior. - let href = OwnedString::new(bun_url::href_from_string(&url_string)); + let href = bun_url::href_from_string(&url_string); // The JS string's own WTF string (no re-encode), same as `Headers.prototype.set`. let location = if href.is_empty() { &url_string } else { &href }; headers.put(HTTPHeaderName::Location, location, global_this)?; diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index 1456f09493bb..1cb2672a58ab 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -243,9 +243,7 @@ fn bun_fetch_preconnect( )); } - // `href_from_js` returns a +1 (`Bun::toStringRef`). `bun_core::String` is - // `Copy` with no `Drop`, so wrap in `OwnedString` for the scope-exit deref. - let url_str = bun_core::OwnedString::new(jsc::URL::href_from_js(arguments[0], global_object)?); + let url_str = jsc::URL::href_from_js(arguments[0], global_object)?; if url_str.tag() == BunStringTag::Dead { return Err(global_object @@ -327,7 +325,7 @@ impl StringOrURL { if out.tag() == BunStringTag::Dead { return Ok(None); } - Ok(Some(out)) + Ok(Some(out.into_inner())) } } @@ -1007,13 +1005,7 @@ fn fetch_impl( // Handle string format: proxy: "http://proxy.example.com:8080" if is_url_instance || (proxy_arg.is_string() && proxy_arg.get_length(ctx)? > 0) { - // `href_from_js` returns a +1 WTFStringImpl ref; `bun_core::String` - // is `Copy` with no `Drop`, so wrap in `OwnedString` for scope-exit - // deref (mirrors `defer href.deref()` in fetch.zig). - let href = bun_core::OwnedString::new(jsc::URL::href_from_js( - proxy_arg, - global_this, - )?); + let href = jsc::URL::href_from_js(proxy_arg, global_this)?; if href.tag() == BunStringTag::Dead { let err = ctx.to_type_error( jsc::ErrorCode::INVALID_ARG_VALUE, @@ -1048,11 +1040,7 @@ fn fetch_impl( if !proxy_url_arg.is_undefined_or_null() { // Deliberately no type gate: `href_from_js` accepts a string // or a `URL` object and is the sole validator (Dead = invalid). - // +1 ref; see the string-format branch above. - let href = bun_core::OwnedString::new(jsc::URL::href_from_js( - proxy_url_arg, - global_this, - )?); + let href = jsc::URL::href_from_js(proxy_url_arg, global_this)?; if href.tag() == BunStringTag::Dead { let err = ctx.to_type_error( jsc::ErrorCode::INVALID_ARG_VALUE, @@ -1553,7 +1541,8 @@ fn fetch_impl( } }; - url_string = jsc::URL::file_url_from_string(BunString::borrow_utf8(temp_file_path)); + url_string = + jsc::URL::file_url_from_string(BunString::borrow_utf8(temp_file_path)).into_inner(); // `find_or_create_file_from_path` is typed against the // `crate::webcore::node_types` stub (until it's swapped to a diff --git a/src/sourcemap_jsc/JSSourceMap.rs b/src/sourcemap_jsc/JSSourceMap.rs index d8468a05dfee..b966d416fca9 100644 --- a/src/sourcemap_jsc/JSSourceMap.rs +++ b/src/sourcemap_jsc/JSSourceMap.rs @@ -66,7 +66,7 @@ fn find_source_map(global: &JSGlobalObject, frame: &CallFrame) -> JsResult Option>; - safe fn URL__protocol(url: &URL) -> String; - safe fn URL__href(url: &URL) -> String; - safe fn URL__hostname(url: &URL) -> String; + safe fn URL__protocol(url: &URL) -> OwnedString; + safe fn URL__href(url: &URL) -> OwnedString; + safe fn URL__hostname(url: &URL) -> OwnedString; safe fn URL__deinit(url: &mut URL); - safe fn URL__pathname(url: &URL) -> String; - safe fn URL__getHref(input: &mut String) -> String; - safe fn URL__getFileURLString(input: &mut String) -> String; - safe fn URL__getHrefJoin(base: &mut String, relative: &mut String) -> String; - safe fn URL__fragmentIdentifier(url: &URL) -> String; + safe fn URL__pathname(url: &URL) -> OwnedString; + safe fn URL__getHref(input: &mut String) -> OwnedString; + safe fn URL__getFileURLString(input: &mut String) -> OwnedString; + safe fn URL__getHrefJoin(base: &mut String, relative: &mut String) -> OwnedString; + safe fn URL__fragmentIdentifier(url: &URL) -> OwnedString; fn URL__originLength(latin1_slice: *const u8, len: usize) -> u32; } - // The C ABI wants a mutable address. We take `&String` (matching existing call sites - // in this crate) and — since `bun_core::String: Copy` — bit-copy into a mutable - // local and pass `&mut local`. This avoids casting - // a shared-ref-derived pointer to `*mut` (read-only provenance). The C++ side - // (`BunString::toWTFString() const`) does not mutate, but the local-copy form is - // sound regardless. + // C ABI wants `*mut String`; `String: Copy` so bit-copy into a mutable local + // instead of casting a shared-ref pointer to `*mut` (read-only provenance). /// Percent-encodes the URL, punycode-encodes the hostname, and returns the normalized /// href. If parsing fails, the returned String's tag is `Dead`. - pub fn href_from_string(str: &String) -> String { + pub fn href_from_string(str: &String) -> OwnedString { let mut input = *str; URL__getHref(&mut input) } - pub fn join(base: &String, relative: &String) -> String { + pub fn join(base: &String, relative: &String) -> OwnedString { let mut base_str = *base; let mut relative_str = *relative; URL__getHrefJoin(&mut base_str, &mut relative_str) } - pub fn file_url_from_string(str: &String) -> String { + pub fn file_url_from_string(str: &String) -> OwnedString { let mut input = *str; URL__getFileURLString(&mut input) } @@ -122,27 +115,21 @@ pub mod whatwg { Self::from_string(&String::borrow_utf8(input)) } /// The URL fragment (the part after `#`), excluding the leading '#'. - pub fn fragment_identifier(&self) -> String { + pub fn fragment_identifier(&self) -> OwnedString { URL__fragmentIdentifier(self) } - pub fn protocol(&self) -> String { + pub fn protocol(&self) -> OwnedString { URL__protocol(self) } - pub fn href(&self) -> String { + pub fn href(&self) -> OwnedString { URL__href(self) } - /// Returns the host WITH the port. - /// - /// Note that this does NOT match JS `hostname`, which excludes the port (that - /// port-less form is `bun_jsc::URL::host`). - /// - /// ```text - /// URL("http://example.com:8080").hostname() => "example.com:8080" - /// ``` - pub fn hostname(&self) -> String { + /// Host WITH the port — opposite of JS `url.hostname` (https://url.spec.whatwg.org/#dom-url-hostname). + /// The port-less form is `bun_jsc::URL::host`. + pub fn hostname(&self) -> OwnedString { URL__hostname(self) } - pub fn pathname(&self) -> String { + pub fn pathname(&self) -> OwnedString { URL__pathname(self) } pub fn deinit(&mut self) { @@ -322,11 +309,9 @@ impl<'a> URL<'a> { if href.tag() == BunStringTag::Dead { return Err(crate::Error::InvalidURL); } - // `to_owned_slice` is infallible so explicit - // ordering suffices (no error path between alloc and deref). - let owned = href.to_owned_slice().into_boxed_slice(); - href.deref(); - Ok(OwnedURL { href: owned }) + Ok(OwnedURL { + href: href.to_owned_slice().into_boxed_slice(), + }) } pub fn display_protocol(&self) -> &[u8] { diff --git a/test/js/node/child_process/child-process-stdio.test.js b/test/js/node/child_process/child-process-stdio.test.js index 78b6454f9243..94f6beb6b20d 100644 --- a/test/js/node/child_process/child-process-stdio.test.js +++ b/test/js/node/child_process/child-process-stdio.test.js @@ -1,7 +1,8 @@ -import { describe, expect, it } from "bun:test"; +import { describe, expect, it, test } from "bun:test"; import { bunEnv, bunExe } from "harness"; import { execSync, spawn } from "node:child_process"; import { once } from "node:events"; +import { finished } from "node:stream/promises"; const CHILD_PROCESS_FILE = import.meta.dir + "/spawned-child.js"; const OUT_FILE = import.meta.dir + "/stdio-test-out.txt"; @@ -119,6 +120,60 @@ describe("process.stdin", () => { }); }); +// https://github.com/oven-sh/bun/pull/31833 +// Short stdio arrays are padded to length 3; the eager-load guard used to read +// the raw pre-padding length, so a 2-element array left stdout un-eagerly- +// loaded and accessing it after exit hit an assertion in native-readable. +describe("short stdio arrays", () => { + test.each([ + [["pipe", "pipe"]], + [["ignore", "pipe"]], + [["inherit", "pipe"]], + [["pipe", "pipe", "pipe"]], // 3-element control row + ])("stdio %j: stdout streams while the child runs", async stdio => { + const child = spawn(bunExe(), ["-e", "process.stdout.write('ok')"], { env: bunEnv, stdio }); + let out = ""; + child.stdout.setEncoding("utf8"); + child.stdout.on("data", d => (out += d)); + child.stderr?.resume(); + const [code, signal] = await once(child, "close"); + expect(out).toBe("ok"); + expect(child.stdout.readable).toBe(false); + expect(signal).toBeNull(); + expect(code).toBe(0); + }); + + // The regression: `.stdio` must not be touched before exit, otherwise it is + // constructed while the handle is still alive and the missing eager load is + // invisible. Without the fix the 2-element rows skip the eager load, so this + // first post-exit access constructs a native Readable over a released handle + // and throws "ASSERTION FAILED: typeof bunNativePtr === object". + // The invariant under test is that reading `.stdout` after exit is safe and + // the stream can still be driven to a clean end, not that the bytes are + // retrievable. + test.each([ + [["pipe", "pipe"]], + [["ignore", "pipe"]], + [["pipe", "pipe", "pipe"]], // 3-element control row + ])("stdio %j: stdout is a usable Readable when first accessed after exit", async stdio => { + const child = spawn(bunExe(), ["-e", "process.stdout.write('ok')"], { env: bunEnv, stdio }); + const [code, signal] = await once(child, "exit"); + expect(signal).toBeNull(); + expect(code).toBe(0); + + // First `.stdout` access of this ChildProcess' lifetime: must not throw. + const stdout = child.stdout; + expect(stdout).not.toBeNull(); + expect(typeof stdout.on).toBe("function"); + + // On Windows the pipe EOF can lag the 'exit' event, so drive the stream to + // completion instead of asserting it is already ended. + stdout.resume(); + await finished(stdout); + expect(stdout.readableEnded).toBe(true); + }); +}); + describe("child.stdin", () => { it("write() after child 'close' returns false and calls back with ERR_STREAM_DESTROYED", async () => { const child = spawn(bunExe(), ["-e", ""], { diff --git a/test/leaksan.supp b/test/leaksan.supp index 35a6c5a82213..0c9a8d90a733 100644 --- a/test/leaksan.supp +++ b/test/leaksan.supp @@ -17,6 +17,7 @@ leak:JSC::ScriptExecutable::newCodeBlockFor leak:JSC::Parser>::parseFunctionExpression leak:JSC::Parser>::parsePrimaryExpression leak:JSC::Parser>::parseStatement +leak:JSC::Parser>::parseImportDeclaration leak:JSCInitialize leak:getaddrinfo_send_reply leak:start_wqthread @@ -118,6 +119,76 @@ leak:WebCore::jsSQLStatementOpenStatementFunction # is called before firing (WaiterListManager::clearTimer on notify/unregister), # the DispatchTimer and its Bun-side WTFTimer Box leak. JSC-owned ref-cycle. leak:WTF::RunLoop::dispatchAfter +# test/js/node/test/parallel/test-worker-terminate-http2-respond-with-file.js +# Live-thread TLS at exit: ParkingLot ThreadData / RunLoop holder of the vm +# watchdog/aux threads still parked when the process exits. +leak:WTF::ParkingLot::parkConditionallyImpl +leak:WTF::RunLoop::currentSingleton +# test/js/node/test/parallel/test-require-builtins.js +# Parser-arena identifiers pinned in the atom table at VM-destroy exit — +# covers all JSC::Parser parse productions (same family as the parse* entries above). +leak:JSC::IdentifierArena::makeIdentifier +# test/js/node/test/parallel/test-require-builtins.js +# ASCIILiteral StringImpl wrapper for internal module names; pinned for process lifetime. +leak:Bun::InternalModuleRegistry::createInternalModuleById +# test/js/node/test/parallel/test-net-dns-lookup.js +# macOS libdispatch/XPC continuation cached inside dns_configuration_free while +# c-ares reads the system resolver config — OS-internal, not reachable by us. +leak:ares_init_sysconfig_macos +# test/js/node/test/parallel/test-fs-watch.js +# FSEvents watcher thread (std::thread spawn block) still running at exit. +leak:FSEventsLoop +# test/js/node/test/parallel/test-tls-connect-simple.js +# Apple CoreAnalytics XPC telemetry triggered inside SecTrustCopyAnchorCertificates / +# system CA reads — OS-internal dispatch continuation. +leak:CoreAnalytics +# test/js/node/test/parallel/test-assert-checktag.js +# ASCIILiteral StringImpl wrapper created while formatting a stack frame's source +# URL on the exit path; same class as the InternalModuleRegistry entry above. +leak:Zig::sourceURL +# test/js/node/test/parallel/test-tls-connect-simple.js +# Apple Security.framework keychain internals reached from our run_once system +# root-CA load — cached for process lifetime by design. +leak:us_get_root_system_cert_instances +# test/js/node/test/parallel/test-shadow-realm-gc.js +# JSC structure-heap bookkeeping (BitVector in StructureMemoryManager); grows +# once per structure block and lives for the VM's lifetime. +leak:JSC::StructureMemoryManager::tryMallocStructureBlock +# test/js/node/test/parallel/test-tls-connect-simple.js +# libsystem_info per-thread user-info cache (getpwuid via CFPreferences inside +# Security.framework) — OS-internal thread-local storage. +leak:LI_get_thread_info +# test/js/node/test/parallel/test-child-process-stdio-inherit.js +# backtrace_symbols() buffer malloc'd inside debug-only stack-trace dumps +# (fd-UAF warning path); diagnostics memory, never freed by design. +leak:backtrace_symbols +# test/js/node/test/parallel/test-require-builtins.js +# Per-VM JSON atom cache entry pinned in the atom table at VM-destroy exit +# (same family as IdentifierArena::makeIdentifier above). +leak:JSC::JSONAtomStringCache +# test/js/node/test/parallel/test-inspector-enabled.js +# Inspector/debugger server thread still parked at exit — live-thread +# allocation. Narrowed to the `Debugger` struct's inherent methods so +# `AsyncTaskTracker`/`TestReporterAgent`/per-timer paths stay observable. +leak:7bun_jsc8debugger8Debugger +# test/js/node/test/parallel/test-worker-message-port.js +# Per-worker WebCore::EventNames not reclaimed when a Worker thread exits — +# bounded by live worker count at exit; needs a ThreadGlobalData teardown +# follow-up rather than blocking every worker test locally. +leak:WebCore::EventNames::operator new +# test/js/node/test/parallel/test-http-agent-keepalive.js +# lol_html's compiled-selector storage (`selectors_vm`) reached via +# HTMLRewriter — process-lifetime once compiled. Narrowed from `9selectors` +# so `bun_css::selectors` (Bun's own CSS parser) stays observable. +leak:8lol_html +# test/js/node/test/parallel/test-crypto-subtle-zero-length.js +# crypto.subtle lazy property: SubtleCrypto impl pinned by its JS wrapper at +# VM-destroy exit. Same JSC-owned ref-cycle class as RunLoop::dispatchAfter. +leak:WebCore::SubtleCrypto::create +# test/js/node/test/parallel/test-require-builtins.js +# Rust std lazily-allocated pthread mutex storage (sys::sync::once_box) — +# intentionally never freed; one block per static mutex. +leak:8once_box # test/cli/run/workspaces.test.ts, test/regression/issue/26207.test.ts — # run_scripts_with_filter ends with Global::exit() so pre_script_name / # post_script_name / script_name_owned locals never Drop (intentional per the