From b5d1e415ba0d1c14600cb02aefa29cd91e2e9445 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 08:44:12 +0000 Subject: [PATCH 01/25] Never hand one StringImpl to two threads' atom tables WTF::StringImpl refcounts are atomic, but two operations are per-thread: `Identifier::fromString` atomizes a non-atom impl *in place* into the current thread's AtomStringTable, and the final `deref()` of an atom removes it from the current thread's table (RELEASE_ASSERT if it is not there). So an impl that two VMs can both reach is unsafe even if nobody drops it off-thread: one VM atomizes it, the other uses a foreign atom as a property key (lookups by the same text miss) and whichever thread drops it last aborts. Four places did this: - `URL.createObjectURL`: the process-global registry stored the Blob's `name` impl (a JS string from the creating worker) and `global_this`, and handed clones of that impl to every VM that resolved the URL. The entry now keeps the name as bytes and a null global; each resolve gets its own impl and the resolving VM's global. - structured clone's dense-array fast path stored property-name impls with `isolatedCopy()` and every receiver atomized them in place; with BroadcastChannel one message has many receivers. Keys are now stored with `isolatedCopyForSharing` (pre-hashed, never-atomize) in all three object fast paths, so each receiver's atom table takes a copy; the simple-object receive path drops its extra re-copy. - `bun build --compile`: `Bun.embeddedFiles`/`Bun.file(embedded)` duped one cached Blob (name impl + first VM's global) into every Worker, and text modules / module source shared one cached impl per file across VMs. The cache now holds only the immortal store; name/global are per VM, and `to_wtf_string` returns a fresh zero-copy external impl per call. - `fetch({ unix })` bit-copied a possibly JS-backed `Utf8Bytes` onto the HTTP thread; the tasklet now owns the bytes and the client borrows them. Worker `Bun.$` argv copies the parent's option strings like `process.argv` already does. Bundler plugin namespace strings are isolated before the bundle thread reads them. `isThreadSafe()` additionally rejects substring and external impls, and the async-fs path that keeps a JS string's own 8-bit impl marks it never-atomize + pre-hashed so the JS thread cannot later atomize it in place while the work pool holds it. --- src/bun_alloc/lib.rs | 7 + src/bun_core/string/mod.rs | 14 +- src/http/AsyncHTTP.rs | 24 ++- src/http/HTTPThread.rs | 11 +- src/http/lib.rs | 34 ++-- src/http/session_cache.rs | 2 +- src/jsc/bindings/BunString.cpp | 20 +-- src/jsc/bindings/BunString.h | 3 + src/jsc/bindings/JSBundlerPlugin.cpp | 4 +- .../webcore/SerializedScriptValue.cpp | 11 +- src/jsc/webcore_types.rs | 10 +- src/runtime/api/BunObject.rs | 19 +-- src/runtime/api/standalone_graph_jsc.rs | 156 ++++++++---------- src/runtime/jsc_hooks.rs | 29 ++-- src/runtime/shell/interpreter.rs | 3 +- src/runtime/webcore/Blob.rs | 18 +- src/runtime/webcore/ObjectURLRegistry.rs | 35 +++- src/runtime/webcore/fetch.rs | 13 +- src/runtime/webcore/fetch/FetchTasklet.rs | 41 ++--- src/standalone_graph/StandaloneModuleGraph.rs | 92 +++++------ test/bundler/bundler_compile.test.ts | 26 +++ .../broadcast-channel.test.ts | 34 ++++ test/js/web/workers/worker_blob.test.ts | 51 ++++++ 23 files changed, 371 insertions(+), 286 deletions(-) diff --git a/src/bun_alloc/lib.rs b/src/bun_alloc/lib.rs index fb9589fe5408..1f74f44ad552 100644 --- a/src/bun_alloc/lib.rs +++ b/src/bun_alloc/lib.rs @@ -934,6 +934,12 @@ impl WTFStringImplStruct { pub fn ensure_hash(&self) { Bun__WTFStringImpl__ensureHash(self); } + /// After this, using the impl as a property key copies it into the atom + /// table instead of atomizing it in place. + #[inline] + pub fn set_never_atomize(&self) { + WTFStringImpl__setNeverAtomize(self); + } } unsafe extern "C" { @@ -948,6 +954,7 @@ unsafe extern "C" { pub fn Bun__WTFStringImpl__destroy(this: *const WTFStringImplStruct); safe fn WTFStringImpl__isThreadSafe(this: &WTFStringImplStruct) -> bool; safe fn Bun__WTFStringImpl__ensureHash(this: &WTFStringImplStruct); + safe fn WTFStringImpl__setNeverAtomize(this: &WTFStringImplStruct); } // ────────────────────────────────────────────────────────────────────────── diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index ab1ce29732bf..6bcb57da754d 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -507,16 +507,18 @@ impl String { core::ptr::null_mut() } } + /// Replace a WTF-backed impl with an isolated copy, for handing the value + /// to one other thread; not for sharing one impl between VMs. pub fn to_thread_safe(&mut self) { if self.tag == Tag::WTFStringImpl { BunString__toThreadSafe(self) } debug_assert!(self.is_thread_safe()); } - /// True iff this `String` may be sent to / shared with another thread - /// without racing the WTF `StringImpl`'s non-atomic refcount: every tag - /// except `WTFStringImpl` is inert (raw slice / static / dead), and a - /// WTF-backed string is safe iff its impl reports `isThreadSafe()`. + /// True iff this `String` may be handed to another thread as-is: every + /// tag except `WTFStringImpl` is inert (raw slice / static / dead), and a + /// WTF-backed string qualifies iff its impl owns its buffer and is in no + /// atom table (`isThreadSafe()`). /// /// Call sites that move a `String` across a thread boundary must ensure /// this holds (typically by calling [`to_thread_safe`] first); see the @@ -918,6 +920,10 @@ impl String { if out.utf8.is_none() { // 8-bit all-ASCII: a thread-safe `string` keeps backing the bytes. if out.string.is_thread_safe() { + let wtf = out.string.as_wtf(); + wtf.ensure_hash(); + // The JS thread still holds this impl; a later property-key use there must copy, not atomize in place. + wtf.set_never_atomize(); return out; } out.utf8 = Some(out.slice().to_vec()); diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index 42ba805f127b..a437cf013a08 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -3,7 +3,7 @@ use core::sync::atomic::{AtomicUsize, Ordering}; use bun_ast::{Loc, Log}; use bun_core::FeatureFlags; -use bun_core::{MutableString, Utf8Bytes}; +use bun_core::MutableString; use bun_threading::IntrusiveWorkTask as _; use bun_threading::thread_pool::{self, Batch, Task}; use bun_url::{PercentEncoding, URL}; @@ -24,9 +24,9 @@ bun_core::declare_scope!(AsyncHTTP, visible); // Lifetime `'a` covers every borrowed input the caller hands in: `url`, // `http_proxy`, `request_header_buf`, the borrowed `HTTPRequestBody::Bytes` -// payload, and `client.{header_buf,hostname,if_modified_since}`. Intrusive -// fields (`real`, `next`) are raw pointers and thus lifetime-erased; the -// HTTP-thread copy uses the same `'a` as the JS-thread original it mirrors. +// payload, and `client.{header_buf,hostname,unix_socket_path,if_modified_since}`. +// Intrusive fields (`real`, `next`) are raw pointers and thus lifetime-erased; +// the HTTP-thread copy uses the same `'a` as the JS-thread original it mirrors. pub struct AsyncHTTP<'a> { pub response: Option>, pub request_headers: headers::EntryList, @@ -191,7 +191,7 @@ fn make_client<'a>( signals, async_http_id, hostname, - unix_socket_path: Utf8Bytes::EMPTY, + unix_socket_path: b"", compress: None, compressed_request_body: Vec::new(), compressed_body_len: 0, @@ -245,7 +245,7 @@ pub struct Options<'a> { pub proxy_headers: Option, pub hostname: Option<&'a [u8]>, pub signals: Option, - pub unix_socket_path: Option>, + pub unix_socket_path: Option<&'a [u8]>, pub disable_timeout: Option, /// Per-request idle timeout override in seconds; see /// `HTTPClient::idle_timeout_seconds`. @@ -315,7 +315,6 @@ impl<'a> AsyncHTTP<'a> { pub fn clear_data(&mut self) { self.response = None; - self.client.unix_socket_path = Utf8Bytes::EMPTY; } } @@ -461,7 +460,6 @@ impl<'a> AsyncHTTP<'a> { signals, }; if let Some(val) = options.unix_socket_path { - debug_assert!(this.client.unix_socket_path.slice().is_empty()); this.client.unix_socket_path = val; } if let Some(val) = options.disable_timeout { @@ -711,11 +709,11 @@ impl<'a> AsyncHTTP<'a> { // (`start_queued_task`), so any owned field that was already // populated at that point — `request_headers`, // `client.header_entries`, `client.proxy_headers`, - // `client.proxy_settings`, `client.tls_props`, - // `client.unix_socket_path` — is *shared* with the original - // and must NOT be dropped here; the original drops them when - // its `Box` is reclaimed. Only the state the clone - // built up itself during request processing is torn down. + // `client.proxy_settings`, `client.tls_props` — is *shared* + // with the original and must NOT be dropped here; the original + // drops them when its `Box` is reclaimed. Only the + // state the clone built up itself during request processing is + // torn down. { // `handle_response_metadata` rewrites per-hop request state in // place: `client.url`/`connected_url` become self-borrows into diff --git a/src/http/HTTPThread.rs b/src/http/HTTPThread.rs index 912f7d4b903d..1077b992fbbf 100644 --- a/src/http/HTTPThread.rs +++ b/src/http/HTTPThread.rs @@ -409,16 +409,9 @@ impl HttpThread { // funnels through here before touching `https_context.{group,secure}`. self.ensure_https_context_init(); } - // Note: borrowck — `slice()` borrows `client`; capture into a - // `bun_ptr::RawSlice` (encapsulated outlives-holder invariant) so the - // borrow of `client` ends before we hand `&mut client` to - // `connect_socket`. Backing storage is `client.unix_socket_path`, which - // `connect_socket` does not touch. - let unix_path = bun_ptr::RawSlice::new(client.unix_socket_path.slice()); + let unix_path = client.unix_socket_path; if !unix_path.is_empty() { - return self - .context::() - .connect_socket(client, unix_path.slice()); + return self.context::().connect_socket(client, unix_path); } if IS_SSL { diff --git a/src/http/lib.rs b/src/http/lib.rs index 63c12ffaba2a..82f4948b92df 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -648,7 +648,6 @@ pub(crate) fn hash_header_name(name: &[u8]) -> u64 { // `bun_uws::NewSocketHandler` methods (`ext`/`timeout`/`raw_write`/`flush`/ // `shutdown`/`connect_group`/…) land. -use bun_core::Utf8Bytes; use bun_url::URL; use core::ptr::NonNull; @@ -791,8 +790,8 @@ fn no_proxy_matches(no_proxy_text: &[u8], hostname: &[u8], host: &[u8]) -> bool // Many of these fields can be moved to a packed struct and use less space // // Lifetime `'a` ties every borrowed input — `url`, `http_proxy`, `header_buf`, -// `if_modified_since`, `hostname`, and the borrowed `HTTPRequestBody::Bytes` -// payload — to the caller's storage. The original port erased these to `'static` +// `if_modified_since`, `hostname`, `unix_socket_path`, and the borrowed +// `HTTPRequestBody::Bytes` payload — to the caller's storage. The original port erased these to `'static` // and lifetime-erased at every call site; threading the lifetime removes that hazard. // Intrusive raw-pointer backrefs (socket ext, h2/h3 streams) store the // lifetime-erased `HTTPClient<'static>` form via [`HTTPClient::as_erased_ptr`]. @@ -870,7 +869,7 @@ pub struct HTTPClient<'a> { pub(crate) signals: Signals, pub(crate) async_http_id: u32, pub(crate) hostname: Option<&'a [u8]>, - pub(crate) unix_socket_path: Utf8Bytes<'static>, + pub(crate) unix_socket_path: &'a [u8], /// `fetch({ compress })` — when set, the body is compressed lazily at /// write time (h1: `send_initial_request_payload`; h2/h3: at attach) so /// the output can borrow `LibdeflateState::shared_buffer`. Persists across @@ -1943,7 +1942,7 @@ impl<'a> HTTPClient<'a> { // through a non-keepalive Agent or `agent: false` skip it, matching Node. // // TCP options do not apply to a unix socket. - if !self.flags.disable_keepalive && self.unix_socket_path.slice().is_empty() { + if !self.flags.disable_keepalive && self.unix_socket_path.is_empty() { let _ = socket.set_keep_alive(true, 60); } @@ -1971,7 +1970,7 @@ impl<'a> HTTPClient<'a> { if self.flags.is_preconnect_only { return false; } - if self.unix_socket_path.slice().len() > 0 { + if !self.unix_socket_path.is_empty() { return false; } if matches!( @@ -2020,7 +2019,7 @@ impl<'a> HTTPClient<'a> { if self.flags.is_preconnect_only { return false; } - if self.unix_socket_path.slice().len() > 0 { + if !self.unix_socket_path.is_empty() { return false; } if matches!( @@ -2259,7 +2258,7 @@ impl<'a> HTTPClient<'a> { pub(crate) fn is_keep_alive_possible(&self) -> bool { if FeatureFlags::ENABLE_KEEPALIVE { // TODO keepalive for unix sockets - if self.unix_socket_path.slice().len() > 0 { + if !self.unix_socket_path.is_empty() { return false; } // check state @@ -2633,15 +2632,9 @@ impl<'a> HTTPClient<'a> { self.flags.is_streaming_request_body = false; } - // Decided before unix_socket_path is forgotten below: a unix-socket connection must not be pooled. + // Decided before unix_socket_path is cleared below: a unix-socket connection must not be pooled. let keep_alive_possible = self.is_keep_alive_possible(); - // There is no struct copy-back - // (`sync_progress_from` skips owned fields) and the original retains - // its own `Owned(Vec)` aliasing the same allocation (the HTTP-thread - // clone was created via `ptr::read`). Dropping it here would - // double-free when the original later runs `clear_data()`. Forget the - // clone's view; the original is the sole owner. - let _ = core::mem::ManuallyDrop::new(core::mem::take(&mut self.unix_socket_path)); + self.unix_socket_path = b""; // TODO: what we do with stream body? let request_body: &[u8] = if self.state.flags.resend_request_body_on_redirect && matches!(self.state.original_request_body, HTTPRequestBody::Bytes(_)) @@ -2863,7 +2856,7 @@ impl<'a> HTTPClient<'a> { self.complete_connecting_process(); return; } - if self.http_proxy.is_some() || self.unix_socket_path.slice().len() > 0 { + if self.http_proxy.is_some() || !self.unix_socket_path.is_empty() { self.fail(crate::Error::HTTP3Unsupported); self.complete_connecting_process(); return; @@ -4357,10 +4350,7 @@ impl<'a> HTTPClient<'a> { if matches!(self.state.original_request_body, HTTPRequestBody::Stream(_)) { self.flags.is_streaming_request_body = false; } - // See `do_redirect`: the HTTP-thread clone shares this allocation - // with the JS-thread original (created via `ptr::read`); dropping it - // here double-frees once the original runs `clear_data()`. - let _ = core::mem::ManuallyDrop::new(core::mem::take(&mut self.unix_socket_path)); + self.unix_socket_path = b""; let request_body: &[u8] = if self.state.flags.resend_request_body_on_redirect && matches!(self.state.original_request_body, HTTPRequestBody::Bytes(_)) { @@ -4953,7 +4943,7 @@ impl<'a> HTTPClient<'a> { // request to the same origin may be h3-eligible even if this // one was pinned/proxied/sendfile. if self.is_https() - && self.unix_socket_path.slice().len() == 0 + && self.unix_socket_path.is_empty() && !(self.flags.proxy_tunneling && self.proxy_tunnel.is_none()) && h3_alt_svc_enabled() { diff --git a/src/http/session_cache.rs b/src/http/session_cache.rs index a1ba85bfc941..0dd7fe0494dc 100644 --- a/src/http/session_cache.rs +++ b/src/http/session_cache.rs @@ -182,7 +182,7 @@ unsafe extern "C" { pub(crate) fn eligible(client: &crate::HTTPClient<'_>) -> bool { client.flags.reject_unauthorized && !client.signals.get(signals::Field::CertErrors) - && client.unix_socket_path.slice().is_empty() + && client.unix_socket_path.is_empty() && !bun_core::env_var::feature_flag::BUN_FEATURE_FLAG_DISABLE_FETCH_TLS_SESSION_CACHE .get() .unwrap_or(false) diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index 9ffd53a1ca4e..a7c97799a74f 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -296,7 +296,7 @@ static constexpr unsigned int kMinCrossThreadShareableLength = 256; // the receivers race the lazy m_hashAndFlags update (debug: ASSERT(!hasHash()) // in setHash; e.g. two workers switch()ing on the same BroadcastChannel // message). Static strings are immortal, pre-hashed and safe to share as-is. -static Ref isolatedCopyForSharing(WTF::StringImpl& impl) +WTF::String isolatedCopyForSharing(WTF::StringImpl& impl) { Ref copy = impl.isolatedCopy(); if (!copy->isStatic()) { @@ -793,18 +793,12 @@ extern "C" BunString BunString__createExternalGloballyAllocatedUTF16( return { BunStringTag::WTFStringImpl, { .wtf = &impl.leakRef() } }; } +// True iff the impl owns its buffer outright and is in no thread's atom +// table, so another thread may hold a ref (after ensureHash + setNeverAtomize). extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadSafe( const WTF::StringImpl* wtf) { - if (wtf->isSymbol()) - return false; - - if (wtf->isAtom()) { - // AtomString destructor would destruct on the wrong string table. - return false; - } - - return true; + return !wtf->isAtom() && !wtf->isSymbol() && wtf->bufferOwnership() != WTF::StringImpl::BufferSubstring && !wtf->isExternal(); } extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__WTFStringImpl__ensureHash(WTF::StringImpl* str) @@ -812,6 +806,12 @@ extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__WTFStringImpl__ensureHash(WTF::Stri str->hash(); } +extern "C" [[ZIG_EXPORT(nothrow)]] void WTFStringImpl__setNeverAtomize(WTF::StringImpl* str) +{ + if (!str->isStatic()) + str->setNeverAtomize(); +} + extern "C" JSC::EncodedJSValue JSC__JSValue__upsertBunStringArray( JSC::EncodedJSValue encodedTarget, JSC::JSGlobalObject* global, diff --git a/src/jsc/bindings/BunString.h b/src/jsc/bindings/BunString.h index 8f81f90b66f4..4a3ac65449d4 100644 --- a/src/jsc/bindings/BunString.h +++ b/src/jsc/bindings/BunString.h @@ -57,5 +57,8 @@ class UTF8View { }; WTF::String toCrossThreadShareable(const WTF::String& string); +// Always copies; the copy is pre-hashed and never atomized in place, so any +// number of threads may hold it and use it as a property key. +WTF::String isolatedCopyForSharing(WTF::StringImpl&); } diff --git a/src/jsc/bindings/JSBundlerPlugin.cpp b/src/jsc/bindings/JSBundlerPlugin.cpp index 074979b915ba..ca57cc2450dc 100644 --- a/src/jsc/bindings/JSBundlerPlugin.cpp +++ b/src/jsc/bindings/JSBundlerPlugin.cpp @@ -65,7 +65,7 @@ void BundlerPlugin::NamespaceList::append(JSC::VM& vm, JSC::RegExp* filter, Stri auto* nsGroup = group(namespaceString, index); if (nsGroup == nullptr) { - namespaces.append(namespaceString); + namespaces.append(namespaceString.isolatedCopy()); groups.append(Vector {}); nsGroup = &groups.last(); index = namespaces.size() - 1; @@ -258,7 +258,7 @@ void BundlerPlugin::NativePluginList::append(JSC::VM& vm, JSC::RegExp* filter, S auto* nsGroup = group(namespaceString, index); if (nsGroup == nullptr) { - namespaces.append(namespaceString); + namespaces.append(namespaceString.isolatedCopy()); groups.append(Vector {}); nsGroup = &groups.last(); index = namespaces.size() - 1; diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index 8979fb8dfe82..cc5721d711be 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/src/jsc/bindings/webcore/SerializedScriptValue.cpp @@ -4611,10 +4611,10 @@ ExceptionOr> SerializedScriptValue::create(JSGlobalOb objOk = false; return false; } - properties.append({ entry.key()->isolatedCopy(), + properties.append({ Bun::isolatedCopyForSharing(*entry.key()), Bun::toCrossThreadShareable(stringValue) }); } else { - properties.append({ entry.key()->isolatedCopy(), propValue }); + properties.append({ Bun::isolatedCopyForSharing(*entry.key()), propValue }); } return true; }); @@ -4693,10 +4693,10 @@ ExceptionOr> SerializedScriptValue::create(JSGlobalOb canUseObjectFastPath = false; return false; } - properties.append({ entry.key()->isolatedCopy(), Bun::toCrossThreadShareable(stringValue) }); + properties.append({ Bun::isolatedCopyForSharing(*entry.key()), Bun::toCrossThreadShareable(stringValue) }); } else { // Primitive values are safe to share across threads. - properties.append({ entry.key()->isolatedCopy(), value }); + properties.append({ Bun::isolatedCopyForSharing(*entry.key()), value }); } return true; @@ -4928,8 +4928,7 @@ JSValue SerializedScriptValue::deserialize(JSGlobalObject& lexicalGlobalObject, } for (const auto& property : m_simpleInMemoryPropertyTable) { - // We **must** clone this so that the atomic flag doesn't get set to true. - JSC::Identifier identifier = JSC::Identifier::fromString(vm, property.propertyName.isolatedCopy()); + JSC::Identifier identifier = JSC::Identifier::fromString(vm, property.propertyName); JSValue value = std::visit( WTF::makeVisitor( [](JSValue value) -> JSValue { return value; }, diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index 42e46b8fd872..d6e73f9d1a2c 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -142,10 +142,12 @@ pub struct Blob { pub name: bun_ptr::JsCell, } -// SAFETY: `Blob` holds a raw `global_this` pointer which defaults to -// `!Send`/`!Sync`. `Blob` moves across threads under `ObjectURLRegistry`'s -// mutex and via the work-pool read/write tasks; `global_this` is an opaque -// JSC handle only ever dereferenced on its owning JS thread. +// SAFETY: a `Blob` reaches another thread only as an `ObjectURLRegistry` +// entry or standalone-graph template (both with null `global_this` and a +// `DEAD` `name`; every dupe handed out is stamped with the resolving thread's +// global and a fresh name impl) or via the work-pool read/write tasks that +// hand it back to the owning JS thread. `store` (`StoreRef`) and +// `content_type` are atomically refcounted. unsafe impl Send for Blob {} // SAFETY: concurrent `&Blob` access only occurs under `ObjectURLRegistry`'s // mutex or on the single owning JS thread; the `Cell` fields are never raced. diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index e2bbe855177b..3f2bd409fcc1 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1889,15 +1889,10 @@ fn get_embedded_files(global_this: &JSGlobalObject, _: &JSObject) -> JsResult = Vec::with_capacity(unsorted_files.len()); for (index, file) in unsorted_files.iter().enumerate() { // Some % of people using `bun build --compile` want to obscure the source code @@ -1922,14 +1917,10 @@ fn get_embedded_files(global_this: &JSGlobalObject, _: &JSObject) -> JsResult &mut Blob; + /// A fresh `Blob` over the shared immortal store, owned by `global`'s VM. + fn file_blob(&self, global: &JSGlobalObject) -> Blob; } -impl FileJsc for File { - fn file_blob(&mut self, global: &JSGlobalObject) -> &mut Blob { - if self.cached_blob.is_none() { - // `contents` is a `'static` slice into the embedded executable - // section — borrow it directly (no copy) and hand it to a `Bytes` - // store with the default allocator. The leaked extra `ref_()` below - // pins the refcount ≥ 1 forever, so `Store::deref` never runs and - // the (otherwise UB) free of a static slice is unreachable. - let contents = self.contents.as_bytes(); - // SAFETY: `contents` is `'static` and never freed (see above); - // the const-cast is sound because Blob consumers only read via - // `shared_view()`. - let bytes = unsafe { - Bytes::from_raw_parts( - contents.as_ptr().cast_mut(), - contents.len() as SizeType, - contents.len() as SizeType, - bun_alloc::basic::C_ALLOCATOR, - ) - }; - // Cannot use `..Default::default()` — `Store: Drop` - // forbids partial moves out of the temporary default. - let store = StoreRef::from(Store::new(Store { - data: Data::Bytes(bytes), - mime_type: MimeType::NONE, - ref_count: bun_ptr::ThreadSafeRefCount::init(), - is_all_ascii: None, - })); - // make it never free - store.ref_(); - - // Hold the raw pointer so we can keep mutating the store after - // `init_with_store` consumes the `StoreRef`. The store outlives - // this fn (leaked above). - let store_ptr = store.as_ptr(); - - let b = Blob::init_with_store(store, global); - - if let Some(mime) = MimeType::by_extension_no_default(strings::trim_leading_char( - bun_paths::extension(self.name), - b'.', - )) { - // SAFETY: `store_ptr` is the sole live mutable view; held ref - // guarantees liveness for the process lifetime. - let store = unsafe { &mut *store_ptr }; - b.content_type - .set(crate::webcore::blob::BlobContentType::from_mime(&mime)); - b.content_type_was_set.set(true); - store.mime_type = mime; - } - - // The real name goes here: - // SAFETY: see above; `data` is `Bytes` by construction. - if let Data::Bytes(bytes) = unsafe { &mut (*store_ptr).data } { - // `Bytes::Drop` and `jsdom_file_construct` both require - // `stored_name` to be heap-owned (or empty); a borrowed - // `'static` slice would be invalid-freed there. - bytes.stored_name = self.name.to_vec().into_boxed_slice(); - } - - // The pretty name goes here: - let prefix = StandaloneModuleGraph::BASE_PUBLIC_PATH_WITH_DEFAULT_SUFFIX.as_bytes(); - if self.name.starts_with(prefix) { - b.name - .set(bstring::String::clone_utf8(&self.name[prefix.len()..])); - } else if !self.name.is_empty() { - b.name.set(bstring::String::clone_utf8(self.name)); - } - - // Heap-promote and stash the raw pointer. The standalone graph (and thus this Blob) lives for the process. - // `cached_blob` is typed against the lower crate's opaque `Blob` - // newtype (it cannot name `webcore::Blob` without a dep cycle), so - // erase via `.cast()` here and back below. - self.cached_blob = Some( - NonNull::new(Blob::new(b)) - .expect("Blob::new returned null") - .cast(), - ); +/// The process-wide part of an embedded file's `Blob`: store + content type. +/// `global_this` is null and `name` is `DEAD`; `file_blob` stamps those per VM. +fn shared_blob(file: &File) -> NonNull { + *file.cached_blob.get_or_init(|| { + // `contents` is a `'static` slice into the embedded executable + // section — borrow it directly (no copy) and hand it to a `Bytes` + // store with the default allocator. The leaked extra `ref_()` below + // pins the refcount ≥ 1 forever, so `Store::deref` never runs and + // the (otherwise UB) free of a static slice is unreachable. + let contents = file.contents.as_bytes(); + // SAFETY: `contents` is `'static` and never freed (see above); + // the const-cast is sound because Blob consumers only read via + // `shared_view()`. + let bytes = unsafe { + Bytes::from_raw_parts( + contents.as_ptr().cast_mut(), + contents.len() as SizeType, + contents.len() as SizeType, + bun_alloc::basic::C_ALLOCATOR, + ) + }; + let mut store = Store { + data: Data::Bytes(bytes), + mime_type: MimeType::NONE, + ref_count: bun_ptr::ThreadSafeRefCount::init(), + is_all_ascii: None, + }; + let blob = Blob::default(); + if let Some(mime) = MimeType::by_extension_no_default(strings::trim_leading_char( + bun_paths::extension(file.name), + b'.', + )) { + blob.content_type + .set(crate::webcore::blob::BlobContentType::from_mime(&mime)); + blob.content_type_was_set.set(true); + store.mime_type = mime; + } + if let Data::Bytes(bytes) = &mut store.data { + // `Bytes::Drop` and `jsdom_file_construct` both require + // `stored_name` to be heap-owned (or empty); a borrowed + // `'static` slice would be invalid-freed there. + bytes.stored_name = file.name.to_vec().into_boxed_slice(); } + let store = StoreRef::from(Store::new(store)); + // make it never free + store.ref_(); + blob.size.set(store.size()); + blob.store.set(Some(store)); + // `cached_blob` is typed against the lower crate's opaque `Blob` + // (it cannot name `webcore::Blob` without a dep cycle). + NonNull::new(Blob::new(blob)) + .expect("Blob::new returned null") + .cast() + }) +} - // SAFETY: populated above; pointer originates from `Blob::new` and is - // never freed for the graph's lifetime (store is intentionally leaked - // via `.ref_()`). Cast restores the real `webcore::Blob` type. - unsafe { self.cached_blob.unwrap().cast::().as_mut() } +impl FileJsc for File { + fn file_blob(&self, global: &JSGlobalObject) -> Blob { + // SAFETY: `shared_blob` returns the pointer from `Blob::new`, never + // freed for the process lifetime; only read here. + let blob = + unsafe { shared_blob(self).cast::().as_ref() }.dupe_with_content_type(true); + blob.global_this.set(global); + let name = self.display_name(); + if !name.is_empty() { + blob.name.set(bstring::String::clone_utf8(name)); + } + blob } } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index b4e57a7ae689..aecc7d6c034d 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1390,11 +1390,10 @@ unsafe fn process_exit(global: *mut JSGlobalObject, code: u8) { /// `vm.standalone_module_graph` here and cast it to `&mut Graph` — that /// shared-ref provenance has no write permission, so the resulting `&mut` is /// instant UB under Stacked Borrows regardless of `INIT_LOCK`. `Graph::get()` -/// hands out the `*mut` directly from the backing `UnsafeCell`, which is the -/// same path every other mutating caller (`node_fs`, `Blob`) uses. +/// hands out the `*mut` directly from the backing `UnsafeCell`. /// -/// Called on the JS thread; `Graph::find` / `LazySourceMap::load` only mutate -/// the per-`File` lazy caches (sourcemap decode is serialized by `INIT_LOCK`). +/// `LazySourceMap::load` is the graph's only post-init mutation and is +/// serialized by `INIT_LOCK`. fn load_standalone_sourcemap( path: &[u8], ) -> Option> { @@ -1479,7 +1478,7 @@ mod vm_loader_ctx { is_blob_url(spec) => crate::webcore::object_url_registry::is_blob_url(spec), resolve_blob(spec) => { crate::webcore::object_url_registry::ObjectURLRegistry::singleton() - .resolve_and_dupe(spec) + .resolve_and_dupe(spec, &*(*this).global) .map(|b| bun_core::heap::into_raw(Box::new(b)).cast::<()>()) }, blob_loader(b) => blob(b).get_loader(&*this), @@ -3749,20 +3748,13 @@ fn __bun_fetch_builtin_module( } // ── Standalone-module-graph probe ─────────────────────────────────── - // The VM field is the resolver's - // read-only `&dyn StandaloneModuleGraph`; for `File::to_wtf_string` - // (mutates the lazy `wtf_string` cache) we need write provenance, so - // reach the concrete `Graph` via its `UnsafeCell` singleton accessor — - // same path as `load_standalone_sourcemap` / `node_fs`. + // The VM field is the resolver's type-erased `&dyn StandaloneModuleGraph`; + // the concrete `Graph` is the sole implementor. if jsc_vm.standalone_module_graph.is_some() { - let graph = bun_standalone_graph::Graph::get() + let graph = bun_standalone_graph::Graph::get_ref() .expect("vm.standalone_module_graph set ⇔ Graph singleton populated"); - // Spec uses `graph.files.getPtr(spec)` (no virtual-root prefix - // check). SAFETY: `graph` is the `UnsafeCell::get()` pointer to the - // process-lifetime singleton; this hook runs on the JS thread and - // the only mutation below (`to_wtf_string`) is the per-`File` - // idempotent `wtf_string` cache. - if let Some(file) = unsafe { (*graph).files.get_mut(spec) } { + // `files.get` (no virtual-root prefix check), not `find_ref`. + if let Some(file) = graph.files.get(spec) { use bun_standalone_graph::StandaloneModuleGraph::ModuleFormat; if matches!(file.loader, Loader::Sqlite | Loader::SqliteEmbedded) { @@ -3996,7 +3988,8 @@ unsafe fn get_loader_and_virtual_source<'a>( // `blob:` ObjectURL → in-memory virtual source. if crate::webcore::object_url_registry::is_blob_url(specifier) { match crate::webcore::object_url_registry::ObjectURLRegistry::singleton() - .resolve_and_dupe(&specifier[b"blob:".len()..]) + // SAFETY: per fn contract — `jsc_vm` is the live per-thread VM. + .resolve_and_dupe(&specifier[b"blob:".len()..], unsafe { &*jsc_vm }.global()) { Some(blob) => { *blob_to_deinit = Some(blob); diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index f8cb92707506..3cdf1eba50c1 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -1623,7 +1623,8 @@ impl Interpreter { if vm_args_utf8.len() != argv.len() { vm_args_utf8.reserve(argv.len()); for arg in argv { - vm_args_utf8.push(bun_core::String::retain_wtf_impl(*arg).into_utf8()); + vm_args_utf8 + .push(crate::node::process::worker_option_string(*arg).into_utf8()); } } out.extend_from_slice(vm_args_utf8[int as usize].slice()); diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 350724b57ce3..53059b43cc0b 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3548,20 +3548,14 @@ impl BlobExt for Blob { // SAFETY: bun_vm() is live for the duration of a host call. if global_this.bun_vm().standalone_module_graph.is_some() { - // `vm.standalone_module_graph` is a - // type-erased `&dyn` so `bun_jsc` doesn't depend on - // `bun_standalone_graph`. The concrete `Graph` is the sole - // implementor and lives in a process-lifetime `OnceLock`; - // `find()` mutates the lazy `wtf_string` cache, so reach it - // via the `UnsafeCell` singleton accessor (same path as - // `jsc_hooks::resolve_embedded_source` / `node_fs`). - let graph = bun_standalone_graph::Graph::get() + // `vm.standalone_module_graph` is a type-erased `&dyn` so + // `bun_jsc` doesn't depend on `bun_standalone_graph`; the + // concrete `Graph` is the sole implementor. + let graph = bun_standalone_graph::Graph::get_ref() .expect("vm.standalone_module_graph set ⇔ Graph singleton populated"); - // SAFETY: `graph` is the `UnsafeCell::get()` pointer to the - // process-lifetime singleton; this runs on the JS thread. - if let Some(file) = unsafe { &mut *graph }.find(path_or_fd.path().slice()) { + if let Some(file) = graph.find_ref(path_or_fd.path().slice()) { use crate::api::standalone_graph_jsc::FileJsc as _; - return file.file_blob(global_this).dupe(); + return file.file_blob(global_this); } } diff --git a/src/runtime/webcore/ObjectURLRegistry.rs b/src/runtime/webcore/ObjectURLRegistry.rs index e1338568e2fd..9d2af8c2ef81 100644 --- a/src/runtime/webcore/ObjectURLRegistry.rs +++ b/src/runtime/webcore/ObjectURLRegistry.rs @@ -26,12 +26,14 @@ impl Default for ObjectURLRegistry { } } +/// Holds no VM-affine state: `blob.global_this` is null and `blob.name` is +/// `DEAD`; the name lives in `name` as bytes and each resolving VM gets its +/// own impl. pub struct Entry { blob: Blob, + name: Box<[u8]>, } -// `Entry` is auto-`Send`: its sole field is `Blob`, which already asserts -// `Send + Sync` (see `webcore_types::Blob`). No `unsafe impl` needed. const _: fn() = || { fn assert_send() {} assert_send::(); @@ -39,9 +41,15 @@ const _: fn() = || { impl Entry { pub(crate) fn init(blob: &Blob) -> Box { - Box::new(Entry { - blob: blob.dupe_with_content_type(true), - }) + let blob = blob.dupe_with_content_type(true); + blob.global_this.set(core::ptr::null()); + let name = blob + .name + .replace(bun_core::String::DEAD) + .into_utf8() + .into_vec() + .into_boxed_slice(); + Box::new(Entry { blob, name }) } } @@ -66,11 +74,20 @@ impl ObjectURLRegistry { REGISTRY.get_or_init(ObjectURLRegistry::default) } - pub(crate) fn resolve_and_dupe(&self, pathname: &[u8]) -> Option { + pub(crate) fn resolve_and_dupe( + &self, + pathname: &[u8], + global_object: &JSGlobalObject, + ) -> Option { let uuid = uuid_from_pathname(pathname)?; let map = self.map.lock(); - map.get(&uuid.bytes) - .map(|e| e.blob.dupe_with_content_type(true)) + let entry = map.get(&uuid.bytes)?; + let blob = entry.blob.dupe_with_content_type(true); + blob.global_this.set(global_object); + if !entry.name.is_empty() { + blob.name.set(bun_core::String::clone_utf8(&entry.name)); + } + Some(blob) } pub(crate) fn resolve_and_dupe_to_js( @@ -78,7 +95,7 @@ impl ObjectURLRegistry { pathname: &[u8], global_object: &JSGlobalObject, ) -> Option { - let blob = Blob::new(self.resolve_and_dupe(pathname)?); + let blob = Blob::new(self.resolve_and_dupe(pathname, global_object)?); // SAFETY: `Blob::new` returns a freshly-boxed heap pointer. Some(unsafe { (*blob).to_js(global_object) }) } diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index e0cc61431e10..33bff18bab61 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -42,7 +42,7 @@ use std::io::Write as _; use crate::webcore::jsc::{ self as jsc, CallFrame, JSGlobalObject, JSPromise, JSValue, JsResult, VirtualMachine, }; -use bun_core::{String as BunString, Tag as BunStringTag, Utf8Bytes}; +use bun_core::{String as BunString, Tag as BunStringTag}; use bun_http::{self as http, FetchRedirect, Headers, HeadersExt as _, MimeType}; use bun_http_jsc::method_jsc; use bun_http_types::Method::Method; @@ -446,7 +446,7 @@ fn fetch_impl( // Custom Hostname let mut hostname: Option> = None; let mut range: Option = None; - let mut unix_socket_path: Utf8Bytes<'static> = Utf8Bytes::EMPTY; + let mut unix_socket_path: Box<[u8]> = Box::default(); // `url_proxy_buffer` gets reassigned while `url`/`proxy` // still point into it (or into the buffer about to replace it). Detach the @@ -729,7 +729,10 @@ fn fetch_impl( if !obj.is_empty() { if let Some(socket_path) = obj.get(global_this, "unix")? { if socket_path.is_string() && socket_path.get_length(ctx)? > 0 { - break 'extract_unix_socket_path socket_path.to_utf8(global_this)?; + break 'extract_unix_socket_path socket_path + .to_bun_string(global_this)? + .to_owned_slice() + .into_boxed_slice(); } } } @@ -1252,7 +1255,7 @@ fn fetch_impl( break 'extract_headers result; }; - if proxy.is_some() && !unix_socket_path.slice().is_empty() { + if proxy.is_some() && !unix_socket_path.is_empty() { let err = ctx.to_type_error( jsc::ErrorCode::INVALID_ARG_VALUE, format_args!("fetch() cannot use a proxy with a unix socket."), @@ -1295,7 +1298,7 @@ fn fetch_impl( // Support blob: urls if url_type == URLType::Blob { if let Some(blob) = - ObjectURLRegistry::singleton().resolve_and_dupe(url_path_decoded) + ObjectURLRegistry::singleton().resolve_and_dupe(url_path_decoded, global_this) { url_string = BunString::create_format(format_args!( "blob:{}", diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 3a854c808c48..7db8e63c3a40 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -5,7 +5,7 @@ use core::sync::atomic::{AtomicBool, Ordering}; use bun_boringssl as boringssl; use bun_cares_sys::c_ares_draft as c_ares; -use bun_core::{MutableString, String as BunString, Utf8Bytes}; +use bun_core::{MutableString, String as BunString}; use bun_event_loop::{ ConcurrentTask::{AutoDeinit, ConcurrentTask}, Task, Taskable, @@ -162,6 +162,7 @@ pub struct FetchTasklet { pub(crate) upgraded_connection: bool, // Custom Hostname pub(crate) hostname: Option>, + pub(crate) unix_socket_path: Box<[u8]>, pub(crate) is_waiting_body: bool, /// Set by `on_start_buffering_callback` (JS thread) and read by /// `callback()` (HTTP thread, under `mutex`): the body is being @@ -479,6 +480,7 @@ impl FetchTasklet { if let Some(_hostname) = self.hostname.take() { // dropped by Box } + self.unix_socket_path = Box::default(); if let Some(certificate) = self.result.certificate_info.take() { drop(certificate); @@ -1899,14 +1901,8 @@ impl FetchTasklet { // reshaped for borrowck — capture metadata fields before to_body_value() takes &mut self let headers = FetchHeaders::create_from_pico_headers(http_response.headers.list); let status_code = http_response.status_code as u16; - // status_text and url must NOT be atomized: this runs on the HTTP - // thread, and atom strings live in a per-thread table — creating or - // deref'ing them off the JS thread trips the `wasRemoved` - // RELEASE_ASSERT in AtomStringImpl::remove(). Plain WTFStringImpl - // refcounts are atomic, so clone_utf8 is safe. // Fast path: when the wire reason phrase matches the canonical text for - // this status code, store a StaticEncodedSlice (deref is a no-op, so still - // safe to drop off-thread) and skip the WTF allocation entirely. + // this status code, store a StaticEncodedSlice and skip the WTF allocation. let status_text = match crate::server::http_status_text::get(status_code) .map(|t| &t[4..]) .filter(|canon| *canon == http_response.status) @@ -2041,6 +2037,7 @@ impl FetchTasklet { reject_unauthorized: fetch_options.reject_unauthorized, upgraded_connection: fetch_options.upgraded_connection, hostname: fetch_options.hostname, + unix_socket_path: fetch_options.unix_socket_path, is_waiting_body: false, is_buffering_body: AtomicBool::new(false), is_waiting_abort: false, @@ -2103,21 +2100,22 @@ impl FetchTasklet { // This task gets queued on the HTTP thread. // `AsyncHTTP::init` takes several `&'static [u8]` borrows - // (headers_buf, request_body, hostname) that point into - // FetchTasklet-owned storage. The tasklet is heap-pinned via + // (headers_buf, request_body, hostname, unix_socket_path) that point + // into FetchTasklet-owned storage. The tasklet is heap-pinned via // `heap::alloc`, so erase the borrow lifetimes through raw pointers. // SAFETY: `fetch_tasklet_ptr` is a stable heap allocation that outlives // the AsyncHTTP (dropped together in `deinit`); the slices below borrow - // its `request_headers.buf`, `request_body`, and `hostname` fields - // which are not reallocated for the lifetime of the request. + // its `request_headers.buf`, `request_body`, `hostname`, and + // `unix_socket_path` fields which are not reallocated for the lifetime + // of the request. // SAFETY (`Interned::assume` — Population B, holder-backed): // `fetch_tasklet_ptr` is a `heap::alloc`'d `FetchTasklet` whose - // `request_headers.buf` / `request_body` / `hostname` fields are not - // reallocated for the request's lifetime, and the tasklet is freed in - // `deinit` only after the owned `AsyncHTTP` is dropped. NOT - // process-lifetime — these should become `RawSlice` once - // `AsyncHTTP::init` accepts holder-lifetime slices; `assume` names the - // owner so the widen is grep-able until then. + // `request_headers.buf` / `request_body` / `hostname` / + // `unix_socket_path` fields are not reallocated for the request's + // lifetime, and the tasklet is freed in `deinit` only after the owned + // `AsyncHTTP` is dropped. NOT process-lifetime — these should become + // `RawSlice` once `AsyncHTTP::init` accepts holder-lifetime slices; + // `assume` names the owner so the widen is grep-able until then. let headers_buf: &'static [u8] = unsafe { bun_ptr::Interned::assume(fetch_tasklet.request_headers.buf.as_slice()) } .as_bytes(); @@ -2129,6 +2127,9 @@ impl FetchTasklet { .as_deref() // SAFETY: see block note above — same `FetchTasklet` owner. .map(|s| unsafe { bun_ptr::Interned::assume(s) }.as_bytes()); + // SAFETY: see block note above — same `FetchTasklet` owner. + let unix_socket_path: &'static [u8] = + unsafe { bun_ptr::Interned::assume(&fetch_tasklet.unix_socket_path) }.as_bytes(); // `MultiArrayList` owns its // allocation, so clone; AsyncHTTP::init clones again for the client. let header_entries = bun_core::handle_oom(fetch_tasklet.request_headers.entries.clone()); @@ -2157,7 +2158,7 @@ impl FetchTasklet { proxy_headers: fetch_options.proxy_headers, hostname, signals: Some(fetch_tasklet.signals), - unix_socket_path: Some(fetch_options.unix_socket_path), + unix_socket_path: Some(unix_socket_path), disable_timeout: Some(fetch_options.disable_timeout), idle_timeout_seconds: fetch_options.idle_timeout_seconds, disable_keepalive: Some(fetch_options.disable_keepalive), @@ -2788,7 +2789,7 @@ pub struct FetchOptions { // Custom Hostname pub(crate) hostname: Option>, pub(crate) check_server_identity: StrongOptional, - pub(crate) unix_socket_path: Utf8Bytes<'static>, + pub(crate) unix_socket_path: Box<[u8]>, pub(crate) ssl_config: Option, pub(crate) upgraded_connection: bool, pub(crate) forced_protocol: Option, diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index 25f7207bc308..ca9f355ef945 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -79,15 +79,12 @@ pub const BASE_PUBLIC_PATH_WITH_DEFAULT_SUFFIX: &str = const_format::concatcp!(" pub const BASE_PUBLIC_PATH_WITH_DEFAULT_SUFFIX: &str = const_format::concatcp!("/$bunfs/", "root/"); // A process-lifetime `OnceLock` (PORTING.md §Concurrency: never `static mut`). -// `get()` returns a raw `*mut`; callers -// mutate `wtf_string` / `cached_blob` / `sourcemap` lazily. A future reshape -// could push interior mutability down to those per-`File` fields (`UnsafeCell<…>`) -// so read-only paths (`find`, `entry_point`, `stat`) can take `&self`. +// `get()` returns a raw `*mut`; the only post-init mutation is +// `File::sourcemap` (`LazySourceMap::load`, serialized by `INIT_LOCK`). struct Instance(core::cell::UnsafeCell); // SAFETY: the graph is populated once at startup before any worker threads; -// post-init mutation is limited to per-`File` lazy fields. NOTE: `INIT_LOCK` -// only guards `LazySourceMap::load`; `File::to_wtf_string` and `cached_blob` -// mutate without any lock and rely on idempotence + JSC's own synchronization. +// after that `File::sourcemap` is mutated only under `INIT_LOCK` and +// `File::cached_blob` is a `OnceLock`. Everything else is read-only. // (`Send` is auto-derived: `UnsafeCell` is `Send`.) unsafe impl Sync for Instance {} @@ -103,7 +100,7 @@ impl StandaloneModuleGraph { INSTANCE.get().map(|cell| cell.0.get()) } - /// Read-only lookups. Use `get()` when mutating per-`File` lazy caches. + /// Read-only lookups. Use `get()` only for `LazySourceMap::load`. pub fn get_ref() -> Option<&'static StandaloneModuleGraph> { // SAFETY: `Instance` is `Sync`; the `&self` methods touch only the immutable tables. INSTANCE.get().map(|cell| unsafe { &*cell.0.get() }) @@ -155,9 +152,8 @@ pub fn is_bun_standalone_file_path(str_: &[u8]) -> bool { } impl StandaloneModuleGraph { - // Callers mutate `wtf_string` / `cached_blob`, so these accessors take - // `&mut self`. Switching to `UnsafeCell` per-`File` - // fields would let read-only paths take `&self`; see the `Instance` note above. + // `&mut` only for `File::sourcemap` (`LazySourceMap::load`); every other + // per-`File` access goes through `get_ref()` / `find_ref()`. pub fn entry_point(&mut self) -> &mut File { &mut self.files.values_mut()[self.entry_point_id as usize] } @@ -285,15 +281,12 @@ fn normalize_file_key<'a>(name: &'a [u8], buf: &'a mut PathBuffer) -> &'a [u8] { } // SAFETY: the graph is the process-global INSTANCE singleton (set once at -// startup, never freed). The raw-pointer / `Cell` fields it carries are -// `bun_runtime`-owned caches (`cached_blob`, `wtf_string`, source-map state) -// that are only ever touched from the JS main thread under the API lock; the -// resolver-facing read path below touches none of them. The graph pointer is -// shared across worker threads through the resolver, which is why the -// `Send + Sync` supertrait on `bun_resolver::StandaloneModuleGraph` must be -// satisfied. +// startup, never freed) shared by the main VM, Workers and resolver threads. +// The raw pointers in `File` point into the immortal section; `cached_blob` +// is a `OnceLock` holding VM-independent state only (no `global_this`, no +// WTF string); `sourcemap` is mutated only under `INIT_LOCK`. unsafe impl Send for StandaloneModuleGraph {} -// SAFETY: see `Send` impl — post-init mutation is confined to per-`File` lazy caches on the JS thread. +// SAFETY: see `Send` impl. unsafe impl Sync for StandaloneModuleGraph {} /// Resolver-facing trait object impl. The resolver and VM hold the graph as @@ -481,9 +474,10 @@ pub struct File { pub loader: Loader, pub contents: &'static ZStr, pub sourcemap: LazySourceMap, - pub cached_blob: Option>, + /// VM-independent `webcore::Blob` template (store + content type); see + /// `standalone_graph_jsc::file_blob`. + pub cached_blob: std::sync::OnceLock>, pub encoding: Encoding, - pub wtf_string: BunString, // BACKREF into the embedded section; JSC mutates the bytecode buffer in place. pub bytecode: *mut [u8], pub module_info: *mut [u8], @@ -522,37 +516,38 @@ impl File { strings::cmp_strings_asc((), lhs.name, rhs.name) } - pub fn to_wtf_string(&mut self) -> BunString { + /// `name` without the `/$bunfs/root/` prefix, as shown to JS (`Blob.name`, + /// `Bun.embeddedFiles`). + pub fn display_name(&self) -> &[u8] { + self.name + .strip_prefix(BASE_PUBLIC_PATH_WITH_DEFAULT_SUFFIX.as_bytes()) + .unwrap_or(self.name) + } + + /// A fresh impl per call so each VM owns its own; Latin-1/UTF-16 are + /// zero-copy externals over the immortal section. + pub fn to_wtf_string(&self) -> BunString { if self.contents.is_empty() { return BunString::EMPTY; } - if self.wtf_string.is_empty() { - match self.encoding { - Encoding::Binary => { - self.wtf_string = BunString::clone_utf8(self.contents.as_bytes()); - } - Encoding::Latin1 => { - self.wtf_string = - BunString::create_static_external(self.contents.as_bytes(), true); - } - Encoding::Utf16 => { - let bytes = self.contents.as_bytes(); - debug_assert!(bytes.as_ptr().addr().is_multiple_of(align_of::())); - #[expect( - clippy::cast_ptr_alignment, - reason = "`to_bytes` writes UTF-16 at an even offset and the section base is page-aligned (the 128-byte bytecode alignment relies on the same property)" - )] - // SAFETY: even byte count at a 2-byte-aligned offset of a - // section that is never freed. - let units = unsafe { - core::slice::from_raw_parts(bytes.as_ptr().cast::(), bytes.len() / 2) - }; - self.wtf_string = BunString::create_static_external_utf16(units); - } + match self.encoding { + Encoding::Binary => BunString::clone_utf8(self.contents.as_bytes()), + Encoding::Latin1 => BunString::create_static_external(self.contents.as_bytes(), true), + Encoding::Utf16 => { + let bytes = self.contents.as_bytes(); + debug_assert!(bytes.as_ptr().addr().is_multiple_of(align_of::())); + #[expect( + clippy::cast_ptr_alignment, + reason = "`to_bytes` writes UTF-16 at an even offset and the section base is page-aligned (the 128-byte bytecode alignment relies on the same property)" + )] + // SAFETY: even byte count at a 2-byte-aligned offset of a + // section that is never freed. + let units = unsafe { + core::slice::from_raw_parts(bytes.as_ptr().cast::(), bytes.len() / 2) + }; + BunString::create_static_external_utf16(units) } } - // The cached `wtf_string` keeps the impl alive for the process; hand the caller its own ref. - self.wtf_string.clone() } } @@ -868,9 +863,8 @@ impl StandaloneModuleGraph { }), module_format: module.module_format, side: module.side, - cached_blob: None, + cached_blob: std::sync::OnceLock::new(), encoding: module.encoding, - wtf_string: BunString::EMPTY, }, ); } diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index 5cc760ed5172..ab966d72364d 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -536,6 +536,32 @@ describe("bundler", () => { outfile: "dist/out", run: { stdout: "Hello, world!", setCwd: true }, }); + itBundled("compile/EmbeddedFileNamesPerThread", { + compile: true, + assetNaming: "[name].[ext]", + files: { + "/entry.ts": /* js */ ` + import { isMainThread, Worker } from "node:worker_threads"; + import "./asset.file"; + function probe() { + const f = [...Bun.embeddedFiles][0]; + const p = {}; + p[f.name] = 1; + return p[f.name.split("").join("")]; + } + console.log(probe()); + if (isMainThread) { + const w = new Worker(new URL(import.meta.url)); + await new Promise(resolve => w.on("exit", resolve)); + Bun.gc(true); + console.log(probe()); + } + `, + "/asset.file": "abcd", + }, + outfile: "dist/out", + run: { stdout: "1\n1\n1", setCwd: true }, + }); itBundled("compile/Bun.isStandaloneExecutable", { compile: true, assetNaming: "[name].[ext]", diff --git a/test/js/web/broadcastchannel/broadcast-channel.test.ts b/test/js/web/broadcastchannel/broadcast-channel.test.ts index fd8114dc58fb..f7acb723e8c5 100644 --- a/test/js/web/broadcastchannel/broadcast-channel.test.ts +++ b/test/js/web/broadcastchannel/broadcast-channel.test.ts @@ -1,3 +1,4 @@ +import { bunEnv, bunExe, tempDir } from "harness"; import util from "util"; test("postMessage results in correct event", done => { @@ -216,6 +217,39 @@ test("broadcast channel used with workers", async () => { } }, 99999); +test("array of objects posted to channels in two workers", async () => { + using dir = tempDir("broadcast-channel-two-workers", { + "main.mjs": `import { Worker, isMainThread, parentPort } from "node:worker_threads"; + const bc = new BroadcastChannel("objects"); + if (isMainThread) { + const workers = [new Worker(new URL(import.meta.url)), new Worker(new URL(import.meta.url))]; + await Promise.all(workers.map(w => new Promise(r => w.once("message", r)))); + for (let r = 0; r < 50; r++) bc.postMessage([{ ["key" + r]: r }]); + bc.postMessage("end"); + const counts = await Promise.all(workers.map(w => new Promise(r => w.once("message", r)))); + console.log(counts.join(" ")); + bc.close(); + await Promise.all(workers.map(w => w.terminate())); + } else { + const seen = {}; + bc.onmessage = e => { + if (e.data === "end") { + bc.close(); + Bun.gc(true); + parentPort.postMessage(Object.keys(seen).length); + return; + } + for (const obj of e.data) for (const k in obj) seen[k] = obj[k]; + }; + parentPort.postMessage("ready"); + }`, + }); + await using proc = Bun.spawn({ cmd: [bunExe(), "main.mjs"], cwd: String(dir), env: bunEnv, stderr: "inherit" }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + expect(stdout.trim()).toBe("50 50"); + expect(exitCode).toBe(0); +}); + test("user options are forwarded through custom inspect", () => { const bc = new BroadcastChannel("hello"); expect(util.inspect(bc, { compact: true, breakLength: 2 })).toBe( diff --git a/test/js/web/workers/worker_blob.test.ts b/test/js/web/workers/worker_blob.test.ts index aaf58ae0ea3e..3df619e6851d 100644 --- a/test/js/web/workers/worker_blob.test.ts +++ b/test/js/web/workers/worker_blob.test.ts @@ -1,4 +1,5 @@ import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; test("Worker from a Blob", async () => { const worker = new Worker( @@ -124,3 +125,53 @@ test("Worker on a revoked blob still works", async () => { expect(revoked).toBe("revoked."); }); + +test("object URL created in one worker is usable and revocable across threads", async () => { + using dir = tempDir("worker-object-url", { + "main.mjs": `import { Worker, isMainThread, parentPort, workerData } from "node:worker_threads"; + if (isMainThread) { + const a = new Worker(new URL(import.meta.url), { workerData: "a" }); + const b = new Worker(new URL(import.meta.url), { workerData: "b" }); + a.on("message", urls => b.postMessage(urls)); + b.on("message", result => { console.log(JSON.stringify(result)); a.postMessage("revoke"); }); + a.on("exit", code => { console.log("a exited", code); b.terminate(); }); + } else if (workerData === "a") { + const urls = []; + globalThis.files = []; + for (let i = 0; i < 100; i++) { + const f = new File(["x"], "ignored"); + f.name = "nm" + i + Math.random().toString(36).slice(2); + files.push(f); + urls.push(URL.createObjectURL(f)); + } + parentPort.postMessage(urls); + parentPort.on("message", () => { + for (const url of urls) URL.revokeObjectURL(url); + globalThis.files = null; + Bun.gc(true); + Bun.gc(true); + process.exit(0); + }); + } else { + parentPort.on("message", async urls => { + let count = 0, ok = true; + for (const url of urls) { + const blob = await (await fetch(url)).blob(); + const o = {}; + o[blob.name] = 1; // use the name as a property key on this thread + ok &&= blob.name.startsWith("nm"); + count++; + } + Bun.gc(true); + Bun.gc(true); + parentPort.postMessage({ count, ok }); + setInterval(() => {}, 1000); // stay alive while "a" drops the last references + }); + } + `, + }); + await using proc = Bun.spawn({ cmd: [bunExe(), "main.mjs"], cwd: String(dir), env: bunEnv, stderr: "inherit" }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + expect(stdout.trim()).toBe('{"count":100,"ok":true}\na exited 0'); + expect(exitCode).toBe(0); +}); From 015ae990afe35da5c55ad806a53e8690b046c4ca Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 13:03:33 +0000 Subject: [PATCH 02/25] review: drop the AsyncHTTP borrower before the buffers it borrows; copy external impls in toCrossThreadShareable; Blob Sync comment; CLAUDE.md cross-thread section --- src/CLAUDE.md | 19 ++++++++++++------- src/jsc/bindings/BunString.cpp | 4 ++-- src/jsc/webcore_types.rs | 4 +++- src/runtime/webcore/fetch/FetchTasklet.rs | 7 ++----- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/CLAUDE.md b/src/CLAUDE.md index 8f7ce00ac1ba..ee0a1980246a 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -384,13 +384,18 @@ non-transferring path UAFs at GC. ### Cross-thread string hazards -`AtomString`s live in a per-thread table. Never deref one from another thread — -it trips `wasRemoved` in `AtomStringImpl::remove()`. If a `bun_core::String` -may be dropped from a non-JS thread (HTTP worker, threadpool, dying VM), build -it via `String::clone_utf8` (a plain `WTFStringImpl` with an atomic refcount), -not from an interned/atomized JS string. See the comment in -`src/runtime/webcore/fetch/FetchTasklet.rs` near `Response::init` for the -canonical example of this bug class and its fix. +`StringImpl` refcounts are atomic, but atom tables are per thread: using a +string as a property key (`Identifier::fromString`) atomizes its impl *in +place* into the current thread's table, and the last `deref()` of an atom +removes it from the current thread's table (`RELEASE_ASSERT(wasRemoved)` if it +is not there). So one impl must never be reachable from two VMs — not via a +process-global registry handing out `String::clone()`s, not via one +`SerializedScriptValue` deserialized by several receivers. Hand another thread +its own bytes (`Box<[u8]>` / `clone_utf8` on arrival) or a +`Bun::isolatedCopyForSharing` copy (pre-hashed, never atomized in place; see +`src/jsc/bindings/BunString.cpp`). `String::to_thread_safe()` is a plain +isolated copy for handing a value to *one* other owner. `ObjectURLRegistry` +and the structured-clone object fast paths are the worked examples. ## Common Patterns diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index a7c97799a74f..ac2d49371863 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -319,8 +319,8 @@ WTF::String toCrossThreadShareable(const WTF::String& string) if (impl->isAtom() || impl->isSymbol()) return isolatedCopyForSharing(*impl); - // 2) Don't share slices - if (impl->bufferOwnership() == StringImpl::BufferSubstring) + // 2) Don't share slices, or external buffers whose finalizer belongs to the creating thread + if (impl->bufferOwnership() == StringImpl::BufferSubstring || impl->isExternal()) return isolatedCopyForSharing(*impl); // 3) Ensure we won't lazily touch hash/flags on the consumer thread diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index d6e73f9d1a2c..09cc2307fdfa 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -150,7 +150,9 @@ pub struct Blob { // `content_type` are atomically refcounted. unsafe impl Send for Blob {} // SAFETY: concurrent `&Blob` access only occurs under `ObjectURLRegistry`'s -// mutex or on the single owning JS thread; the `Cell` fields are never raced. +// mutex, on the standalone-graph template (never written after its `OnceLock` +// init; dupes only bump atomic refcounts), or on the single owning JS thread; +// the `Cell` fields are never raced with a writer. unsafe impl Sync for Blob {} impl Default for Blob { diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 7db8e63c3a40..33e8a10c662b 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -473,6 +473,8 @@ impl FetchTasklet { fn clear_data(&mut self) { bun_output::scoped_log!(FetchTasklet, "clearData "); + // `http.client` borrows `url_proxy_buffer` / `hostname` / `unix_socket_path` / `request_headers`. + self.http = None; if !self.url_proxy_buffer.is_empty() { self.url_proxy_buffer = Box::default(); } @@ -489,10 +491,6 @@ impl FetchTasklet { // Drop on assignment runs the cleanup. MultiArrayList has no `clear()`. self.request_headers = Headers::default(); - if let Some(http_) = self.http.as_mut() { - http_.clear_data(); - } - if let Some(metadata) = self.metadata.take() { drop(metadata); } @@ -531,7 +529,6 @@ impl FetchTasklet { // SAFETY: this was allocated via heap::alloc in `get()`; ref_count == 0 so exclusive let mut boxed = unsafe { bun_core::heap::take(this) }; boxed.clear_data(); - // self.http: Option> dropped here automatically drop(boxed); } From 3304f3baf61c446e4f080910ebacb27481016ec7 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:06:02 +0000 Subject: [PATCH 03/25] [autofix.ci] apply automated fixes --- src/CLAUDE.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/CLAUDE.md b/src/CLAUDE.md index ee0a1980246a..af06fe78c020 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -385,8 +385,8 @@ non-transferring path UAFs at GC. ### Cross-thread string hazards `StringImpl` refcounts are atomic, but atom tables are per thread: using a -string as a property key (`Identifier::fromString`) atomizes its impl *in -place* into the current thread's table, and the last `deref()` of an atom +string as a property key (`Identifier::fromString`) atomizes its impl _in +place_ into the current thread's table, and the last `deref()` of an atom removes it from the current thread's table (`RELEASE_ASSERT(wasRemoved)` if it is not there). So one impl must never be reachable from two VMs — not via a process-global registry handing out `String::clone()`s, not via one @@ -394,7 +394,7 @@ process-global registry handing out `String::clone()`s, not via one its own bytes (`Box<[u8]>` / `clone_utf8` on arrival) or a `Bun::isolatedCopyForSharing` copy (pre-hashed, never atomized in place; see `src/jsc/bindings/BunString.cpp`). `String::to_thread_safe()` is a plain -isolated copy for handing a value to *one* other owner. `ObjectURLRegistry` +isolated copy for handing a value to _one_ other owner. `ObjectURLRegistry` and the structured-clone object fast paths are the worked examples. ## Common Patterns From 2bec77918d06e8d994158cd01d72bc82a8539e31 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 13:15:58 +0000 Subject: [PATCH 04/25] docs: cross-thread rule is about atomizable impls No-Verification-Needed: doc-only --- src/CLAUDE.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/CLAUDE.md b/src/CLAUDE.md index ee0a1980246a..7be2018f3559 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -388,12 +388,13 @@ non-transferring path UAFs at GC. string as a property key (`Identifier::fromString`) atomizes its impl *in place* into the current thread's table, and the last `deref()` of an atom removes it from the current thread's table (`RELEASE_ASSERT(wasRemoved)` if it -is not there). So one impl must never be reachable from two VMs — not via a -process-global registry handing out `String::clone()`s, not via one +is not there). So an atomizable impl must never be reachable from two VMs — +not via a process-global registry handing out `String::clone()`s, not via one `SerializedScriptValue` deserialized by several receivers. Hand another thread -its own bytes (`Box<[u8]>` / `clone_utf8` on arrival) or a -`Bun::isolatedCopyForSharing` copy (pre-hashed, never atomized in place; see -`src/jsc/bindings/BunString.cpp`). `String::to_thread_safe()` is a plain +its own bytes (`Box<[u8]>` / `clone_utf8` on arrival), or share a +`Bun::isolatedCopyForSharing` / `toCrossThreadShareable` string (pre-hashed and +marked never-atomize, so a receiver's atom table copies it instead; see +`src/jsc/bindings/BunString.cpp`) or a static string. `String::to_thread_safe()` is a plain isolated copy for handing a value to *one* other owner. `ObjectURLRegistry` and the structured-clone object fast paths are the worked examples. From 5444d80715f4e923cf2b9c24fec156d64a698410 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 13:28:48 +0000 Subject: [PATCH 05/25] ObjectURLRegistry: keep an isolated copy of the name and give each resolve its own copy (lossless for empty / lone-surrogate names) --- src/runtime/webcore/ObjectURLRegistry.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/runtime/webcore/ObjectURLRegistry.rs b/src/runtime/webcore/ObjectURLRegistry.rs index 9d2af8c2ef81..c0fa86d4ceaf 100644 --- a/src/runtime/webcore/ObjectURLRegistry.rs +++ b/src/runtime/webcore/ObjectURLRegistry.rs @@ -26,12 +26,10 @@ impl Default for ObjectURLRegistry { } } -/// Holds no VM-affine state: `blob.global_this` is null and `blob.name` is -/// `DEAD`; the name lives in `name` as bytes and each resolving VM gets its -/// own impl. +/// Holds no VM-affine state: `blob.global_this` is null and `blob.name` is an +/// isolated copy that is never handed out; each resolving VM gets its own copy. pub struct Entry { blob: Blob, - name: Box<[u8]>, } const _: fn() = || { @@ -43,13 +41,8 @@ impl Entry { pub(crate) fn init(blob: &Blob) -> Box { let blob = blob.dupe_with_content_type(true); blob.global_this.set(core::ptr::null()); - let name = blob - .name - .replace(bun_core::String::DEAD) - .into_utf8() - .into_vec() - .into_boxed_slice(); - Box::new(Entry { blob, name }) + blob.name.with_mut(|name| name.to_thread_safe()); + Box::new(Entry { blob }) } } @@ -84,9 +77,7 @@ impl ObjectURLRegistry { let entry = map.get(&uuid.bytes)?; let blob = entry.blob.dupe_with_content_type(true); blob.global_this.set(global_object); - if !entry.name.is_empty() { - blob.name.set(bun_core::String::clone_utf8(&entry.name)); - } + blob.name.with_mut(|name| name.to_thread_safe()); Some(blob) } From 4c7b18fc7b46ab732e6d0bb123fbe0dd0ff7e367 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 13:34:05 +0000 Subject: [PATCH 06/25] use StringImpl::isSubString() No-Verification-Needed: equivalent accessor --- src/jsc/bindings/BunString.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index ac2d49371863..80b21cf491fd 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -320,7 +320,7 @@ WTF::String toCrossThreadShareable(const WTF::String& string) return isolatedCopyForSharing(*impl); // 2) Don't share slices, or external buffers whose finalizer belongs to the creating thread - if (impl->bufferOwnership() == StringImpl::BufferSubstring || impl->isExternal()) + if (impl->isSubString() || impl->isExternal()) return isolatedCopyForSharing(*impl); // 3) Ensure we won't lazily touch hash/flags on the consumer thread @@ -798,7 +798,7 @@ extern "C" BunString BunString__createExternalGloballyAllocatedUTF16( extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadSafe( const WTF::StringImpl* wtf) { - return !wtf->isAtom() && !wtf->isSymbol() && wtf->bufferOwnership() != WTF::StringImpl::BufferSubstring && !wtf->isExternal(); + return !wtf->isAtom() && !wtf->isSymbol() && !wtf->isSubString() && !wtf->isExternal(); } extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__WTFStringImpl__ensureHash(WTF::StringImpl* str) From 0181b24f166c62681390d0fc3736e7e436f68245 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 13:39:51 +0000 Subject: [PATCH 07/25] tests: pipe and assert stderr No-Verification-Needed: test-only --- test/js/web/broadcastchannel/broadcast-channel.test.ts | 5 +++-- test/js/web/workers/worker_blob.test.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/js/web/broadcastchannel/broadcast-channel.test.ts b/test/js/web/broadcastchannel/broadcast-channel.test.ts index f7acb723e8c5..cd86b9cc5838 100644 --- a/test/js/web/broadcastchannel/broadcast-channel.test.ts +++ b/test/js/web/broadcastchannel/broadcast-channel.test.ts @@ -244,8 +244,9 @@ test("array of objects posted to channels in two workers", async () => { parentPort.postMessage("ready"); }`, }); - await using proc = Bun.spawn({ cmd: [bunExe(), "main.mjs"], cwd: String(dir), env: bunEnv, stderr: "inherit" }); - const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + await using proc = Bun.spawn({ cmd: [bunExe(), "main.mjs"], cwd: String(dir), env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); expect(stdout.trim()).toBe("50 50"); expect(exitCode).toBe(0); }); diff --git a/test/js/web/workers/worker_blob.test.ts b/test/js/web/workers/worker_blob.test.ts index 3df619e6851d..57cbf14dc086 100644 --- a/test/js/web/workers/worker_blob.test.ts +++ b/test/js/web/workers/worker_blob.test.ts @@ -170,8 +170,9 @@ test("object URL created in one worker is usable and revocable across threads", } `, }); - await using proc = Bun.spawn({ cmd: [bunExe(), "main.mjs"], cwd: String(dir), env: bunEnv, stderr: "inherit" }); - const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + await using proc = Bun.spawn({ cmd: [bunExe(), "main.mjs"], cwd: String(dir), env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); expect(stdout.trim()).toBe('{"count":100,"ok":true}\na exited 0'); expect(exitCode).toBe(0); }); From 3a472a55ec6c6e0ca84651cc1e364a20bce12c0a Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 14:07:59 +0000 Subject: [PATCH 08/25] One primitive for cross-VM strings: String::share() (isolate if needed, pre-hash, never-atomize), handed out by clone() - BunString__share / String::share(): in place for a plain impl, isolated copy for an atom/symbol/substring. ObjectURLRegistry entries and the --compile graph's per-file name/source string are shared once and cloned per VM (same cost as before this PR); the async-fs hand-off uses it too (one FFI). - External impls are shareable again (their finalizers free from any thread); substrings are not. - Store.is_all_ascii is atomic (stores are shared across threads). - new File([blob], name) / Blob__setAsFile set the File's own name instead of renaming the shared store (also fixed `a.name` changing after `new File([a], "b")`); Blob::get_file_name is name-first. - Tests: SimpleObject path in the BroadcastChannel test, text-module probe in the compile test, File rename. --- src/CLAUDE.md | 12 ++-- src/bun_alloc/lib.rs | 7 -- src/bun_core/string/mod.rs | 44 ++++++------- src/bundler/lib.rs | 2 +- src/bundler/options.rs | 2 +- src/jsc/bindings/BunString.cpp | 30 +++++---- src/jsc/bindings/webcore/JSDOMFormData.cpp | 4 +- src/jsc/webcore_types.rs | 54 +++++++++++++--- src/runtime/api/BunObject.rs | 10 +-- src/runtime/api/standalone_graph_jsc.rs | 43 +++++-------- src/runtime/jsc_hooks.rs | 10 +-- src/runtime/server/RequestContext.rs | 2 +- src/runtime/webcore/Blob.rs | 64 ++++++------------- src/runtime/webcore/ObjectURLRegistry.rs | 8 +-- src/runtime/webcore/blob/Store.rs | 8 +-- src/standalone_graph/StandaloneModuleGraph.rs | 61 +++++++++++------- test/bundler/bundler_compile.test.ts | 8 ++- .../broadcast-channel.test.ts | 11 ++-- test/js/web/fetch/blob.test.ts | 10 +++ 19 files changed, 215 insertions(+), 175 deletions(-) diff --git a/src/CLAUDE.md b/src/CLAUDE.md index b83a1b32fe58..88cf8b9e8de2 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -392,11 +392,13 @@ is not there). So an atomizable impl must never be reachable from two VMs — not via a process-global registry handing out `String::clone()`s, not via one `SerializedScriptValue` deserialized by several receivers. Hand another thread its own bytes (`Box<[u8]>` / `clone_utf8` on arrival), or share a -`Bun::isolatedCopyForSharing` / `toCrossThreadShareable` string (pre-hashed and -marked never-atomize, so a receiver's atom table copies it instead; see -`src/jsc/bindings/BunString.cpp`) or a static string. `String::to_thread_safe()` is a plain -isolated copy for handing a value to _one_ other owner. `ObjectURLRegistry` -and the structured-clone object fast paths are the worked examples. +`String::share()`d / `Bun::isolatedCopyForSharing` / `toCrossThreadShareable` +string (pre-hashed and marked never-atomize, so a receiver's atom table copies +it instead; see `src/jsc/bindings/BunString.cpp`) or a static string — make it +shared once, then hand it out with plain `clone()`. `String::to_thread_safe()` +is a plain isolated copy for handing a value to _one_ other owner. +`ObjectURLRegistry`, `StandaloneModuleGraph::File` and the structured-clone +object fast paths are the worked examples. ## Common Patterns diff --git a/src/bun_alloc/lib.rs b/src/bun_alloc/lib.rs index 1f74f44ad552..fb9589fe5408 100644 --- a/src/bun_alloc/lib.rs +++ b/src/bun_alloc/lib.rs @@ -934,12 +934,6 @@ impl WTFStringImplStruct { pub fn ensure_hash(&self) { Bun__WTFStringImpl__ensureHash(self); } - /// After this, using the impl as a property key copies it into the atom - /// table instead of atomizing it in place. - #[inline] - pub fn set_never_atomize(&self) { - WTFStringImpl__setNeverAtomize(self); - } } unsafe extern "C" { @@ -954,7 +948,6 @@ unsafe extern "C" { pub fn Bun__WTFStringImpl__destroy(this: *const WTFStringImplStruct); safe fn WTFStringImpl__isThreadSafe(this: &WTFStringImplStruct) -> bool; safe fn Bun__WTFStringImpl__ensureHash(this: &WTFStringImplStruct); - safe fn WTFStringImpl__setNeverAtomize(this: &WTFStringImplStruct); } // ────────────────────────────────────────────────────────────────────────── diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index 6bcb57da754d..cc9b15e905c2 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -104,6 +104,7 @@ unsafe extern "C" { // fields in place; the type encodes the sole pointer-validity precondition, // so `safe fn` discharges the link-time proof here. safe fn BunString__toThreadSafe(this: &mut String); + safe fn BunString__share(this: &mut String); fn BunString__createAtom(bytes: *const u8, len: usize) -> String; fn BunString__tryCreateAtom(bytes: *const u8, len: usize) -> String; fn BunString__createStaticExternal(bytes: *const u8, len: usize, isLatin1: bool) -> String; @@ -515,10 +516,19 @@ impl String { } debug_assert!(self.is_thread_safe()); } + /// Make a WTF-backed impl safe to hand to any number of threads/VMs: + /// isolated if it was an atom/symbol/substring, pre-hashed, and never + /// atomized in place; hand it out with `clone()`. + pub fn share(&mut self) { + if self.tag == Tag::WTFStringImpl { + BunString__share(self) + } + debug_assert!(self.is_thread_safe()); + } /// True iff this `String` may be handed to another thread as-is: every /// tag except `WTFStringImpl` is inert (raw slice / static / dead), and a - /// WTF-backed string qualifies iff its impl owns its buffer and is in no - /// atom table (`isThreadSafe()`). + /// WTF-backed string qualifies iff its impl is not an atom, symbol or + /// substring (`isThreadSafe()`). /// /// Call sites that move a `String` across a thread boundary must ensure /// this holds (typically by calling [`to_thread_safe`] first); see the @@ -918,17 +928,11 @@ impl String { let mut out = self.into_utf8_with_string(); if out.string.tag == Tag::WTFStringImpl { if out.utf8.is_none() { - // 8-bit all-ASCII: a thread-safe `string` keeps backing the bytes. - if out.string.is_thread_safe() { - let wtf = out.string.as_wtf(); - wtf.ensure_hash(); - // The JS thread still holds this impl; a later property-key use there must copy, not atomize in place. - wtf.set_never_atomize(); - return out; - } - out.utf8 = Some(out.slice().to_vec()); + // 8-bit all-ASCII: a shared `string` keeps backing the bytes. + out.string.share(); + return out; } - // Transcoded or copied; drop the WTF backing to release memory. + // Transcoded; drop the WTF backing to release memory. out.string = String::EMPTY; } out @@ -979,20 +983,16 @@ impl Default for String { } // SAFETY: `String` is a tag + raw ptr to a `WTF::StringImpl` (or a borrowed // `EncodedSlice` slice / static / dead sentinel). All non-WTF tags are trivially -// `Send + Sync` (no interior mutability, no refcount). The WTF tag is the -// hazard: `WTF::StringImpl`'s refcount is non-atomic unless the impl was -// created thread-safe, so sending/sharing a non-thread-safe impl across -// threads and then `ref_()`/`deref()`ing it is a data race. +// `Send + Sync` (no interior mutability, no refcount). A `WTF::StringImpl`'s +// refcount is atomic; the hazards are per-thread atom tables and the lazily +// computed hash/flags (see "Cross-thread string hazards" in `src/CLAUDE.md`). // // We keep the blanket impls to match the C++ `BunString` // FFI contract (the type must round-trip by value through `extern "C"` and sit // in `Send + Sync` containers), and instead enforce the invariant at the -// boundary: any code that moves a `String` to another thread MUST first call -// [`String::to_thread_safe`] (or otherwise guarantee [`String::is_thread_safe`] -// returns `true`). [`String::debug_assert_thread_safe`] is the debug-build -// checkpoint for that hand-off; `to_thread_safe()` itself asserts its own -// postcondition. A `ThreadSafeString` newtype split would make this static, -// but is deferred until the FFI surface can be reshaped. +// boundary: code that moves a `String` to one other thread calls +// [`String::to_thread_safe`], code that shares one impl between threads/VMs +// calls [`String::share`] (both assert [`String::is_thread_safe`]). unsafe impl Send for String {} // SAFETY: same contract as the `Send` impl above — sharing requires // `is_thread_safe()`; non-WTF tags are inert and trivially `Sync`. diff --git a/src/bundler/lib.rs b/src/bundler/lib.rs index 5b22a82b6e77..4d5969ef8596 100644 --- a/src/bundler/lib.rs +++ b/src/bundler/lib.rs @@ -364,7 +364,7 @@ bun_dispatch::link_interface! { fn is_blob_url(specifier: &[u8]) -> bool; fn resolve_blob(specifier: &[u8]) -> Option; fn blob_loader(blob: options::OpaqueBlob) -> Option; - fn blob_file_name(blob: options::OpaqueBlob) -> Option<&'static [u8]>; + fn blob_store_path(blob: options::OpaqueBlob) -> Option<&'static [u8]>; fn blob_needs_read_file(blob: options::OpaqueBlob) -> bool; fn blob_shared_view(blob: options::OpaqueBlob) -> &'static [u8]; fn blob_deinit(blob: options::OpaqueBlob); diff --git a/src/bundler/options.rs b/src/bundler/options.rs index 1a12624a1dfc..2d34ae3f9013 100644 --- a/src/bundler/options.rs +++ b/src/bundler/options.rs @@ -520,7 +520,7 @@ pub fn get_loader_and_virtual_source<'a>( // "file:" loader makes no sense for blobs // so let's default to tsx. - if let Some(filename) = jsc_vm.blob_file_name(blob) { + if let Some(filename) = jsc_vm.blob_store_path(blob) { let current_path = Fs::Path::init(filename); // Only treat it as a file if is a Bun.file() diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index 80b21cf491fd..c641047d6b44 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -215,6 +215,20 @@ extern "C" [[ZIG_EXPORT(nothrow)]] void BunString__toThreadSafe(BunString* str) } } +extern "C" [[ZIG_EXPORT(nothrow)]] void BunString__share(BunString* str) +{ + if (str->tag != BunStringTag::WTFStringImpl) + return; + auto* impl = str->impl.wtf; + if (impl->isAtom() || impl->isSymbol() || impl->isSubString()) { + str->impl.wtf = isolatedCopyForSharing(*impl).releaseImpl().leakRef(); + impl->deref(); + } else if (!impl->isStatic()) { + impl->hash(); + impl->setNeverAtomize(); + } +} + BunString toStringRef(JSC::JSGlobalObject* globalObject, JSValue value) { auto str = value.toWTFString(globalObject); @@ -319,8 +333,8 @@ WTF::String toCrossThreadShareable(const WTF::String& string) if (impl->isAtom() || impl->isSymbol()) return isolatedCopyForSharing(*impl); - // 2) Don't share slices, or external buffers whose finalizer belongs to the creating thread - if (impl->isSubString() || impl->isExternal()) + // 2) Don't share slices + if (impl->isSubString()) return isolatedCopyForSharing(*impl); // 3) Ensure we won't lazily touch hash/flags on the consumer thread @@ -793,12 +807,12 @@ extern "C" BunString BunString__createExternalGloballyAllocatedUTF16( return { BunStringTag::WTFStringImpl, { .wtf = &impl.leakRef() } }; } -// True iff the impl owns its buffer outright and is in no thread's atom -// table, so another thread may hold a ref (after ensureHash + setNeverAtomize). +// True iff the impl owns its buffer (or is external) and is not an atom, +// symbol or substring, so another thread may hold a ref. extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadSafe( const WTF::StringImpl* wtf) { - return !wtf->isAtom() && !wtf->isSymbol() && !wtf->isSubString() && !wtf->isExternal(); + return !wtf->isAtom() && !wtf->isSymbol() && !wtf->isSubString(); } extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__WTFStringImpl__ensureHash(WTF::StringImpl* str) @@ -806,12 +820,6 @@ extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__WTFStringImpl__ensureHash(WTF::Stri str->hash(); } -extern "C" [[ZIG_EXPORT(nothrow)]] void WTFStringImpl__setNeverAtomize(WTF::StringImpl* str) -{ - if (!str->isStatic()) - str->setNeverAtomize(); -} - extern "C" JSC::EncodedJSValue JSC__JSValue__upsertBunStringArray( JSC::EncodedJSValue encodedTarget, JSC::JSGlobalObject* global, diff --git a/src/jsc/bindings/webcore/JSDOMFormData.cpp b/src/jsc/bindings/webcore/JSDOMFormData.cpp index cdccfa5b805f..57c5f6812073 100644 --- a/src/jsc/bindings/webcore/JSDOMFormData.cpp +++ b/src/jsc/bindings/webcore/JSDOMFormData.cpp @@ -308,7 +308,7 @@ static inline JSC::EncodedJSValue jsDOMFormDataPrototypeFunction_append2Body(JSC RETURN_IF_EXCEPTION(throwScope, {}); EnsureStillAliveScope argument2 = callFrame->argument(2); - auto filename = argument2.value().isUndefined() ? Blob__getFileNameString(blobValue->impl()).toWTFString(BunString::ZeroCopy) : convert(*lexicalGlobalObject, argument2.value()); + auto filename = argument2.value().isUndefined() ? Blob__getFileNameString(blobValue->impl()).transferToWTFString() : convert(*lexicalGlobalObject, argument2.value()); RETURN_IF_EXCEPTION(throwScope, {}); RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.append(WTF::move(name), WTF::move(blobValue), WTF::move(filename)); }))); @@ -471,7 +471,7 @@ static inline JSC::EncodedJSValue jsDOMFormDataPrototypeFunction_set2Body(JSC::J } RETURN_IF_EXCEPTION(throwScope, {}); - auto filename = argument2.value().isUndefined() ? Blob__getFileNameString(blobValue->impl()).toWTFString(BunString::ZeroCopy) : convert(*lexicalGlobalObject, argument2.value()); + auto filename = argument2.value().isUndefined() ? Blob__getFileNameString(blobValue->impl()).transferToWTFString() : convert(*lexicalGlobalObject, argument2.value()); RETURN_IF_EXCEPTION(throwScope, {}); RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.set(WTF::move(name), WTF::move(blobValue), WTF::move(filename)); }))); diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index 09cc2307fdfa..be1e87f7ba09 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -143,11 +143,14 @@ pub struct Blob { } // SAFETY: a `Blob` reaches another thread only as an `ObjectURLRegistry` -// entry or standalone-graph template (both with null `global_this` and a -// `DEAD` `name`; every dupe handed out is stamped with the resolving thread's -// global and a fresh name impl) or via the work-pool read/write tasks that +// entry or standalone-graph template (null `global_this`, `name` made +// `String::share()`d so dupes clone it into any VM; each dupe is stamped with +// the resolving thread's global) or via the work-pool read/write tasks that // hand it back to the owning JS thread. `store` (`StoreRef`) and -// `content_type` are atomically refcounted. +// `content_type` are atomically refcounted; a shared `Store`'s `is_all_ascii` +// is atomic, a `Bytes` store is not otherwise written after construction, and +// a `File` store's stat fields (`seekable`, `max_size`, `mode`, +// `last_modified`) are lazily cached by whichever dupe stats it. unsafe impl Send for Blob {} // SAFETY: concurrent `&Blob` access only occurs under `ObjectURLRegistry`'s // mutex, on the standalone-graph template (never written after its `OnceLock` @@ -433,9 +436,20 @@ impl Blob { matches!(self.store.get().as_deref(), Some(s) if matches!(s.data, store::Data::File(_))) } - /// `Blob.getFileName()` — the user-visible name: `Bytes.stored_name`, - /// the file path, or the S3 key. `None` for fd-backed or unnamed blobs. - pub fn get_file_name(&self) -> Option<&[u8]> { + /// The name a `File` reports: `name` if set, else [`store_path`]. + /// + /// [`store_path`]: Self::store_path + pub fn get_file_name(&self) -> Option> { + let name = self.name.get(); + if name.tag() != bun_core::Tag::Dead { + return Some(name.to_utf8()); + } + self.store_path().map(bun_core::Utf8Bytes::Borrowed) + } + + /// `Bytes.stored_name`, the file path, or the S3 key, ignoring `name`. + /// `None` for fd-backed or unnamed stores. + pub fn store_path(&self) -> Option<&[u8]> { match &self.store.get().as_deref()?.data { store::Data::Bytes(bytes) => { let n = &bytes.stored_name[..]; @@ -541,7 +555,29 @@ pub mod store { pub data: Data, pub mime_type: MimeType, pub ref_count: bun_ptr::ThreadSafeRefCount, - pub is_all_ascii: Option, + pub is_all_ascii: IsAllAscii, + } + + /// Tri-state "are the bytes all ASCII", learned lazily by whichever + /// thread's dupe reads or writes the full store first. + #[derive(Default)] + pub struct IsAllAscii(core::sync::atomic::AtomicU8); + + impl IsAllAscii { + #[inline] + pub fn get(&self) -> Option { + match self.0.load(core::sync::atomic::Ordering::Relaxed) { + 0 => None, + v => Some(v == 2), + } + } + #[inline] + pub fn set(&self, is_all_ascii: bool) { + self.0.store( + 1 + u8::from(is_all_ascii), + core::sync::atomic::Ordering::Relaxed, + ); + } } /// Backing data for a `Store`. @@ -895,7 +931,7 @@ pub mod store { data: Data::Bytes(Bytes::init(bytes)), mime_type: bun_http_types::MimeType::NONE, ref_count: bun_ptr::ThreadSafeRefCount::init(), - is_all_ascii: None, + is_all_ascii: IsAllAscii::default(), })) } diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 3f2bd409fcc1..bb1ecaad57b4 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1916,11 +1916,13 @@ fn get_embedded_files(global_this: &JSGlobalObject, _: &JSObject) -> JsResult Blob; -} - -/// The process-wide part of an embedded file's `Blob`: store + content type. -/// `global_this` is null and `name` is `DEAD`; `file_blob` stamps those per VM. +/// The process-wide part of an embedded file's `Blob`: store, content type +/// and a shared `name`. `global_this` is null; `file_blob` stamps it per VM. fn shared_blob(file: &File) -> NonNull { *file.cached_blob.get_or_init(|| { // `contents` is a `'static` slice into the embedded executable @@ -44,7 +38,7 @@ fn shared_blob(file: &File) -> NonNull NonNull NonNull NonNull Blob { - // SAFETY: `shared_blob` returns the pointer from `Blob::new`, never - // freed for the process lifetime; only read here. - let blob = - unsafe { shared_blob(self).cast::().as_ref() }.dupe_with_content_type(true); - blob.global_this.set(global); - let name = self.display_name(); - if !name.is_empty() { - blob.name.set(bstring::String::clone_utf8(name)); - } - blob - } +/// A fresh `Blob` over the shared immortal store, owned by `global`'s VM. +pub(crate) fn file_blob(file: &File, global: &JSGlobalObject) -> Blob { + // SAFETY: `shared_blob` returns the pointer from `Blob::new`, never + // freed for the process lifetime; only read here. + let blob = unsafe { shared_blob(file).cast::().as_ref() }.dupe_with_content_type(true); + blob.global_this.set(global); + blob } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index aecc7d6c034d..4be12363eb0f 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1485,8 +1485,8 @@ mod vm_loader_ctx { // Returned slices borrow blob heap storage that lives until // `blob_deinit`; erased to `'static` per the interface signature — // sound because the bundler caller drops them before `blob_deinit`. - blob_file_name(b) => blob(b) - .get_file_name() + blob_store_path(b) => blob(b) + .store_path() .map(|s| core::slice::from_raw_parts(s.as_ptr(), s.len())), blob_needs_read_file(b) => blob(b).needs_to_read_file(), blob_shared_view(b) => { @@ -4002,7 +4002,7 @@ unsafe fn get_loader_and_virtual_source<'a>( loader = blob.get_loader(unsafe { &*jsc_vm }); // "file:" loader makes no sense for blobs, so default to tsx. - if let Some(filename) = blob.get_file_name() { + if let Some(filename) = blob.store_path() { // Only treat it as a file if it is a `Bun.file()`. if blob.needs_to_read_file() { // Note: borrowck — `Fs::Path<'a>` borrows @@ -4825,7 +4825,7 @@ pub(crate) fn parse_http_date(value: &[u8]) -> Option { #[unsafe(no_mangle)] fn __bun_stdio_blob_store_new(fd: bun_sys::Fd, is_atty: bool, mode: bun_sys::Mode) -> *mut () { use bun_jsc::node_path::PathOrFileDescriptor; - use bun_jsc::webcore_types::store::{Data, File, Store}; + use bun_jsc::webcore_types::store::{Data, File, IsAllAscii, Store}; let store: Box = Store::new(Store { data: Data::File(File { pathlike: PathOrFileDescriptor::Fd(fd), @@ -4835,7 +4835,7 @@ fn __bun_stdio_blob_store_new(fd: bun_sys::Fd, is_atty: bool, mode: bun_sys::Mod }), mime_type: bun_http_types::MimeType::NONE, ref_count: bun_ptr::ThreadSafeRefCount::init_exact_refs(2), - is_all_ascii: None, + is_all_ascii: IsAllAscii::default(), }); bun_core::heap::into_raw(store).cast() } diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index 5c7bf2c02211..f220c2cac118 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -3829,7 +3829,7 @@ where // 2. The content-disposition header is not present if !has_content_disposition && content_type.category.autoset_filename() { if let Some(filename) = blob.get_file_name() { - let basename = bun_paths::basename(filename); + let basename = bun_paths::basename(&filename); if !basename.is_empty() { let mut filename_buf = [0u8; 1024]; let truncated = &basename[..basename.len().min(1024 - 32)]; diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 53059b43cc0b..426db7b221e6 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -2036,7 +2036,7 @@ impl BlobExt for Blob { if self.name.get().tag() != bun_core::Tag::Dead { return Some(self.name.get()); } - if let Some(path) = self.get_file_name() { + if let Some(path) = self.store_path() { self.name.set(BunString::clone_utf8(path)); return Some(self.name.get()); } @@ -2073,7 +2073,7 @@ impl BlobExt for Blob { fn get_loader(&self, jsc_vm: &VirtualMachine) -> Option { use bun_resolver::fs::PathResolverExt as _; if let Some(filename) = self.get_file_name() { - let current_path = bun_resolver::fs::Path::init(filename); + let current_path = bun_resolver::fs::Path::init(&filename); return Some( current_path .loader(&jsc_vm.transpiler.options.loaders) @@ -2367,8 +2367,7 @@ impl BlobExt for Blob { let len = bytes.len(); if len > 0 { let s = Store::init(bytes); - // SAFETY: freshly-minted Store with refcount==1; no other alias. - unsafe { (*s.as_ptr()).is_all_ascii = Some(is_all_ascii) }; + s.is_all_ascii.set(is_all_ascii); store = Some(s); } let blob = Blob::default(); @@ -2418,7 +2417,7 @@ impl BlobExt for Blob { data: store::Data::Bytes(result), mime_type: bun_http_types::MimeType::NONE, ref_count: bun_ptr::ThreadSafeRefCount::init(), - is_all_ascii: None, + is_all_ascii: store::IsAllAscii::default(), })); let blob = Blob::init_with_store(store, global_this); if was_string && blob.content_type_slice().is_empty() { @@ -2513,14 +2512,9 @@ impl BlobExt for Blob { // if this Blob represents the entire binary data // we can update the store's is_all_ascii flag if self.size.get() > 0 && self.offset.get() == 0 { - if let Some(store_ref) = self.store() { - let store = store_ref.as_ptr(); - // SAFETY: `store` is live (we hold a `StoreRef`); single-threaded - // JS execution means no concurrent &Store borrow is outstanding. - unsafe { - if matches!((*store).data, store::Data::Bytes(_)) { - (*store).is_all_ascii = Some(is_all_ascii); - } + if let Some(store) = self.store() { + if matches!(store.data, store::Data::Bytes(_)) { + store.is_all_ascii.set(is_all_ascii); } } } @@ -2577,7 +2571,7 @@ impl BlobExt for Blob { // false == can't be let could_be_all_ascii = self .is_all_ascii() - .or_else(|| self.store().and_then(|s| s.is_all_ascii)); + .or_else(|| self.store().and_then(|s| s.is_all_ascii.get())); if could_be_all_ascii.is_none() || !could_be_all_ascii.unwrap() { // if to_utf16_alloc returns None, it means there are no non-ASCII characters @@ -2805,7 +2799,7 @@ impl BlobExt for Blob { // false == can't be let could_be_all_ascii = self .is_all_ascii() - .or_else(|| self.store().and_then(|s| s.is_all_ascii)); + .or_else(|| self.store().and_then(|s| s.is_all_ascii.get())); // When a BOM is present `buf` is an interior slice of `raw_bytes`; we must // free the original allocation, not the offset pointer. let _free = (LIFETIME == Lifetime::Temporary).then(|| TemporaryBytes(raw_bytes)); @@ -3554,8 +3548,7 @@ impl BlobExt for Blob { let graph = bun_standalone_graph::Graph::get_ref() .expect("vm.standalone_module_graph set ⇔ Graph singleton populated"); if let Some(file) = graph.find_ref(path_or_fd.path().slice()) { - use crate::api::standalone_graph_jsc::FileJsc as _; - return file.file_blob(global_this); + return crate::api::standalone_graph_jsc::file_blob(file, global_this); } } @@ -4139,16 +4132,8 @@ pub(crate) extern "C" fn Blob__dupeFromJS(value: JSValue) -> Option`) and freed by `Bytes::Drop`. - bytes.stored_name = path_str.to_owned_slice().into_boxed_slice(); - } - } + if !path_str.is_empty() && this.get_file_name().is_none() { + this.name.set(path_str.clone()); } } @@ -4158,8 +4143,9 @@ pub(crate) extern "C" fn Blob__dupe(this: &Blob) -> *mut Blob { } #[unsafe(no_mangle)] -pub(crate) extern "C" fn Blob__getFileNameString(this: &Blob) -> bun_core::StringView<'_> { - bun_core::StringView::from_bytes(this.get_file_name().unwrap_or_default()) +pub(crate) extern "C" fn Blob__getFileNameString(this: &Blob) -> BunString { + this.get_name_string() + .map_or(BunString::EMPTY, Clone::clone) } // ────────────────────────────────────────────────────────────────────────── @@ -5424,19 +5410,9 @@ pub(crate) fn jsdom_file_construct( let name_value_str = BunString::from_js(args[1], global_this)?; blob = Blob::get::(global_this, args[0])?; - if let Some(store_) = blob.store.get() { - match store_.data_mut() { - store::Data::Bytes(bytes) => { - // `get::<_, true>` on a single-Blob sequence returns - // `dupe()` (a shared StoreRef), so this `Bytes` may already - // carry an owned `stored_name` from the source blob; the - // assignment drops (frees) the previous `Box<[u8]>`. - bytes.stored_name = name_value_str.to_owned_slice().into_boxed_slice(); - } - store::Data::S3(_) | store::Data::File(_) => { - blob.name.set(name_value_str); - } - } + if blob.store.get().is_some() { + // The store may be shared with the source blob (`dupe()`); never rename it. + blob.name.set(name_value_str); } else if !name_value_str.is_empty() { // not store but we have a name so we need a store blob.store.set(Some(StoreRef::from(Store::new(Store { @@ -5445,7 +5421,7 @@ pub(crate) fn jsdom_file_construct( )), ref_count: bun_ptr::ThreadSafeRefCount::init(), mime_type: bun_http_types::MimeType::NONE, - is_all_ascii: None, + is_all_ascii: store::IsAllAscii::default(), })))); } } @@ -6202,7 +6178,7 @@ impl Any { } } - pub(crate) fn get_file_name(&self) -> Option<&[u8]> { + pub(crate) fn get_file_name(&self) -> Option> { match self { Any::Blob(b) => b.get_file_name(), Any::WTFStringImpl(_) | Any::InternalBlob(_) => None, diff --git a/src/runtime/webcore/ObjectURLRegistry.rs b/src/runtime/webcore/ObjectURLRegistry.rs index c0fa86d4ceaf..b91e3d90828d 100644 --- a/src/runtime/webcore/ObjectURLRegistry.rs +++ b/src/runtime/webcore/ObjectURLRegistry.rs @@ -26,8 +26,9 @@ impl Default for ObjectURLRegistry { } } -/// Holds no VM-affine state: `blob.global_this` is null and `blob.name` is an -/// isolated copy that is never handed out; each resolving VM gets its own copy. +/// Holds no VM-affine state: `blob.global_this` is null and `blob.name` is a +/// shared impl (pre-hashed, never atomized in place), so dupes may clone it +/// into any VM. pub struct Entry { blob: Blob, } @@ -41,7 +42,7 @@ impl Entry { pub(crate) fn init(blob: &Blob) -> Box { let blob = blob.dupe_with_content_type(true); blob.global_this.set(core::ptr::null()); - blob.name.with_mut(|name| name.to_thread_safe()); + blob.name.with_mut(|name| name.share()); Box::new(Entry { blob }) } } @@ -77,7 +78,6 @@ impl ObjectURLRegistry { let entry = map.get(&uuid.bytes)?; let blob = entry.blob.dupe_with_content_type(true); blob.global_this.set(global_object); - blob.name.with_mut(|name| name.to_thread_safe()); Some(blob) } diff --git a/src/runtime/webcore/blob/Store.rs b/src/runtime/webcore/blob/Store.rs index 17cd6d76d8a2..42897467edc7 100644 --- a/src/runtime/webcore/blob/Store.rs +++ b/src/runtime/webcore/blob/Store.rs @@ -30,7 +30,7 @@ use super::SizeType; // ────────────────────────────────────────────────────────────────────────── pub use bun_jsc::webcore_types::store::{ - Bytes, Data, DataTag, File, S3, SerializeTag, Store, StoreRef, + Bytes, Data, DataTag, File, IsAllAscii, S3, SerializeTag, Store, StoreRef, }; // ────────────────────────────────────────────────────────────────────────── @@ -139,7 +139,7 @@ impl StoreExt for Store { data: Data::S3(S3::init(path, mime_type, credentials)), mime_type: bun_http_types::MimeType::NONE, ref_count: bun_ptr::ThreadSafeRefCount::init(), - is_all_ascii: None, + is_all_ascii: IsAllAscii::default(), })) } @@ -158,7 +158,7 @@ impl StoreExt for Store { data: Data::File(File::init(pathlike, mime_type)), mime_type: bun_http_types::MimeType::NONE, ref_count: bun_ptr::ThreadSafeRefCount::init(), - is_all_ascii: None, + is_all_ascii: IsAllAscii::default(), })) } @@ -170,7 +170,7 @@ impl StoreExt for Store { data: Data::Bytes(Bytes::init_mmap(slice)), mime_type: bun_http_types::MimeType::NONE, ref_count: bun_ptr::ThreadSafeRefCount::init(), - is_all_ascii: None, + is_all_ascii: IsAllAscii::default(), })) } diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index ca9f355ef945..27896b55e3d9 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -84,7 +84,8 @@ pub const BASE_PUBLIC_PATH_WITH_DEFAULT_SUFFIX: &str = const_format::concatcp!(" struct Instance(core::cell::UnsafeCell); // SAFETY: the graph is populated once at startup before any worker threads; // after that `File::sourcemap` is mutated only under `INIT_LOCK` and -// `File::cached_blob` is a `OnceLock`. Everything else is read-only. +// `File::cached_blob` / `File::wtf_string` are `OnceLock`s. Everything else +// is read-only. // (`Send` is auto-derived: `UnsafeCell` is `Send`.) unsafe impl Sync for Instance {} @@ -283,8 +284,9 @@ fn normalize_file_key<'a>(name: &'a [u8], buf: &'a mut PathBuffer) -> &'a [u8] { // SAFETY: the graph is the process-global INSTANCE singleton (set once at // startup, never freed) shared by the main VM, Workers and resolver threads. // The raw pointers in `File` point into the immortal section; `cached_blob` -// is a `OnceLock` holding VM-independent state only (no `global_this`, no -// WTF string); `sourcemap` is mutated only under `INIT_LOCK`. +// and `wtf_string` are `OnceLock`s holding VM-independent state only (null +// `global_this`, shared string impls); `sourcemap` is mutated only under +// `INIT_LOCK`. unsafe impl Send for StandaloneModuleGraph {} // SAFETY: see `Send` impl. unsafe impl Sync for StandaloneModuleGraph {} @@ -474,10 +476,11 @@ pub struct File { pub loader: Loader, pub contents: &'static ZStr, pub sourcemap: LazySourceMap, - /// VM-independent `webcore::Blob` template (store + content type); see + /// VM-independent `webcore::Blob` template (store, content type, shared name); see /// `standalone_graph_jsc::file_blob`. pub cached_blob: std::sync::OnceLock>, pub encoding: Encoding, + wtf_string: std::sync::OnceLock, // BACKREF into the embedded section; JSC mutates the bytecode buffer in place. pub bytecode: *mut [u8], pub module_info: *mut [u8], @@ -524,30 +527,41 @@ impl File { .unwrap_or(self.name) } - /// A fresh impl per call so each VM owns its own; Latin-1/UTF-16 are - /// zero-copy externals over the immortal section. + /// One shared impl per process (see `BunString::share`); Latin-1/UTF-16 + /// are zero-copy externals over the immortal section. pub fn to_wtf_string(&self) -> BunString { if self.contents.is_empty() { return BunString::EMPTY; } - match self.encoding { - Encoding::Binary => BunString::clone_utf8(self.contents.as_bytes()), - Encoding::Latin1 => BunString::create_static_external(self.contents.as_bytes(), true), - Encoding::Utf16 => { - let bytes = self.contents.as_bytes(); - debug_assert!(bytes.as_ptr().addr().is_multiple_of(align_of::())); - #[expect( - clippy::cast_ptr_alignment, - reason = "`to_bytes` writes UTF-16 at an even offset and the section base is page-aligned (the 128-byte bytecode alignment relies on the same property)" - )] - // SAFETY: even byte count at a 2-byte-aligned offset of a - // section that is never freed. - let units = unsafe { - core::slice::from_raw_parts(bytes.as_ptr().cast::(), bytes.len() / 2) + self.wtf_string + .get_or_init(|| { + let mut s = match self.encoding { + Encoding::Binary => BunString::clone_utf8(self.contents.as_bytes()), + Encoding::Latin1 => { + BunString::create_static_external(self.contents.as_bytes(), true) + } + Encoding::Utf16 => { + let bytes = self.contents.as_bytes(); + debug_assert!(bytes.as_ptr().addr().is_multiple_of(align_of::())); + #[expect( + clippy::cast_ptr_alignment, + reason = "`to_bytes` writes UTF-16 at an even offset and the section base is page-aligned (the 128-byte bytecode alignment relies on the same property)" + )] + // SAFETY: even byte count at a 2-byte-aligned offset of a + // section that is never freed. + let units = unsafe { + core::slice::from_raw_parts( + bytes.as_ptr().cast::(), + bytes.len() / 2, + ) + }; + BunString::create_static_external_utf16(units) + } }; - BunString::create_static_external_utf16(units) - } - } + s.share(); + s + }) + .clone() } } @@ -865,6 +879,7 @@ impl StandaloneModuleGraph { side: module.side, cached_blob: std::sync::OnceLock::new(), encoding: module.encoding, + wtf_string: std::sync::OnceLock::new(), }, ); } diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index ab966d72364d..e9e73c3328aa 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -543,11 +543,14 @@ describe("bundler", () => { "/entry.ts": /* js */ ` import { isMainThread, Worker } from "node:worker_threads"; import "./asset.file"; + import txt from "./t.txt" with { type: "text" }; function probe() { const f = [...Bun.embeddedFiles][0]; const p = {}; p[f.name] = 1; - return p[f.name.split("").join("")]; + const o = {}; + o[txt] = 1; + return p[f.name.split("").join("")] + " " + o[["embedded", "text", "module"].join("-")]; } console.log(probe()); if (isMainThread) { @@ -558,9 +561,10 @@ describe("bundler", () => { } `, "/asset.file": "abcd", + "/t.txt": "embedded-text-module", }, outfile: "dist/out", - run: { stdout: "1\n1\n1", setCwd: true }, + run: { stdout: "1 1\n1 1\n1 1", setCwd: true }, }); itBundled("compile/Bun.isStandaloneExecutable", { compile: true, diff --git a/test/js/web/broadcastchannel/broadcast-channel.test.ts b/test/js/web/broadcastchannel/broadcast-channel.test.ts index cd86b9cc5838..10898df42e43 100644 --- a/test/js/web/broadcastchannel/broadcast-channel.test.ts +++ b/test/js/web/broadcastchannel/broadcast-channel.test.ts @@ -217,14 +217,17 @@ test("broadcast channel used with workers", async () => { } }, 99999); -test("array of objects posted to channels in two workers", async () => { +test("objects posted to channels in two workers", async () => { using dir = tempDir("broadcast-channel-two-workers", { "main.mjs": `import { Worker, isMainThread, parentPort } from "node:worker_threads"; const bc = new BroadcastChannel("objects"); if (isMainThread) { const workers = [new Worker(new URL(import.meta.url)), new Worker(new URL(import.meta.url))]; await Promise.all(workers.map(w => new Promise(r => w.once("message", r)))); - for (let r = 0; r < 50; r++) bc.postMessage([{ ["key" + r]: r }]); + for (let r = 0; r < 50; r++) { + bc.postMessage([{ ["key" + r]: r }]); + bc.postMessage({ ["obj" + r]: r }); + } bc.postMessage("end"); const counts = await Promise.all(workers.map(w => new Promise(r => w.once("message", r)))); console.log(counts.join(" ")); @@ -239,7 +242,7 @@ test("array of objects posted to channels in two workers", async () => { parentPort.postMessage(Object.keys(seen).length); return; } - for (const obj of e.data) for (const k in obj) seen[k] = obj[k]; + for (const obj of Array.isArray(e.data) ? e.data : [e.data]) for (const k in obj) seen[k] = obj[k]; }; parentPort.postMessage("ready"); }`, @@ -247,7 +250,7 @@ test("array of objects posted to channels in two workers", async () => { await using proc = Bun.spawn({ cmd: [bunExe(), "main.mjs"], cwd: String(dir), env: bunEnv, stderr: "pipe" }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - expect(stdout.trim()).toBe("50 50"); + expect(stdout.trim()).toBe("100 100"); expect(exitCode).toBe(0); }); diff --git a/test/js/web/fetch/blob.test.ts b/test/js/web/fetch/blob.test.ts index 974cae2a4292..217495f189d7 100644 --- a/test/js/web/fetch/blob.test.ts +++ b/test/js/web/fetch/blob.test.ts @@ -307,6 +307,16 @@ test("#12894", () => { expect(new File([bunFile], "bar.txt").name).toBe("bar.txt"); }); +test("new File([file], name) does not rename the source", () => { + const a = new File(["x"], "a.txt"); + const b = new File([a], "b.txt"); + expect([a.name, b.name]).toEqual(["a.txt", "b.txt"]); + const c = new Blob(["y"]); + const d = new File([c], "d.txt"); + const e = new File([c], "e.txt"); + expect([d.name, e.name]).toEqual(["d.txt", "e.txt"]); +}); + test("dupeWithContentType does not alias the source's allocated content_type", async () => { // Regression: #23015 refactored Blob to be ref-counted and moved // `setNotHeapAllocated()` before the `isHeapAllocated()` guard in From 5c8436365fcd1a204afa2b9940034c32851a41d5 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 22:52:26 +0000 Subject: [PATCH 09/25] Rename: String::thread_isolated_copy() (was to_thread_safe) and make_thread_shareable()/is_thread_shareable()/ThreadShareable (was share/is_thread_safe/ThreadSafe); C++ exports to match --- src/CLAUDE.md | 19 +-- src/bun_alloc/lib.rs | 6 +- src/bun_core/string/mod.rs | 55 +++++---- src/js/internal-for-testing.ts | 4 +- src/jsc/bindings/BunString.cpp | 22 ++-- src/jsc/bindings/InternalForTesting.cpp | 21 ++-- src/jsc/bindings/InternalForTesting.h | 2 +- src/jsc/lib.rs | 4 +- src/jsc/node_path.rs | 60 ++++----- src/jsc/webcore_types.rs | 6 +- src/runtime/api/BunObject.rs | 6 +- src/runtime/api/JSBundler.rs | 4 +- src/runtime/api/JSTranspiler.rs | 8 +- src/runtime/api/standalone_graph_jsc.rs | 2 +- src/runtime/bake/production.rs | 6 +- src/runtime/crypto/PBKDF2.rs | 8 +- src/runtime/node/node_crypto_binding.rs | 10 +- src/runtime/node/node_fs.rs | 116 +++++++++--------- src/runtime/node/node_fs_binding.rs | 2 +- src/runtime/node/types.rs | 28 ++--- src/runtime/webcore/Blob.rs | 2 +- src/runtime/webcore/ObjectURLRegistry.rs | 2 +- src/runtime/webcore/blob/Store.rs | 2 +- src/standalone_graph/StandaloneModuleGraph.rs | 4 +- ...=> bunstring-thread-isolated-copy.test.ts} | 28 ++--- 25 files changed, 211 insertions(+), 216 deletions(-) rename test/js/bun/util/{bunstring-tothreadsafe.test.ts => bunstring-thread-isolated-copy.test.ts} (61%) diff --git a/src/CLAUDE.md b/src/CLAUDE.md index 88cf8b9e8de2..e722ba3639f7 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -115,16 +115,16 @@ source is a bare `&[u8]`/`EncodedSlice` view. `Utf8Bytes<'a>` is `Borrowed(&'a [u8]) | Owned(Vec) | Shared(String)` (`Shared` holds an 8-bit all-ASCII WTF-backed `String` and reads its buffer); it derefs to `[u8]`; `is_owned()` ⇔ the bytes were transcoded/copied. -`Utf8WithString` (`String::into_utf8_with_string[_thread_safe]()`) keeps the +`Utf8WithString` (`String::into_utf8_with_string[_thread_shareable]()`) keeps the UTF-8 bytes _and_ the source `String` so the value can go back to JS without re-encoding; `Utf8WithString::js_only(string)` wraps an output-only string. -`PathLike<'a>` / `StringOrBuffer<'a>` arms: `String`/`ThreadsafeString` +`PathLike<'a>` / `StringOrBuffer<'a>` arms: `String`/`ThreadShareableString` (`Utf8WithString` from a JS string), `Utf8(Utf8Bytes<'a>)` (transcoded JS string, or Rust-side bytes: `PathLike::borrowed(bytes)` lends `&'a [u8]` to a synchronous call, `PathLike::owned(vec)` when the value must own them), `Buffer`. Anything -parsed from JS, stored, or sent to another thread (`to_thread_safe`, -`ThreadSafe`, the fs `args::*<'static>` async path) is `'static`. +parsed from JS, stored, or sent to another thread (`make_thread_shareable`, +`ThreadShareable`, the fs `args::*<'static>` async path) is `'static`. `EncodedSlice<'a>` is the `{ptr, len}` + encoding-bits (Latin-1/UTF-8/UTF-16) borrowed view handed to C++. Constructors name the encoding of the bytes: @@ -392,11 +392,12 @@ is not there). So an atomizable impl must never be reachable from two VMs — not via a process-global registry handing out `String::clone()`s, not via one `SerializedScriptValue` deserialized by several receivers. Hand another thread its own bytes (`Box<[u8]>` / `clone_utf8` on arrival), or share a -`String::share()`d / `Bun::isolatedCopyForSharing` / `toCrossThreadShareable` -string (pre-hashed and marked never-atomize, so a receiver's atom table copies -it instead; see `src/jsc/bindings/BunString.cpp`) or a static string — make it -shared once, then hand it out with plain `clone()`. `String::to_thread_safe()` -is a plain isolated copy for handing a value to _one_ other owner. +`String::make_thread_shareable()`d / `Bun::isolatedCopyForSharing` / +`toCrossThreadShareable` string (pre-hashed and marked never-atomize, so a +receiver's atom table copies it instead; see `src/jsc/bindings/BunString.cpp`) +or a static string — make it shareable once, then hand it out with plain +`clone()`. `String::thread_isolated_copy()` is a plain isolated copy for +handing a value to _one_ other owner. `ObjectURLRegistry`, `StandaloneModuleGraph::File` and the structured-clone object fast paths are the worked examples. diff --git a/src/bun_alloc/lib.rs b/src/bun_alloc/lib.rs index fb9589fe5408..7f28dbbfbf00 100644 --- a/src/bun_alloc/lib.rs +++ b/src/bun_alloc/lib.rs @@ -926,8 +926,8 @@ impl WTFStringImplStruct { } } #[inline] - pub fn is_thread_safe(&self) -> bool { - WTFStringImpl__isThreadSafe(self) + pub fn is_thread_shareable(&self) -> bool { + WTFStringImpl__isThreadShareable(self) } /// Compute the hash() if necessary #[inline] @@ -946,7 +946,7 @@ unsafe extern "C" { // `destroy` path crosses FFI. `*const` + `unsafe`: it frees the // allocation backing the pointer. pub fn Bun__WTFStringImpl__destroy(this: *const WTFStringImplStruct); - safe fn WTFStringImpl__isThreadSafe(this: &WTFStringImplStruct) -> bool; + safe fn WTFStringImpl__isThreadShareable(this: &WTFStringImplStruct) -> bool; safe fn Bun__WTFStringImpl__ensureHash(this: &WTFStringImplStruct); } diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index 3212d9d82ec0..7d1b63f8d9b1 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -103,8 +103,8 @@ unsafe extern "C" { // `assert_ffi_layout!` above). C++ reads/writes only the `tag`/`value` // fields in place; the type encodes the sole pointer-validity precondition, // so `safe fn` discharges the link-time proof here. - safe fn BunString__toThreadSafe(this: &mut String); - safe fn BunString__share(this: &mut String); + safe fn BunString__threadIsolatedCopy(this: &String) -> String; + safe fn BunString__makeThreadShareable(this: &mut String); fn BunString__createAtom(bytes: *const u8, len: usize) -> String; fn BunString__tryCreateAtom(bytes: *const u8, len: usize) -> String; fn BunString__createStaticExternal(bytes: *const u8, len: usize, isLatin1: bool) -> String; @@ -508,36 +508,40 @@ impl String { core::ptr::null_mut() } } - /// Replace a WTF-backed impl with an isolated copy, for handing the value - /// to one other thread; not for sharing one impl between VMs. - pub fn to_thread_safe(&mut self) { - if self.tag == Tag::WTFStringImpl { - BunString__toThreadSafe(self) - } - debug_assert!(self.is_thread_safe()); + /// An isolated copy of a WTF-backed impl (+1, `clone()` for other tags), + /// for handing the value to one other thread; not for sharing one impl + /// between VMs. + pub fn thread_isolated_copy(&self) -> String { + let copy = if self.tag == Tag::WTFStringImpl { + BunString__threadIsolatedCopy(self) + } else { + self.clone() + }; + debug_assert!(copy.is_thread_shareable()); + copy } /// Make a WTF-backed impl safe to hand to any number of threads/VMs: /// isolated if it was an atom/symbol/substring, pre-hashed, and never /// atomized in place; hand it out with `clone()`. - pub fn share(&mut self) { + pub fn make_thread_shareable(&mut self) { if self.tag == Tag::WTFStringImpl { - BunString__share(self) + BunString__makeThreadShareable(self) } - debug_assert!(self.is_thread_safe()); + debug_assert!(self.is_thread_shareable()); } /// True iff this `String` may be handed to another thread as-is: every /// tag except `WTFStringImpl` is inert (raw slice / static / dead), and a /// WTF-backed string qualifies iff its impl is not an atom, symbol or - /// substring (`isThreadSafe()`). + /// substring (`isThreadShareable()`). /// /// Call sites that move a `String` across a thread boundary must ensure - /// this holds (typically by calling [`to_thread_safe`] first); see the + /// this holds (typically via [`thread_isolated_copy`] first); see the /// `Send`/`Sync` SAFETY comment for the full contract. #[inline] - pub(crate) fn is_thread_safe(&self) -> bool { + pub(crate) fn is_thread_shareable(&self) -> bool { if self.tag == Tag::WTFStringImpl { // SAFETY: WTF tag guarantees `value.wtf` is a valid live impl. - self.as_wtf().is_thread_safe() + self.as_wtf().is_thread_shareable() } else { true } @@ -924,12 +928,12 @@ impl String { /// safe to send to another thread. /// /// [`into_utf8_with_string`]: Self::into_utf8_with_string - pub fn into_utf8_with_string_thread_safe(self) -> Utf8WithString { + pub fn into_utf8_with_string_thread_shareable(self) -> Utf8WithString { let mut out = self.into_utf8_with_string(); if out.string.tag == Tag::WTFStringImpl { if out.utf8.is_none() { - // 8-bit all-ASCII: a thread-safe `string` keeps backing the bytes. - if out.string.is_thread_safe() { + // 8-bit all-ASCII: a thread-shareable `string` keeps backing the bytes. + if out.string.is_thread_shareable() { return out; } out.utf8 = Some(out.slice().to_vec()); @@ -993,11 +997,12 @@ impl Default for String { // FFI contract (the type must round-trip by value through `extern "C"` and sit // in `Send + Sync` containers), and instead enforce the invariant at the // boundary: code that moves a `String` to one other thread calls -// [`String::to_thread_safe`], code that shares one impl between threads/VMs -// calls [`String::share`] (both assert [`String::is_thread_safe`]). +// [`String::thread_isolated_copy`], code that shares one impl between +// threads/VMs calls [`String::make_thread_shareable`] (both assert +// [`String::is_thread_shareable`]). unsafe impl Send for String {} // SAFETY: same contract as the `Send` impl above — sharing requires -// `is_thread_safe()`; non-WTF tags are inert and trivially `Sync`. +// `is_thread_shareable()`; non-WTF tags are inert and trivially `Sync`. unsafe impl Sync for String {} impl Drop for String { @@ -1568,9 +1573,9 @@ impl Utf8WithString { (self.utf8, self.string) } - /// If `string` is WTF-backed, migrate it to a thread-safe impl. - pub fn to_thread_safe(&mut self) { - self.string.to_thread_safe(); + /// If `string` is WTF-backed, replace it with a thread-isolated copy. + pub fn make_thread_shareable(&mut self) { + self.string = self.string.thread_isolated_copy(); } } diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 75351c397299..40e31c1508c4 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -658,9 +658,9 @@ export const structuredCloneAdvanced: ( export const isASANEnabled: () => boolean = $newCppFunction("InternalForTesting.cpp", "jsFunction_isASANEnabled", 0); -export const BunString_toThreadSafeRefCountDelta: () => number = $newCppFunction( +export const BunString_threadIsolatedCopyRefCountDelta: () => number = $newCppFunction( "InternalForTesting.cpp", - "jsFunction_BunString_toThreadSafeRefCountDelta", + "jsFunction_BunString_threadIsolatedCopyRefCountDelta", 0, ); diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index c641047d6b44..74f5e8fd2739 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -200,22 +200,14 @@ JSC::JSString* toJS(JSC::JSGlobalObject* globalObject, BunString bunString) UNREACHABLE(); } -extern "C" [[ZIG_EXPORT(nothrow)]] void BunString__toThreadSafe(BunString* str) -{ - if (str->tag == BunStringTag::WTFStringImpl) { - auto* existing = str->impl.wtf; - // StringImpl::isolatedCopy() always returns a freshly-allocated impl, - // so when we replace the pointer we must release the ref we were - // holding to the original; otherwise every call leaks one ref. - auto impl = existing->isolatedCopy(); - if (impl.ptr() != existing) { - str->impl.wtf = &impl.leakRef(); - existing->deref(); - } - } +extern "C" [[ZIG_EXPORT(nothrow)]] BunString BunString__threadIsolatedCopy(const BunString* str) +{ + if (str->tag == BunStringTag::WTFStringImpl) + return { BunStringTag::WTFStringImpl, { .wtf = &str->impl.wtf->isolatedCopy().leakRef() } }; + return *str; } -extern "C" [[ZIG_EXPORT(nothrow)]] void BunString__share(BunString* str) +extern "C" [[ZIG_EXPORT(nothrow)]] void BunString__makeThreadShareable(BunString* str) { if (str->tag != BunStringTag::WTFStringImpl) return; @@ -809,7 +801,7 @@ extern "C" BunString BunString__createExternalGloballyAllocatedUTF16( // True iff the impl owns its buffer (or is external) and is not an atom, // symbol or substring, so another thread may hold a ref. -extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadSafe( +extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadShareable( const WTF::StringImpl* wtf) { return !wtf->isAtom() && !wtf->isSymbol() && !wtf->isSubString(); diff --git a/src/jsc/bindings/InternalForTesting.cpp b/src/jsc/bindings/InternalForTesting.cpp index 46b5faa9a3a1..234f4766ad16 100644 --- a/src/jsc/bindings/InternalForTesting.cpp +++ b/src/jsc/bindings/InternalForTesting.cpp @@ -9,7 +9,7 @@ #include #include -extern "C" void BunString__toThreadSafe(BunString* str); +extern "C" BunString BunString__threadIsolatedCopy(const BunString* str); namespace Bun { @@ -60,15 +60,15 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_isASANEnabled, (JSC::JSGlobalObject * global } // Returns the net refcount change on the *original* StringImpl after a -// BunString owning one ref to it is passed through BunString__toThreadSafe -// and then released. A correct implementation must return 0; a positive -// value means BunString__toThreadSafe leaked a reference to the original -// StringImpl when it installed the isolated copy. -JSC_DEFINE_HOST_FUNCTION(jsFunction_BunString_toThreadSafeRefCountDelta, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +// BunString owning one ref to it is passed through BunString__threadIsolatedCopy +// and then both are released. A correct implementation must return 0; a +// positive value means BunString__threadIsolatedCopy leaked a reference to the +// original StringImpl. +JSC_DEFINE_HOST_FUNCTION(jsFunction_BunString_threadIsolatedCopyRefCountDelta, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { // Create a fresh, non-static, non-atom StringImpl with exactly one ref // held by `original`. - Ref original = WTF::String::fromLatin1("BunString__toThreadSafe leak test").releaseImpl().releaseNonNull(); + Ref original = WTF::String::fromLatin1("BunString__threadIsolatedCopy leak test").releaseImpl().releaseNonNull(); const unsigned before = original->refCount(); @@ -77,11 +77,10 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_BunString_toThreadSafeRefCountDelta, (JSC::J original->ref(); BunString str = { BunStringTag::WTFStringImpl, { .wtf = original.ptr() } }; - BunString__toThreadSafe(&str); + BunString copy = BunString__threadIsolatedCopy(&str); - // Drop whatever the BunString now owns (the isolated copy, or the - // original if the implementation ever decides no copy is needed). - ASSERT(str.tag == BunStringTag::WTFStringImpl); + ASSERT(copy.tag == BunStringTag::WTFStringImpl); + copy.impl.wtf->deref(); str.impl.wtf->deref(); const unsigned after = original->refCount(); diff --git a/src/jsc/bindings/InternalForTesting.h b/src/jsc/bindings/InternalForTesting.h index aa8a31069781..2b6c3bdfc4cc 100644 --- a/src/jsc/bindings/InternalForTesting.h +++ b/src/jsc/bindings/InternalForTesting.h @@ -8,7 +8,7 @@ namespace Bun { JSC_DECLARE_HOST_FUNCTION(jsFunction_arrayBufferViewHasBuffer); JSC_DECLARE_HOST_FUNCTION(jsFunction_hasReifiedStatic); JSC_DECLARE_HOST_FUNCTION(jsFunction_isASANEnabled); -JSC_DECLARE_HOST_FUNCTION(jsFunction_BunString_toThreadSafeRefCountDelta); +JSC_DECLARE_HOST_FUNCTION(jsFunction_BunString_threadIsolatedCopyRefCountDelta); JSC_DECLARE_HOST_FUNCTION(jsFunction_lowercaseHeaderNameSIMD); JSC_DECLARE_HOST_FUNCTION(jsFunction_emitMemoryPressure); JSC_DECLARE_HOST_FUNCTION(jsFunction_isMemoryPressureWatcherInstalled); diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index 7c1547f7355c..11c8d69b0123 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -1251,9 +1251,9 @@ pub use self::array_buffer::JSTypedArrayBytesDeallocator; pub mod node_path; #[path = "webcore_types.rs"] pub mod webcore_types; -// RAII pair for `to_thread_safe()`/`unprotect()` — re-exported at crate root +// RAII pair for `make_thread_shareable()`/`unprotect()` — re-exported at crate root // so `bun_runtime` callers don't reach through `node_path`. -pub use self::node_path::{ThreadSafe, Unprotect}; +pub use self::node_path::{ThreadShareable, Unprotect}; /// `jsc.WebCore` (deprecated alias) — only the data-shape subset /// that was hoisted to this tier. Reach for `bun_runtime::webcore` for the diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index 324b905d05c4..dc3037d174e2 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -13,37 +13,37 @@ use bun_sys::Fd; use crate::array_buffer::MarkedArrayBuffer; // ────────────────────────────────────────────────────────────────────────── -// RAII for `protect()`/`unprotect()` pairs taken by `to_thread_safe()`. +// RAII for `protect()`/`unprotect()` pairs taken by `make_thread_shareable()`. // -// The async-fs path calls `to_thread_safe()` (which `JSValue::protect()`s +// The async-fs path calls `make_thread_shareable()` (which `JSValue::protect()`s // any borrowed JS-backed buffers so the work-pool thread may read them) and // must later release them. The "deinit" half is // already `Drop`; only the JS-side `unprotect()` needs an explicit hook, and // pairing it with the protect via a guard type removes the leak hazard on -// every early return between `to_thread_safe` and the manual cleanup. +// every early return between `make_thread_shareable` and the manual cleanup. // ────────────────────────────────────────────────────────────────────────── -/// Undo the `JSValue::protect()` calls taken by [`to_thread_safe`]( -/// PathLike::to_thread_safe) (or an `args::*` type's `to_thread_safe`). +/// Undo the `JSValue::protect()` calls taken by [`make_thread_shareable`]( +/// PathLike::make_thread_shareable) (or an `args::*` type's `make_thread_shareable`). /// /// Implementations release **only** the JS-GC protect refcount — owned Rust /// payloads (Vec, `Utf8WithString`, …) are freed by the type's own /// `Drop`, which runs immediately after when the value is held in a -/// [`ThreadSafe`]. +/// [`ThreadShareable`]. pub trait Unprotect { fn unprotect(&mut self); } -/// RAII guard returned by `into_thread_safe()`: a `T` whose JS-backed buffers +/// RAII guard returned by `into_thread_shareable()`: a `T` whose JS-backed buffers /// have been `protect()`ed. `Drop` calls [`Unprotect::unprotect`] then drops /// the inner `T` normally. /// /// `repr(transparent)` so identity-casts in the const-generic dispatch macros /// (see `node_fs.rs`'s `args_as!`) remain bit-exact. #[repr(transparent)] -pub struct ThreadSafe(T); +pub struct ThreadShareable(T); -impl ThreadSafe { +impl ThreadShareable { /// Wrap an **already-protected** `T`. Use when the protect was taken /// elsewhere (e.g. inside `from_js_maybe_async(.., Flavor::Async, ..)`). #[inline] @@ -56,9 +56,9 @@ impl ThreadSafe { // GC-protected for as long as it is held, so a pool job may read them (under // its `Ticket`, which keeps the VM alive); the job comes back to the JS // thread, where this is dropped and the protection released. -unsafe impl Send for ThreadSafe {} +unsafe impl Send for ThreadShareable {} -impl core::ops::Deref for ThreadSafe { +impl core::ops::Deref for ThreadShareable { type Target = T; #[inline] fn deref(&self) -> &T { @@ -66,14 +66,14 @@ impl core::ops::Deref for ThreadSafe { } } -impl core::ops::DerefMut for ThreadSafe { +impl core::ops::DerefMut for ThreadShareable { #[inline] fn deref_mut(&mut self) -> &mut T { &mut self.0 } } -impl Drop for ThreadSafe { +impl Drop for ThreadShareable { #[inline] fn drop(&mut self) { // The same argument types serve mini-loop threads (the shell's `cp` via @@ -87,14 +87,14 @@ impl Drop for ThreadSafe { } } -impl Default for ThreadSafe { +impl Default for ThreadShareable { #[inline] fn default() -> Self { Self(T::default()) } } -// `ThreadSafe` crosses to the work-pool thread; auto-`Send` iff `T: Send`. +// `ThreadShareable` crosses to the work-pool thread; auto-`Send` iff `T: Send`. /// `node.PathLike`. Parsed from JS it is `PathLike<'static>`; `Utf8` may /// instead borrow Rust-side bytes for a synchronous call ([`PathLike::borrowed`]). @@ -103,7 +103,7 @@ pub enum PathLike<'a> { /// Always shares its WTF string's bytes (built by `shared_or_utf8`); /// transcoded paths are `Utf8(Owned)`. String(Utf8WithString), - ThreadsafeString(Utf8WithString), + ThreadShareableString(Utf8WithString), Utf8(Utf8Bytes<'a>), } @@ -128,7 +128,7 @@ impl Clone for PathLike<'_> { pinned: false, }), Self::String(s) => Self::String(s.clone()), - Self::ThreadsafeString(s) => Self::ThreadsafeString(s.clone()), + Self::ThreadShareableString(s) => Self::ThreadShareableString(s.clone()), Self::Utf8(s) => Self::Utf8(s.clone()), } } @@ -143,7 +143,7 @@ impl Drop for PathLike<'_> { b.buffer.unpin(); } } - Self::String(_) | Self::ThreadsafeString(_) | Self::Utf8(_) => {} + Self::String(_) | Self::ThreadShareableString(_) | Self::Utf8(_) => {} } } } @@ -171,7 +171,7 @@ impl<'a> PathLike<'a> { pub fn slice(&self) -> &[u8] { match self { Self::Buffer(b) => b.slice(), - Self::String(s) | Self::ThreadsafeString(s) => s.slice(), + Self::String(s) | Self::ThreadShareableString(s) => s.slice(), Self::Utf8(s) => s.slice(), } } @@ -179,7 +179,7 @@ impl<'a> PathLike<'a> { pub(crate) fn estimated_size(&self) -> usize { match self { Self::Buffer(b) => b.slice().len(), - Self::String(_) | Self::ThreadsafeString(_) => 0, + Self::String(_) | Self::ThreadShareableString(_) => 0, Self::Utf8(s) => s.slice().len(), } } @@ -187,31 +187,31 @@ impl<'a> PathLike<'a> { impl PathLike<'static> { /// Promote any borrowed-JS - /// payload to a thread-safe representation. For `Buffer` the variant is + /// payload to a thread-shareable representation. For `Buffer` the variant is /// kept and the backing JS value is `protect()`ed (paired with /// [`Unprotect::unprotect`]); the discriminant is preserved so callers /// matching on `Buffer` after this call see the same shape. /// - /// Called in place by the fs `args::*` types' `into_thread_safe`, which - /// wrap the result in a [`ThreadSafe`] guard. - pub fn to_thread_safe(&mut self) { + /// Called in place by the fs `args::*` types' `into_thread_shareable`, which + /// wrap the result in a [`ThreadShareable`] guard. + pub fn make_thread_shareable(&mut self) { match self { Self::String(s) => { - s.to_thread_safe(); + s.make_thread_shareable(); let owned = core::mem::take(s); - *self = Self::ThreadsafeString(owned); + *self = Self::ThreadShareableString(owned); } Self::Buffer(b) => { b.buffer.value.protect(); } - Self::ThreadsafeString(_) | Self::Utf8(_) => {} + Self::ThreadShareableString(_) | Self::Utf8(_) => {} } } } impl Unprotect for PathLike<'static> { /// JS-side half of cleanup — undo - /// the `protect()` taken by [`Self::to_thread_safe`] / + /// the `protect()` taken by [`Self::make_thread_shareable`] / /// `ArgumentsSlice::protect_eat`. Owned payloads are released by `Drop`. #[inline] fn unprotect(&mut self) { @@ -264,9 +264,9 @@ impl PathOrFileDescriptorSerializeTag { impl PathOrFileDescriptor<'static> { #[inline] - pub fn to_thread_safe(&mut self) { + pub fn make_thread_shareable(&mut self) { if let Self::Path(p) = self { - p.to_thread_safe(); + p.make_thread_shareable(); } } } diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index f2135aeb5ecd..667f84ca71f2 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -141,9 +141,9 @@ pub struct Blob { // SAFETY: a `Blob` reaches another thread only as an `ObjectURLRegistry` // entry or standalone-graph template (null `global_this`, `name` made -// `String::share()`d so dupes clone it into any VM; each dupe is stamped with -// the resolving thread's global) or via the work-pool read/write tasks that -// hand it back to the owning JS thread. `store` (`StoreRef`) and +// `String::make_thread_shareable()`d so dupes clone it into any VM; each dupe +// is stamped with the resolving thread's global) or via the work-pool +// read/write tasks that hand it back to the owning JS thread. `store` (`StoreRef`) and // `content_type` are atomically refcounted; a shared `Store`'s `is_all_ascii` // is atomic, a `Bytes` store is not otherwise written after construction, and // a `File` store's stat fields (`seekable`, `max_size`, `mode`, diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 1dc837fe4c0a..953e0d9c5b60 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -2775,8 +2775,8 @@ pub mod JSZstd { /// `Bun.zstdCompress` / `Bun.zstdDecompress` off the JS thread. pub(crate) struct ZstdJob { /// Created with `Flavor::Async` (JS-backed buffer protected); the - /// [`bun_jsc::ThreadSafe`] releases that with the job. - pub buffer: bun_jsc::ThreadSafe>, + /// [`bun_jsc::ThreadShareable`] releases that with the job. + pub buffer: bun_jsc::ThreadShareable>, pub is_compress: bool, pub level: i32, /// Filled in by `run`. @@ -2833,7 +2833,7 @@ pub mod JSZstd { jsc::Job::::schedule( &cx, ZstdJob { - buffer: bun_jsc::ThreadSafe::adopt(buffer), + buffer: bun_jsc::ThreadShareable::adopt(buffer), is_compress, level, result: Ok(Box::default()), diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index eec08f040934..5e10e1f2fde3 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -78,12 +78,12 @@ pub mod js_bundler { while let Some((prop, property_value)) = files_iter.next()? { // Parse the value as BlobOrStringOrBuffer using async mode for thread safety. // Async mode `protect()`s any JS-backed buffer; adopt into a - // `ThreadSafe` so the guard unprotects + drops at end of iteration. + // `ThreadShareable` so the guard unprotects + drops at end of iteration. let blob_or_string = match crate::node::BlobOrStringOrBuffer::from_js_async( global_this, property_value, )? { - Some(v) => bun_jsc::ThreadSafe::adopt(v), + Some(v) => bun_jsc::ThreadShareable::adopt(v), None => { return Err(global_this.throw_invalid_arguments(format_args!("Expected file content to be a string, Blob, File, TypedArray, or ArrayBuffer"))); } diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index d15d46b5895a..01e588d41812 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -633,7 +633,7 @@ impl Config { /// into the owning `JSTranspiler`'s config (its `Transpiler` is bit-copied), /// which the job's Js side keeps alive and the pool borrow keeps valid. pub(crate) struct TransformTask { - pub input_code: bun_jsc::ThreadSafe>, + pub input_code: bun_jsc::ThreadShareable>, pub output_code: BunString, pub transpiler: core::mem::ManuallyDrop>, pub log: bun_ast::Log, @@ -673,7 +673,7 @@ impl TransformTask { fn schedule( transpiler: &JSTranspiler, transpiler_js: JSValue, - input_code: bun_jsc::ThreadSafe>, + input_code: bun_jsc::ThreadShareable>, global: &JSGlobalObject, loader: Loader, ) -> JSValue { @@ -1383,9 +1383,9 @@ impl JSTranspiler { code = StringOrBuffer::owned(bytes); } // `errdefer code.deinitAndUnprotect()` — `from_js_with_encoding_maybe_async` - // (`Flavor::Async`) already protected; adopt into a `ThreadSafe` so any + // (`Flavor::Async`) already protected; adopt into a `ThreadShareable` so any // early-return drop unprotects. `TransformTask::create` takes the guard. - let code = bun_jsc::ThreadSafe::adopt(code); + let code = bun_jsc::ThreadShareable::adopt(code); args.eat(); let loader: Option = 'brk: { diff --git a/src/runtime/api/standalone_graph_jsc.rs b/src/runtime/api/standalone_graph_jsc.rs index 47f35ebb508a..af1b49196de7 100644 --- a/src/runtime/api/standalone_graph_jsc.rs +++ b/src/runtime/api/standalone_graph_jsc.rs @@ -62,7 +62,7 @@ fn shared_blob(file: &File) -> NonNull crate::Result< if let Some(entry_point_index) = file.entry_point_index { if (entry_point_index as usize) < module_keys.len() { - let mut str = BunString::create_format(format_args!( + let str = BunString::create_format(format_args!( "bake:/{}", BStr::new(without_prefix) )); - str.to_thread_safe(); - module_keys[entry_point_index as usize] = str; + module_keys[entry_point_index as usize] = + str.thread_isolated_copy(); } } diff --git a/src/runtime/crypto/PBKDF2.rs b/src/runtime/crypto/PBKDF2.rs index 29a7ee772408..202e578d46a5 100644 --- a/src/runtime/crypto/PBKDF2.rs +++ b/src/runtime/crypto/PBKDF2.rs @@ -61,7 +61,7 @@ impl PBKDF2 { // `password`/`salt` are `StringOrBuffer` whose `Drop` releases the // slice/WTF ref, so no explicit cleanup hook is needed — // dropping `PBKDF2` is sufficient for the sync path. The async path holds - // `ThreadSafe`, whose `Drop` additionally unprotects JS-rooted + // `ThreadShareable`, whose `Drop` additionally unprotects JS-rooted // buffers via the `Unprotect` impl below. /// The second element is the validated callback on the `Async` flavor and @@ -267,8 +267,8 @@ impl bun_jsc::Unprotect for PBKDF2 { /// `crypto.pbkdf2` off the JS thread. pub(crate) struct Pbkdf2Job { /// `from_js(.., Flavor::Async)` protected the input buffers; the - /// [`bun_jsc::ThreadSafe`] releases that with the job. - pub pbkdf2: bun_jsc::ThreadSafe, + /// [`bun_jsc::ThreadShareable`] releases that with the job. + pub pbkdf2: bun_jsc::ThreadShareable, pub output: Vec, pub err: bool, } @@ -343,7 +343,7 @@ pub(crate) fn create_job(global_this: &JSGlobalObject, data: PBKDF2, callback: J &cx, Pbkdf2Job { // `from_js(.., Flavor::Async)` already protected — adopt, don't re-protect. - pbkdf2: bun_jsc::ThreadSafe::adopt(data), + pbkdf2: bun_jsc::ThreadShareable::adopt(data), output: Vec::new(), err: false, }, diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index 865608a93488..6388c005e19f 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -755,9 +755,9 @@ pub mod random { // Scrypt // ─────────────────────────────────────────────────────────────────────────── pub(crate) struct Scrypt { - // Plain `StringOrBuffer` — NOT `ThreadSafe<_>`. The struct serves both + // Plain `StringOrBuffer` — NOT `ThreadShareable<_>`. The struct serves both // `scryptSync` (no protect taken) and async `scrypt` (protect taken in - // `from_js_maybe_async(.., Flavor::Async, ..)`, adopted into a `ThreadSafe` + // `from_js_maybe_async(.., Flavor::Async, ..)`, adopted into a `ThreadShareable` // by the job). password: StringOrBuffer<'static>, salt: StringOrBuffer<'static>, @@ -1087,7 +1087,7 @@ mod _impl { impl bun_jsc::Unprotect for Scrypt { /// Release the `protect()` taken by `from_js_maybe_async(.., Flavor::Async, ..)` - /// on the async path (via the job's `ThreadSafe`). The sync path never calls this. + /// on the async path (via the job's `ThreadShareable`). The sync path never calls this. #[inline] fn unprotect(&mut self) { bun_jsc::Unprotect::unprotect(&mut self.password); @@ -1098,7 +1098,7 @@ mod _impl { /// `crypto.scrypt` off the JS thread: derives straight into the result /// ArrayBuffer's bytes under the job's ticket, which keeps their VM alive. pub(crate) struct ScryptJob { - params: bun_jsc::ThreadSafe, + params: bun_jsc::ThreadShareable, result: JsPtr<[u8]>, err: Option, } @@ -1307,7 +1307,7 @@ mod _impl { fn scrypt(global: &JSGlobalObject, call_frame: &CallFrame) -> JsResult { let (ctx, callback) = Scrypt::from_js::(global, call_frame)?; // Protected by `from_js::`; released with the job wherever it ends. - let params = bun_jsc::ThreadSafe::adopt(ctx); + let params = bun_jsc::ThreadShareable::adopt(ctx); if params.keylen as usize > jsc::virtual_machine::synthetic_allocation_limit() { return Err(global.throw_out_of_memory()); } diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 4aa37c71cc46..592d8c7045fe 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -17,7 +17,7 @@ use bun_jsc::AbortSignal; use bun_jsc::debugger::AsyncTaskTracker; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ - EventLoopHandle, JSGlobalObject, JSValue, JsResult, StringJsc as _, ThreadSafe, Unprotect, + EventLoopHandle, JSGlobalObject, JSValue, JsResult, StringJsc as _, ThreadShareable, Unprotect, }; use bun_paths::{self as paths, OSPathBuffer, OSPathChar, OSPathSliceZ, PathBuffer}; use bun_sys::FdExt as _; @@ -640,8 +640,8 @@ mod _async_tasks { #[cfg(windows)] pub struct UVFSRequest { pub(crate) promise: JSPromiseStrong, - /// Wrapped in [`ThreadSafe`] so the paired `unprotect()` runs on drop. - pub args: ThreadSafe, + /// Wrapped in [`ThreadShareable`] so the paired `unprotect()` runs on drop. + pub args: ThreadShareable, pub(crate) global_object: bun_ptr::BackRef, pub(crate) req: uv::fs_t, pub(crate) result: Maybe, @@ -671,7 +671,7 @@ mod _async_tasks { ) -> JSValue { let task = Box::new(Self { promise: JSPromiseStrong::init(global_object), - args: task_args.into_thread_safe(), + args: task_args.into_thread_shareable(), // Sentinel — overwritten by `uv_callback` (or the early-return arms // below) before any read on the JS thread. `Maybe` is // `Result` and may be niche-optimised for arbitrary @@ -704,7 +704,7 @@ mod _async_tasks { ($Args:ty) => {{ debug_assert_eq!(core::mem::size_of::(), core::mem::size_of::<$Args>()); // SAFETY: identity cast — `A == $Args` for this `F` (see `async_::*`). - // `ThreadSafe` is `repr(transparent)`; deref through it for the inner `A`. + // `ThreadShareable` is `repr(transparent)`; deref through it for the inner `A`. unsafe { &*(&*task.args as *const A as *const $Args) } }}; } @@ -766,7 +766,7 @@ mod _async_tasks { let buf = &buf[..buf.len().min(args.length as usize)]; let bufs = [uv::uv_buf_t::init(buf)]; // SAFETY: libuv copies the iovec descriptor before return; the - // backing Buffer is JS-protected via `to_thread_safe`. + // backing Buffer is JS-protected via `make_thread_shareable`. let rc = unsafe { uv::uv_fs_read( loop_, @@ -994,9 +994,9 @@ mod _async_tasks { // NewAsyncFSTask — runs a NodeFS method on the thread pool. // ────────────────────────────────────────────────────────────────────────── - /// Trait abstracting over Argument types' deinit/toThreadSafe. + /// Trait abstracting over Argument types' deinit/make_thread_shareable. /// - /// Every Arguments struct defines `to_thread_safe` (clone + /// Every Arguments struct defines `make_thread_shareable` (clone /// any borrowed JS-backed slices so the work-pool callback may run off-thread). /// The trait methods are **required** so missing impls are a compile error rather /// than a silent UAF/leak. @@ -1007,36 +1007,36 @@ mod _async_tasks { /// `from_js`; the trait forwards to it so the generic `Bindings` in /// `node_fs_binding.rs` can call it without per-type macro arms. fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult; - fn to_thread_safe(&mut self); + fn make_thread_shareable(&mut self); /// Consume `self`, protect any JS-backed buffers, and return a guard that /// unprotects on drop — /// string/slice ownership is handled by each field's `Drop` (PathLike, /// StringOrBuffer, Vec); only the JS-side `unprotect()` needs the guard. #[inline] - fn into_thread_safe(mut self) -> ThreadSafe { - self.to_thread_safe(); - ThreadSafe::adopt(self) + fn into_thread_shareable(mut self) -> ThreadShareable { + self.make_thread_shareable(); + ThreadShareable::adopt(self) } fn signal(&self) -> Option<&AbortSignal> { None } } - /// Forward [`FsArgument`] to the inherent `from_js` / `to_thread_safe` + /// Forward [`FsArgument`] to the inherent `from_js` / `make_thread_shareable` /// methods each `args::*` struct already defines. /// [`Unprotect`] is implemented per-type alongside. macro_rules! impl_fs_argument { ( $( $ty:ty ),+ $(,)? ) => { $( impl FsArgument for $ty { #[inline] fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { <$ty>::from_js(ctx, arguments) } - #[inline] fn to_thread_safe(&mut self) { <$ty>::to_thread_safe(self) } + #[inline] fn make_thread_shareable(&mut self) { <$ty>::make_thread_shareable(self) } } )+ }; - // Fd-only types — `to_thread_safe` is a no-op (these hold only `FD`/scalars). + // Fd-only types — `make_thread_shareable` is a no-op (these hold only `FD`/scalars). ( @fd $( $ty:ty ),+ $(,)? ) => { $( impl FsArgument for $ty { #[inline] fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { <$ty>::from_js(ctx, arguments) } - #[inline] fn to_thread_safe(&mut self) { <$ty>::to_thread_safe(self) } + #[inline] fn make_thread_shareable(&mut self) { <$ty>::make_thread_shareable(self) } } impl Unprotect for $ty { #[inline] fn unprotect(&mut self) {} @@ -1085,8 +1085,8 @@ mod _async_tasks { args::ReadFile::from_js(ctx, arguments) } #[inline] - fn to_thread_safe(&mut self) { - args::ReadFile::to_thread_safe(self) + fn make_thread_shareable(&mut self) { + args::ReadFile::make_thread_shareable(self) } #[inline] fn signal(&self) -> Option<&AbortSignal> { @@ -1100,8 +1100,8 @@ mod _async_tasks { args::WriteFile::from_js(ctx, arguments) } #[inline] - fn to_thread_safe(&mut self) { - args::WriteFile::to_thread_safe(self) + fn make_thread_shareable(&mut self) { + args::WriteFile::make_thread_shareable(self) } #[inline] fn signal(&self) -> Option<&AbortSignal> { @@ -1116,8 +1116,8 @@ mod _async_tasks { .map(args::AppendFile) } #[inline] - fn to_thread_safe(&mut self) { - self.0.to_thread_safe(); + fn make_thread_shareable(&mut self) { + self.0.make_thread_shareable(); } #[inline] fn signal(&self) -> Option<&AbortSignal> { @@ -1227,13 +1227,13 @@ mod _async_tasks { } /// One `fs.promises.*` operation on the work pool. The arguments' JS-backed - /// buffers are protected (`ThreadSafe`) and read under the job's ticket. + /// buffers are protected (`ThreadShareable`) and read under the job's ticket. pub struct AsyncFSTask { - pub args: ThreadSafe, + pub args: ThreadShareable, pub(crate) result: Maybe, } // SAFETY: results are plain data / owned buffers / WTF strings built off - // thread for hand-off (`ret::*`); `ThreadSafe` is Send by its contract. + // thread for hand-off (`ret::*`); `ThreadShareable` is Send by its contract. unsafe impl Send for AsyncFSTask {} /// The JS-thread half of an async fs operation. @@ -1331,7 +1331,7 @@ mod _async_tasks { bun_jsc::Job::::schedule( &global_object.js_thread(), Self { - args: args.into_thread_safe(), + args: args.into_thread_shareable(), // Sentinel — overwritten by `run` before any read. `Maybe` // may be niche-optimised; never construct an all-zero `Result`. result: Err(sys::Error::default()), @@ -1356,8 +1356,8 @@ mod _async_tasks { pub struct NewAsyncCpTask { pub(crate) promise: JSPromiseStrong, - /// Wrapped in [`ThreadSafe`] so the paired `unprotect()` runs on drop. - pub args: ThreadSafe>, + /// Wrapped in [`ThreadShareable`] so the paired `unprotect()` runs on drop. + pub args: ThreadShareable>, /// Owning-thread uses (global object, keep-alive context). pub(crate) evtloop: EventLoopHandle, /// How the last subtask's thread delivers the completion (moved out @@ -1584,7 +1584,7 @@ mod _async_tasks { ) -> *mut Self { let mut task = Box::new(Self { promise, - args: cp_args.into_thread_safe(), + args: cp_args.into_thread_shareable(), has_result: AtomicBool::new(false), // Sentinel — overwritten by `finish_concurrently` (gated by the // `has_result` CAS) before any read on the JS thread. @@ -1768,8 +1768,8 @@ mod _async_tasks { let ctx = event_loop_handle_to_ctx(task.evtloop); task.r#ref.unref(ctx); } - // `Drop for ThreadSafe` releases the `protect()` taken by - // `to_thread_safe()` when `src`/`dest` are Buffers, so nothing leaks here. + // `Drop for ThreadShareable` releases the `protect()` taken by + // `make_thread_shareable()` when `src`/`dest` are Buffers, so nothing leaks here. } /// Directory scanning + clonefile will block this thread, then each individual file copy (what the sync version @@ -2136,7 +2136,7 @@ mod _async_tasks { pub struct AsyncReaddirRecursiveTask { /// Protected arguments; their JS-backed path is not read off-thread /// (`root_path` is the owned copy). - pub args: ThreadSafe>, + pub args: ThreadShareable>, pub(crate) tag: ret::ReaddirTag, pub(crate) encoding: Encoding, /// The completion token, finished by whichever subtask ends the scan. @@ -2169,7 +2169,7 @@ mod _async_tasks { pub(crate) pending_err_mutex: bun_threading::Mutex, } // SAFETY: shared by the pool subtasks through atomics / the lock-free - // queue / the mutex; `args` is Send by `ThreadSafe`'s contract; results + // queue / the mutex; `args` is Send by `ThreadShareable`'s contract; results // are owned buffers and WTF strings built off-thread for hand-off. unsafe impl Send for AsyncReaddirRecursiveTask {} @@ -2360,7 +2360,7 @@ mod _async_tasks { bun_jsc::Job::::schedule( &global_object.js_thread(), AsyncReaddirRecursiveTask { - args: FsArgument::into_thread_safe(args), + args: FsArgument::into_thread_shareable(args), tag, encoding, done: None, @@ -2607,12 +2607,12 @@ pub use _async_tasks::{ pub mod args { use super::*; - /// Derive the `Unprotect` impl + inherent `to_thread_safe` for an `args::*` + /// Derive the `Unprotect` impl + inherent `make_thread_shareable` for an `args::*` /// struct whose only JS-backed state is one-or-more path-like fields. Each - /// listed `$field` must expose `.unprotect()` and `.to_thread_safe()` (i.e. + /// listed `$field` must expose `.unprotect()` and `.make_thread_shareable()` (i.e. /// `PathLike` or `PathOrFileDescriptor`); the expansion is byte-identical to /// the hand-written boilerplate it replaces, so `impl_fs_argument!`'s - /// `<$ty>::to_thread_safe(self)` forwarder and `ThreadSafe`'s drop-guard + /// `<$ty>::make_thread_shareable(self)` forwarder and `ThreadShareable`'s drop-guard /// keep working unchanged. Structs with non-path JS state (`Read`, `Write`, /// `Writev`, `Readv`, `Exists`, `ReadFile`, `WriteFile`) keep bespoke impls. macro_rules! fs_args_path_forwarders { @@ -2621,7 +2621,7 @@ pub mod args { #[inline] fn unprotect(&mut self) { $( self.$field.unprotect(); )+ } } impl $ty<'static> { - pub fn to_thread_safe(&mut self) { $( self.$field.to_thread_safe(); )+ } + pub fn make_thread_shareable(&mut self) { $( self.$field.make_thread_shareable(); )+ } } }; } @@ -2699,7 +2699,7 @@ pub mod args { } } impl FdVectorIo { - pub(crate) fn to_thread_safe(&mut self) { + pub(crate) fn make_thread_shareable(&mut self) { self.buffers.value.protect(); self.buffers.buffers = self.buffers.buffers.as_slice().to_vec(); } @@ -2737,7 +2737,7 @@ pub mod args { fn unprotect(&mut self) {} } impl FTruncate { - pub(crate) fn to_thread_safe(&self) {} + pub(crate) fn make_thread_shareable(&self) {} pub fn from_js( ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice, @@ -2811,7 +2811,7 @@ pub mod args { pub(crate) gid: GidT, } impl Fchown { - pub(crate) fn to_thread_safe(&self) {} + pub(crate) fn make_thread_shareable(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let uid: UidT = 'brk: { @@ -2927,7 +2927,7 @@ pub mod args { pub(crate) mode: Mode, } impl FChmod { - pub(crate) fn to_thread_safe(&self) {} + pub(crate) fn make_thread_shareable(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let mode_arg = arguments.next().unwrap_or(JSValue::UNDEFINED); @@ -3015,7 +3015,7 @@ pub mod args { pub(crate) big_int: bool, } impl Fstat { - pub(crate) fn to_thread_safe(&mut self) {} + pub(crate) fn make_thread_shareable(&mut self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let big_int = 'brk: { @@ -3223,8 +3223,8 @@ pub mod args { pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { Ok(Rm(RmDir::from_js_impl(ctx, arguments, true)?)) } - pub(crate) fn to_thread_safe(&mut self) { - self.0.to_thread_safe(); + pub(crate) fn make_thread_shareable(&mut self) { + self.0.make_thread_shareable(); } } @@ -3455,7 +3455,7 @@ pub mod args { pub(crate) fd: FD, } impl Close { - pub(crate) fn to_thread_safe(&self) {} + pub(crate) fn make_thread_shareable(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; Ok(Close { fd }) @@ -3505,7 +3505,7 @@ pub mod args { pub(crate) mtime: TimeLike, } impl Futimes { - pub(crate) fn to_thread_safe(&self) {} + pub(crate) fn make_thread_shareable(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let atime = node::time_like_from_js( @@ -3583,8 +3583,8 @@ pub mod args { } } impl Write<'static> { - pub(crate) fn to_thread_safe(&mut self) { - self.buffer.to_thread_safe(); + pub(crate) fn make_thread_shareable(&mut self) { + self.buffer.make_thread_shareable(); } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; @@ -3736,7 +3736,7 @@ pub mod args { pub(crate) pinned: bool, } impl Read { - pub(crate) fn to_thread_safe(&self) { + pub(crate) fn make_thread_shareable(&self) { self.buffer.buffer.value.protect(); } } @@ -3983,8 +3983,8 @@ pub mod args { } } impl ReadFile<'static> { - pub(crate) fn to_thread_safe(&mut self) { - self.path.to_thread_safe(); + pub(crate) fn make_thread_shareable(&mut self) { + self.path.make_thread_shareable(); } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { // `Drop` on `path` covers every @@ -4071,8 +4071,8 @@ pub mod args { } } impl WriteFile<'static> { - pub(crate) fn to_thread_safe(&mut self) { - self.file.to_thread_safe(); + pub(crate) fn make_thread_shareable(&mut self) { + self.file.make_thread_shareable(); } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { Self::from_js_with_default_flag(ctx, arguments, FileSystemFlags::W) @@ -4192,9 +4192,9 @@ pub mod args { } } impl Exists<'static> { - pub(crate) fn to_thread_safe(&mut self) { + pub(crate) fn make_thread_shareable(&mut self) { if let Some(p) = &mut self.path { - p.to_thread_safe(); + p.make_thread_shareable(); } } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { @@ -4225,7 +4225,7 @@ pub mod args { pub(crate) fd: FD, } impl FdataSync { - pub(crate) fn to_thread_safe(&self) {} + pub(crate) fn make_thread_shareable(&self) {} pub fn from_js( ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice, @@ -4318,7 +4318,7 @@ pub mod args { pub(crate) fd: FD, } impl Fsync { - pub(crate) fn to_thread_safe(&self) {} + pub(crate) fn make_thread_shareable(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; Ok(Fsync { fd }) diff --git a/src/runtime/node/node_fs_binding.rs b/src/runtime/node/node_fs_binding.rs index c58f8457087b..b0fd5980b6c5 100644 --- a/src/runtime/node/node_fs_binding.rs +++ b/src/runtime/node/node_fs_binding.rs @@ -89,7 +89,7 @@ fn parse_async_args( // `ManuallyDrop` keeps `slice` alive past return when ownership transfers // to the Task: dropped only on the early-return // error/abort branches; on the success path the Task owns `args` (whose - // protected JSValues are released by `Drop for ThreadSafe` when the + // protected JSValues are released by `Drop for ThreadShareable` when the // Task completes), and `slice` is intentionally not dropped — its // `Drop`-unprotect would race that. diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 39994e94200e..4f0508921964 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -272,7 +272,7 @@ impl BlobOrStringOrBuffer { /// Rust-side bytes for a synchronous call ([`StringOrBuffer::borrowed`]). pub enum StringOrBuffer<'a> { String(Utf8WithString), - ThreadsafeString(Utf8WithString), + ThreadShareableString(Utf8WithString), Utf8(Utf8Bytes<'a>), Buffer(Buffer), } @@ -299,7 +299,7 @@ impl<'a> StringOrBuffer<'a> { pub(crate) fn slice(&self) -> &[u8] { match self { Self::String(str) => str.slice(), - Self::ThreadsafeString(str) => str.slice(), + Self::ThreadShareableString(str) => str.slice(), Self::Utf8(str) => str.slice(), Self::Buffer(str) => str.slice(), } @@ -309,7 +309,7 @@ impl<'a> StringOrBuffer<'a> { impl bun_jsc::Unprotect for BlobOrStringOrBuffer { /// JS-side half of cleanup — owned /// payloads are released by `Drop` (which runs next when held in a - /// [`bun_jsc::ThreadSafe`]). + /// [`bun_jsc::ThreadShareable`]). #[inline] fn unprotect(&mut self) { if let Self::StringOrBuffer(sob) = self { @@ -320,7 +320,7 @@ impl bun_jsc::Unprotect for BlobOrStringOrBuffer { impl bun_jsc::Unprotect for StringOrBuffer<'static> { /// JS-side half of cleanup — undo the - /// `protect()` taken by [`StringOrBuffer::to_thread_safe`] / + /// `protect()` taken by [`StringOrBuffer::make_thread_shareable`] / /// `from_js_maybe_async(.., Flavor::Async, ..)`. Owned slices are released /// by `Drop`. #[inline] @@ -338,7 +338,7 @@ impl bun_jsc::Unprotect for StringOrBuffer<'static> { impl StringOrBuffer<'_> { pub fn into_js(self, ctx: &JSGlobalObject) -> JsResult { match self { - Self::ThreadsafeString(str) | Self::String(str) => str.into_js(ctx), + Self::ThreadShareableString(str) | Self::String(str) => str.into_js(ctx), Self::Utf8(utf8) => bun_string_jsc::create_utf8_for_js(ctx, &utf8), Self::Buffer(mut buffer) => { if buffer.buffer.value != JSValue::ZERO { @@ -361,14 +361,14 @@ impl StringOrBuffer<'_> { } impl StringOrBuffer<'static> { - pub(crate) fn to_thread_safe(&mut self) { + pub(crate) fn make_thread_shareable(&mut self) { match self { Self::String(s) => { - s.to_thread_safe(); + s.make_thread_shareable(); let str = core::mem::take(s); - *self = Self::ThreadsafeString(str); + *self = Self::ThreadShareableString(str); } - Self::ThreadsafeString(_) | Self::Utf8(_) => {} + Self::ThreadShareableString(_) | Self::Utf8(_) => {} Self::Buffer(buffer) => { buffer.buffer.value.protect(); } @@ -421,8 +421,8 @@ impl StringOrBuffer<'static> { *out = if flavor == Flavor::Async { shared_or_utf8( global, - str.into_utf8_with_string_thread_safe(), - Self::ThreadsafeString, + str.into_utf8_with_string_thread_shareable(), + Self::ThreadShareableString, Self::Utf8, ) } else { @@ -821,7 +821,7 @@ pub struct NameTooLong; /// `bun_runtime`-tier behaviour layered on `bun_jsc::node_path::PathLike`. /// -/// `to_thread_safe` / `into_thread_safe` / `slice` / `estimated_size` are +/// `make_thread_shareable` / `into_thread_shareable` / `slice` / `estimated_size` are /// inherent on the lower-tier type (see `bun_jsc::node_path`); this trait /// adds only the path-buffer slicers and JS-argument parsing that depend on /// `bun_runtime` types (`Valid`, `ArgumentsSlice` cursor flow). @@ -1233,7 +1233,7 @@ fn path_like_from_string( will_be_async: bool, ) -> JsResult> { let utf8 = if will_be_async { - str.into_utf8_with_string_thread_safe() + str.into_utf8_with_string_thread_shareable() } else { str.into_utf8_with_string() }; @@ -1241,7 +1241,7 @@ fn path_like_from_string( Valid::path_null_bytes(utf8.slice(), global)?; let shared = if will_be_async { - PathLike::ThreadsafeString + PathLike::ThreadShareableString } else { PathLike::String }; diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 9f0dae05cdc1..64223c13e319 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3573,7 +3573,7 @@ impl BlobExt for Blob { } } - path_or_fd.to_thread_safe(); + path_or_fd.make_thread_shareable(); core::mem::replace(path_or_fd, PathOrFileDescriptor::Path(PathLike::default())) } PathOrFileDescriptor::Fd(fd) => { diff --git a/src/runtime/webcore/ObjectURLRegistry.rs b/src/runtime/webcore/ObjectURLRegistry.rs index b91e3d90828d..525d9bbe6e81 100644 --- a/src/runtime/webcore/ObjectURLRegistry.rs +++ b/src/runtime/webcore/ObjectURLRegistry.rs @@ -42,7 +42,7 @@ impl Entry { pub(crate) fn init(blob: &Blob) -> Box { let blob = blob.dupe_with_content_type(true); blob.global_this.set(core::ptr::null()); - blob.name.with_mut(|name| name.share()); + blob.name.with_mut(|name| name.make_thread_shareable()); Box::new(Entry { blob }) } } diff --git a/src/runtime/webcore/blob/Store.rs b/src/runtime/webcore/blob/Store.rs index b1c7dccfa872..337665f5fda1 100644 --- a/src/runtime/webcore/blob/Store.rs +++ b/src/runtime/webcore/blob/Store.rs @@ -128,7 +128,7 @@ impl StoreExt for Store { ) -> Result, crate::Error> { let mut path = pathlike; // this actually protects/refs the pathlike - path.to_thread_safe(); + path.make_thread_shareable(); // Compute the extension-derived fallback before moving `path` into the // Store so we don't need to clone the owned PathLike. diff --git a/src/standalone_graph/StandaloneModuleGraph.rs b/src/standalone_graph/StandaloneModuleGraph.rs index 13e68bc22270..81d16ac5d2ff 100644 --- a/src/standalone_graph/StandaloneModuleGraph.rs +++ b/src/standalone_graph/StandaloneModuleGraph.rs @@ -611,7 +611,7 @@ impl File { .unwrap_or(self.name) } - /// One shared impl per process (see `BunString::share`); Latin-1/UTF-16 + /// One shared impl per process (see `BunString::make_thread_shareable`); Latin-1/UTF-16 /// are zero-copy externals over the immortal section. pub fn to_wtf_string(&self) -> BunString { if self.contents.is_empty() { @@ -642,7 +642,7 @@ impl File { BunString::create_static_external_utf16(units) } }; - s.share(); + s.make_thread_shareable(); s }) .clone() diff --git a/test/js/bun/util/bunstring-tothreadsafe.test.ts b/test/js/bun/util/bunstring-thread-isolated-copy.test.ts similarity index 61% rename from test/js/bun/util/bunstring-tothreadsafe.test.ts rename to test/js/bun/util/bunstring-thread-isolated-copy.test.ts index ed0d0ae59bc5..0d2665ed4d14 100644 --- a/test/js/bun/util/bunstring-tothreadsafe.test.ts +++ b/test/js/bun/util/bunstring-thread-isolated-copy.test.ts @@ -1,29 +1,27 @@ -import { BunString_toThreadSafeRefCountDelta } from "bun:internal-for-testing"; +import { BunString_threadIsolatedCopyRefCountDelta } from "bun:internal-for-testing"; import { expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; -// BunString__toThreadSafe must release the ref it held on the previous -// StringImpl when it installs the isolated copy. Before the fix this -// leaked one ref per call because StringImpl::isolatedCopy() always -// returns a brand-new impl and the old pointer was overwritten without -// ever being deref'd. -test("BunString__toThreadSafe does not leak a ref on the original StringImpl", () => { - expect(typeof BunString_toThreadSafeRefCountDelta).toBe("function"); +// BunString__threadIsolatedCopy must not leak a ref on the original +// StringImpl: it returns a +1 isolated copy and leaves the source's +// refcount untouched. +test("BunString__threadIsolatedCopy does not leak a ref on the original StringImpl", () => { + expect(typeof BunString_threadIsolatedCopyRefCountDelta).toBe("function"); // A correct implementation leaves the original StringImpl's refcount - // unchanged once the BunString is released. A positive delta means the + // unchanged once both BunStrings are released. A positive delta means the // original ref was leaked. for (let i = 0; i < 8; i++) { - expect(BunString_toThreadSafeRefCountDelta()).toBe(0); + expect(BunString_threadIsolatedCopyRefCountDelta()).toBe(0); } }); // Exercise the real callers (Bun.file / async fs.write) whose Zig-side -// SliceWithUnderlyingString.toThreadSafe wrappers were updated alongside the +// Utf8WithString::make_thread_shareable wrappers were updated alongside the // C++ fix. With ASAN this would crash on a double-deref if the two sides ever // disagree on who owns the old StringImpl. -test("toThreadSafe callers (Bun.file / fs.write) keep refcounts balanced", async () => { - using dir = tempDir("bunstring-tothreadsafe", { +test("make_thread_shareable callers (Bun.file / fs.write) keep refcounts balanced", async () => { + using dir = tempDir("bunstring-thread-isolated-copy", { "target.txt": "", }); const src = ` @@ -34,7 +32,7 @@ test("toThreadSafe callers (Bun.file / fs.write) keep refcounts balanced", async const targetPath = path.join(process.env.TEST_DIR, "target.txt"); - // Bun.file(path) routes through SliceWithUnderlyingString.toThreadSafe. + // Bun.file(path) routes through PathLike::make_thread_shareable. for (let i = 0; i < 64; i++) { const p = targetPath + ""; const f = Bun.file(p); @@ -42,7 +40,7 @@ test("toThreadSafe callers (Bun.file / fs.write) keep refcounts balanced", async } // Async fs.write(fd, string) routes the data string through - // StringOrBuffer -> SliceWithUnderlyingString.toThreadSafe. + // StringOrBuffer::make_thread_shareable. const fd = fs.openSync(targetPath, "w"); const payload = Buffer.alloc(48, "p").toString(); let total = 0; From 8c215f397e2bd6497d22c1dadc808145c7a04cd6 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 22:54:02 +0000 Subject: [PATCH 10/25] source-lints: ThreadSafe -> ThreadShareable in the thread-door inventory No-Verification-Needed: lint inventory --- test/internal/source-lints/vm-thread-door.inventory.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/internal/source-lints/vm-thread-door.inventory.json b/test/internal/source-lints/vm-thread-door.inventory.json index 028d50fec01c..4fa878aecbb2 100644 --- a/test/internal/source-lints/vm-thread-door.inventory.json +++ b/test/internal/source-lints/vm-thread-door.inventory.json @@ -50,7 +50,7 @@ }, "src/jsc/node_path.rs": { "unsafe impl Send": [ - "ThreadSafe" + "ThreadShareable" ] }, "src/jsc/web_worker.rs": { From 92ac3815e9d54593df5123048ccbfee674cd97b4 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 23:16:03 +0000 Subject: [PATCH 11/25] is_thread_isolated() and is_thread_shareable() check exactly what thread_isolated_copy() and make_thread_shareable() produce --- src/bun_alloc/lib.rs | 5 +++++ src/bun_core/string/mod.rs | 37 ++++++++++++++++------------------ src/jsc/bindings/BunString.cpp | 13 +++++++++--- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/bun_alloc/lib.rs b/src/bun_alloc/lib.rs index 7f28dbbfbf00..c5f47132adc9 100644 --- a/src/bun_alloc/lib.rs +++ b/src/bun_alloc/lib.rs @@ -926,6 +926,10 @@ impl WTFStringImplStruct { } } #[inline] + pub fn is_thread_isolated(&self) -> bool { + WTFStringImpl__isThreadIsolated(self) + } + #[inline] pub fn is_thread_shareable(&self) -> bool { WTFStringImpl__isThreadShareable(self) } @@ -946,6 +950,7 @@ unsafe extern "C" { // `destroy` path crosses FFI. `*const` + `unsafe`: it frees the // allocation backing the pointer. pub fn Bun__WTFStringImpl__destroy(this: *const WTFStringImplStruct); + safe fn WTFStringImpl__isThreadIsolated(this: &WTFStringImplStruct) -> bool; safe fn WTFStringImpl__isThreadShareable(this: &WTFStringImplStruct) -> bool; safe fn Bun__WTFStringImpl__ensureHash(this: &WTFStringImplStruct); } diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index 7d1b63f8d9b1..dfd44065e6a9 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -517,7 +517,7 @@ impl String { } else { self.clone() }; - debug_assert!(copy.is_thread_shareable()); + debug_assert!(copy.is_thread_isolated()); copy } /// Make a WTF-backed impl safe to hand to any number of threads/VMs: @@ -529,22 +529,18 @@ impl String { } debug_assert!(self.is_thread_shareable()); } - /// True iff this `String` may be handed to another thread as-is: every - /// tag except `WTFStringImpl` is inert (raw slice / static / dead), and a - /// WTF-backed string qualifies iff its impl is not an atom, symbol or - /// substring (`isThreadShareable()`). - /// - /// Call sites that move a `String` across a thread boundary must ensure - /// this holds (typically via [`thread_isolated_copy`] first); see the - /// `Send`/`Sync` SAFETY comment for the full contract. + /// What [`thread_isolated_copy`] yields: no thread-affine state (not an + /// atom, symbol or substring), so the value may be handed to one other + /// thread. Non-WTF tags are inert and always qualify. + #[inline] + pub(crate) fn is_thread_isolated(&self) -> bool { + self.tag != Tag::WTFStringImpl || self.as_wtf().is_thread_isolated() + } + /// What [`make_thread_shareable`] yields: isolated, pre-hashed and never + /// atomized in place, so any number of threads may hold it. #[inline] pub(crate) fn is_thread_shareable(&self) -> bool { - if self.tag == Tag::WTFStringImpl { - // SAFETY: WTF tag guarantees `value.wtf` is a valid live impl. - self.as_wtf().is_thread_shareable() - } else { - true - } + self.tag != Tag::WTFStringImpl || self.as_wtf().is_thread_shareable() } #[inline] @@ -933,7 +929,7 @@ impl String { if out.string.tag == Tag::WTFStringImpl { if out.utf8.is_none() { // 8-bit all-ASCII: a thread-shareable `string` keeps backing the bytes. - if out.string.is_thread_shareable() { + if out.string.is_thread_isolated() { return out; } out.utf8 = Some(out.slice().to_vec()); @@ -998,11 +994,12 @@ impl Default for String { // in `Send + Sync` containers), and instead enforce the invariant at the // boundary: code that moves a `String` to one other thread calls // [`String::thread_isolated_copy`], code that shares one impl between -// threads/VMs calls [`String::make_thread_shareable`] (both assert -// [`String::is_thread_shareable`]). +// threads/VMs calls [`String::make_thread_shareable`] (asserting +// [`String::is_thread_isolated`] / [`String::is_thread_shareable`]). unsafe impl Send for String {} -// SAFETY: same contract as the `Send` impl above — sharing requires -// `is_thread_shareable()`; non-WTF tags are inert and trivially `Sync`. +// SAFETY: same contract as the `Send` impl above — a hand-off requires +// `is_thread_isolated()`, sharing `is_thread_shareable()`; non-WTF tags are +// inert and trivially `Sync`. unsafe impl Sync for String {} impl Drop for String { diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index 74f5e8fd2739..0148519bdf2f 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -799,14 +799,21 @@ extern "C" BunString BunString__createExternalGloballyAllocatedUTF16( return { BunStringTag::WTFStringImpl, { .wtf = &impl.leakRef() } }; } -// True iff the impl owns its buffer (or is external) and is not an atom, -// symbol or substring, so another thread may hold a ref. -extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadShareable( +// What isolatedCopy() yields: no atom-table membership and no base impl. +extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadIsolated( const WTF::StringImpl* wtf) { return !wtf->isAtom() && !wtf->isSymbol() && !wtf->isSubString(); } +// What BunString__makeThreadShareable yields: isolated, and no holder can +// mutate it (hash already computed, never atomized in place). +extern "C" [[ZIG_EXPORT(nothrow)]] bool WTFStringImpl__isThreadShareable( + const WTF::StringImpl* wtf) +{ + return WTFStringImpl__isThreadIsolated(wtf) && (wtf->isStatic() || (wtf->hasHash() && !wtf->canBecomeAtom())); +} + extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__WTFStringImpl__ensureHash(WTF::StringImpl* str) { str->hash(); From 1d9c6d0bb53e197e7aa48a306c7e651395df566a Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 23:17:51 +0000 Subject: [PATCH 12/25] makeThreadShareable: don't rewrite the never-atomize flag on an impl that already has it (other threads may hold it) --- src/jsc/bindings/BunString.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index 0148519bdf2f..42719ca2ef78 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -217,7 +217,8 @@ extern "C" [[ZIG_EXPORT(nothrow)]] void BunString__makeThreadShareable(BunString impl->deref(); } else if (!impl->isStatic()) { impl->hash(); - impl->setNeverAtomize(); + if (impl->canBecomeAtom()) + impl->setNeverAtomize(); } } From fbc53f9986168acb8c647ebedebf2d1b8ef72af3 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 23:36:20 +0000 Subject: [PATCH 13/25] review: hand-off family is ThreadIsolated/make_thread_isolated (one other thread) vs String::make_thread_shareable (many); async hand-off keeps main's zero-copy predicate; Bun::makeThreadShareable factored; File name always on the Blob; bake module keys shareable; test hook covers makeThreadShareable --- REVIEW.md | 2 +- src/CLAUDE.md | 44 +++---- src/bun_alloc/lib.rs | 10 ++ src/bun_core/string/mod.rs | 45 ++++--- src/js/internal-for-testing.ts | 6 + src/jsc/bindings/BunString.cpp | 43 +++---- src/jsc/bindings/BunString.h | 11 +- src/jsc/bindings/InternalForTesting.cpp | 37 ++++-- src/jsc/bindings/InternalForTesting.h | 1 + src/jsc/lib.rs | 4 +- src/jsc/node_path.rs | 76 ++++++++---- src/jsc/webcore_types.rs | 22 +--- src/runtime/api/BunObject.rs | 20 +-- src/runtime/api/JSBundler.rs | 4 +- src/runtime/api/JSTranspiler.rs | 8 +- src/runtime/api/standalone_graph_jsc.rs | 18 ++- src/runtime/bake/production.rs | 6 +- src/runtime/crypto/PBKDF2.rs | 8 +- src/runtime/jsc_hooks.rs | 100 ++++++++------- src/runtime/node/node_crypto_binding.rs | 10 +- src/runtime/node/node_fs.rs | 116 +++++++++--------- src/runtime/node/node_fs_binding.rs | 2 +- src/runtime/node/node_process.rs | 4 +- src/runtime/node/types.rs | 28 ++--- src/runtime/webcore/Blob.rs | 39 ++---- src/runtime/webcore/fetch/FetchTasklet.rs | 4 +- .../vm-thread-door.inventory.json | 2 +- ....ts => bunstring-thread-shareable.test.ts} | 32 ++--- 28 files changed, 356 insertions(+), 346 deletions(-) rename test/js/bun/util/{bunstring-thread-isolated-copy.test.ts => bunstring-thread-shareable.test.ts} (61%) diff --git a/REVIEW.md b/REVIEW.md index 265e589cc224..909abbbc5229 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -25,7 +25,7 @@ Several situational sections live in `.claude/docs/landing-prs.md` — read the - **Never let a pointer or slice outlive the memory it points into** (unsafe Rust, C++, FFI). Rejected shapes: slices of stack buffers; pointers into growable containers held across any call that can append; network/parser callback buffers stored without cloning (they are reused); `.data()` of a dead temporary; slices of small-string-optimized values; buffers handed to layers that store them past the call. If background threads reference stack state, every exit path must join them first. - **Root or copy every JSValue held beyond the current call.** WriteBarrier members declared in `.classes.ts` and visited in visitChildrenImpl (same change); Strong/protect only for justified self-keepalive; MarkedArgumentBuffer for values accumulated across slow calls — never raw JSValues in malloc'd memory or std containers. A Strong ref does not prevent ArrayBuffer detach; pin() is not a GC root; hasPendingActivity uses a counter; zero-copy toSlice-style helpers return borrowed views — consume synchronously or clone. Prove GC-safety with a stress test (thousands of iterations + `Bun.gc(true)`). Don't add Strong refs or ensureStillAlive you can't justify — and don't silently delete existing ones. - **Anything that can run user JS can synchronously free your state** — toString/valueOf, getters, Proxy traps, event emits, close(). Do all coercions first while holding no raw pointers; read mutable state (byteLength, typed-array vectors) once, after all observable side effects; re-validate liveness guards after every callback; copy-and-null stored one-shot callbacks before calling them; bracket entry points that can reach synchronous teardown with ref()/defer deref(); register state and listeners BEFORE the call that can trigger them; null member fields before calling close on a local copy. -- **Know the thread affinity of every line you touch.** JS-heap operations run only on the JS thread — marshal raw data and enqueue a task. Atomics for every shared counter (even metrics); a mutex only counts if EVERY accessor takes it; benign same-value races are still UB. Copy or `toThreadSafe` strings before another thread touches them; default to seq_cst and comment any weakened ordering. Cross-thread lifetime needs refcounts — a "finalized" boolean cannot prevent UAF. Never invoke callbacks while holding a non-recursive lock. Never back per-VM state with globals or thread-locals — workers share them. +- **Know the thread affinity of every line you touch.** JS-heap operations run only on the JS thread — marshal raw data and enqueue a task. Atomics for every shared counter (even metrics); a mutex only counts if EVERY accessor takes it; benign same-value races are still UB. `thread_isolated_copy()` a string before one other thread touches it, `make_thread_shareable()` before several VMs can reach it; default to seq_cst and comment any weakened ordering. Cross-thread lifetime needs refcounts — a "finalized" boolean cannot prevent UAF. Never invoke callbacks while holding a non-recursive lock. Never back per-VM state with globals or thread-locals — workers share them. - **Reference counts provably balanced on every terminal path** — success, error, cancellation, finalize. Map each ref to a named owner; take a ref only after a fallible enqueue succeeds; never use saturating arithmetic on counts; never add a ref just to silence ASAN — find the actual imbalance. The released ref may be the last one mid-callback; dropping the final reference while holding the object's own lock is UB. ## Correctness: the bug class, not the bug diff --git a/src/CLAUDE.md b/src/CLAUDE.md index e722ba3639f7..1bcb4d735d68 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -115,16 +115,16 @@ source is a bare `&[u8]`/`EncodedSlice` view. `Utf8Bytes<'a>` is `Borrowed(&'a [u8]) | Owned(Vec) | Shared(String)` (`Shared` holds an 8-bit all-ASCII WTF-backed `String` and reads its buffer); it derefs to `[u8]`; `is_owned()` ⇔ the bytes were transcoded/copied. -`Utf8WithString` (`String::into_utf8_with_string[_thread_shareable]()`) keeps the +`Utf8WithString` (`String::into_utf8_with_string[_thread_isolated]()`) keeps the UTF-8 bytes _and_ the source `String` so the value can go back to JS without re-encoding; `Utf8WithString::js_only(string)` wraps an output-only string. -`PathLike<'a>` / `StringOrBuffer<'a>` arms: `String`/`ThreadShareableString` +`PathLike<'a>` / `StringOrBuffer<'a>` arms: `String`/`ThreadIsolatedString` (`Utf8WithString` from a JS string), `Utf8(Utf8Bytes<'a>)` (transcoded JS string, or Rust-side bytes: `PathLike::borrowed(bytes)` lends `&'a [u8]` to a synchronous call, `PathLike::owned(vec)` when the value must own them), -`Buffer`. Anything -parsed from JS, stored, or sent to another thread (`make_thread_shareable`, -`ThreadShareable`, the fs `args::*<'static>` async path) is `'static`. +`Buffer`. Anything parsed from JS, stored, or sent to another thread +(`make_thread_isolated` / `ThreadIsolated` for the fs `args::*<'static>` +async path, `PathLike::make_thread_shareable` for a `Blob` store) is `'static`. `EncodedSlice<'a>` is the `{ptr, len}` + encoding-bits (Latin-1/UTF-8/UTF-16) borrowed view handed to C++. Constructors name the encoding of the bytes: @@ -384,22 +384,24 @@ non-transferring path UAFs at GC. ### Cross-thread string hazards -`StringImpl` refcounts are atomic, but atom tables are per thread: using a -string as a property key (`Identifier::fromString`) atomizes its impl _in -place_ into the current thread's table, and the last `deref()` of an atom -removes it from the current thread's table (`RELEASE_ASSERT(wasRemoved)` if it -is not there). So an atomizable impl must never be reachable from two VMs — -not via a process-global registry handing out `String::clone()`s, not via one -`SerializedScriptValue` deserialized by several receivers. Hand another thread -its own bytes (`Box<[u8]>` / `clone_utf8` on arrival), or share a -`String::make_thread_shareable()`d / `Bun::isolatedCopyForSharing` / -`toCrossThreadShareable` string (pre-hashed and marked never-atomize, so a -receiver's atom table copies it instead; see `src/jsc/bindings/BunString.cpp`) -or a static string — make it shareable once, then hand it out with plain -`clone()`. `String::thread_isolated_copy()` is a plain isolated copy for -handing a value to _one_ other owner. -`ObjectURLRegistry`, `StandaloneModuleGraph::File` and the structured-clone -object fast paths are the worked examples. +`StringImpl` refcounts are atomic; two things are per-thread: using a string as +a property key (`Identifier::fromString`) atomizes a non-atom impl _in place_ +into the current thread's atom table, and the last `deref()` of an atom removes +it from the _current_ thread's table (`RELEASE_ASSERT(wasRemoved)`). The lazily +computed hash/flags word is also unsynchronized. Rules: + +- Handing a value to one other thread (work pool, HTTP thread): + `String::thread_isolated_copy()`, `ThreadIsolated`, or own bytes + (`Box<[u8]>`, `clone_utf8` on arrival). +- Letting several VMs reach one impl (process-global registry, one + `SerializedScriptValue` with many receivers): `String::make_thread_shareable()` + (C++ `Bun::makeThreadShareable` / `isolatedCopyForSharing` / + `toCrossThreadShareable`) once — pre-hashed, never atomized in place, so each + receiver's atom table takes its own copy — then hand out plain `clone()`s. + Static strings already qualify. + +Worked examples: `ObjectURLRegistry`, `StandaloneModuleGraph::File`, the +structured-clone object fast paths. ## Common Patterns diff --git a/src/bun_alloc/lib.rs b/src/bun_alloc/lib.rs index c5f47132adc9..288aab179dff 100644 --- a/src/bun_alloc/lib.rs +++ b/src/bun_alloc/lib.rs @@ -806,6 +806,8 @@ impl WTFStringImplStruct { // These details must stay in sync with WTFStringImpl.h in WebKit! // --------------------------------------------------------------------- pub(crate) const S_HASH_FLAG_8BIT_BUFFER: u32 = 1 << 2; + pub(crate) const S_HASH_FLAG_STRING_KIND_IS_ATOM: u32 = 1 << 4; + pub(crate) const S_HASH_FLAG_STRING_KIND_IS_SYMBOL: u32 = 1 << 5; /// The bottom bit in the ref count indicates a static (immortal) string. pub(crate) const S_REF_COUNT_FLAG_IS_STATIC_STRING: u32 = 0x1; /// This allows us to ref / deref without disturbing the static string flag. @@ -926,6 +928,14 @@ impl WTFStringImplStruct { } } #[inline] + pub fn is_atom(&self) -> bool { + (self.m_hash_and_flags.get() & Self::S_HASH_FLAG_STRING_KIND_IS_ATOM) != 0 + } + #[inline] + pub fn is_symbol(&self) -> bool { + (self.m_hash_and_flags.get() & Self::S_HASH_FLAG_STRING_KIND_IS_SYMBOL) != 0 + } + #[inline] pub fn is_thread_isolated(&self) -> bool { WTFStringImpl__isThreadIsolated(self) } diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index dfd44065e6a9..b76ceba9d917 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -920,23 +920,13 @@ impl String { Utf8WithString { utf8, string: self } } - /// Like [`into_utf8_with_string`] but guarantees the resulting buffer is - /// safe to send to another thread. + /// [`into_utf8_with_string`] then [`Utf8WithString::make_thread_isolated`]. /// /// [`into_utf8_with_string`]: Self::into_utf8_with_string - pub fn into_utf8_with_string_thread_shareable(self) -> Utf8WithString { + #[inline] + pub fn into_utf8_with_string_thread_isolated(self) -> Utf8WithString { let mut out = self.into_utf8_with_string(); - if out.string.tag == Tag::WTFStringImpl { - if out.utf8.is_none() { - // 8-bit all-ASCII: a thread-shareable `string` keeps backing the bytes. - if out.string.is_thread_isolated() { - return out; - } - out.utf8 = Some(out.slice().to_vec()); - } - // Transcoded or copied; drop the WTF backing to release memory. - out.string = String::EMPTY; - } + out.make_thread_isolated(); out } @@ -1570,9 +1560,32 @@ impl Utf8WithString { (self.utf8, self.string) } - /// If `string` is WTF-backed, replace it with a thread-isolated copy. + /// For handing the value to one work-pool job that is dropped back on + /// the JS thread: keep a WTF-backed `string` only when it backs the bytes + /// and is not an atom/symbol, otherwise own the bytes and drop it. + pub fn make_thread_isolated(&mut self) { + if self.string.tag != Tag::WTFStringImpl { + return; + } + if self.utf8.is_none() { + let wtf = self.string.as_wtf(); + // The job only reads the bytes and the deref happens on the JS thread, so a plain/substring impl may stay. + if !wtf.is_atom() && !wtf.is_symbol() { + return; + } + self.utf8 = Some(self.slice().to_vec()); + } + self.string = String::EMPTY; + } + + /// For a holder that any thread may read or drop (a `Blob` store): as + /// [`make_thread_isolated`], and a kept `string` is made shareable so JS + /// can no longer atomize it in place. + /// + /// [`make_thread_isolated`]: Self::make_thread_isolated pub fn make_thread_shareable(&mut self) { - self.string = self.string.thread_isolated_copy(); + self.make_thread_isolated(); + self.string.make_thread_shareable(); } } diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 40e31c1508c4..a321d4e725db 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -664,6 +664,12 @@ export const BunString_threadIsolatedCopyRefCountDelta: () => number = $newCppFu 0, ); +export const BunString_makeThreadShareableRefCountDelta: () => number = $newCppFunction( + "InternalForTesting.cpp", + "jsFunction_BunString_makeThreadShareableRefCountDelta", + 0, +); + export const lowercaseHeaderNameSIMD: (name: string) => string = $newCppFunction( "InternalForTesting.cpp", "jsFunction_lowercaseHeaderNameSIMD", diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index 42719ca2ef78..7a570b8f9245 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -212,13 +212,10 @@ extern "C" [[ZIG_EXPORT(nothrow)]] void BunString__makeThreadShareable(BunString if (str->tag != BunStringTag::WTFStringImpl) return; auto* impl = str->impl.wtf; - if (impl->isAtom() || impl->isSymbol() || impl->isSubString()) { - str->impl.wtf = isolatedCopyForSharing(*impl).releaseImpl().leakRef(); + Ref shared = makeThreadShareable(*impl); + if (shared.ptr() != impl) { + str->impl.wtf = &shared.leakRef(); impl->deref(); - } else if (!impl->isStatic()) { - impl->hash(); - if (impl->canBecomeAtom()) - impl->setNeverAtomize(); } } @@ -303,7 +300,7 @@ static constexpr unsigned int kMinCrossThreadShareableLength = 256; // the receivers race the lazy m_hashAndFlags update (debug: ASSERT(!hasHash()) // in setHash; e.g. two workers switch()ing on the same BroadcastChannel // message). Static strings are immortal, pre-hashed and safe to share as-is. -WTF::String isolatedCopyForSharing(WTF::StringImpl& impl) +Ref isolatedCopyForSharing(const WTF::StringImpl& impl) { Ref copy = impl.isolatedCopy(); if (!copy->isStatic()) { @@ -313,29 +310,27 @@ WTF::String isolatedCopyForSharing(WTF::StringImpl& impl) return copy; } +Ref makeThreadShareable(WTF::StringImpl& impl) +{ + if (impl.isAtom() || impl.isSymbol() || impl.isSubString()) + return isolatedCopyForSharing(impl); + if (!impl.isStatic()) { + impl.hash(); + // Already set means other threads may hold this impl; don't write the flags word. + if (impl.canBecomeAtom()) + impl.setNeverAtomize(); + } + return impl; +} + WTF::String toCrossThreadShareable(const WTF::String& string) { auto* impl = string.impl(); if (!impl) return string; - - if (string.length() < kMinCrossThreadShareableLength) + if (impl->length() < kMinCrossThreadShareableLength) return isolatedCopyForSharing(*impl); - - // 1) Never share AtomStringImpl/symbols - they have special thread-unsafe behavior - if (impl->isAtom() || impl->isSymbol()) - return isolatedCopyForSharing(*impl); - - // 2) Don't share slices - if (impl->isSubString()) - return isolatedCopyForSharing(*impl); - - // 3) Ensure we won't lazily touch hash/flags on the consumer thread - // Force hash computation on this thread before sharing - const_cast(impl)->hash(); - const_cast(impl)->setNeverAtomize(); - - return string; + return makeThreadShareable(*impl); } } diff --git a/src/jsc/bindings/BunString.h b/src/jsc/bindings/BunString.h index 4a3ac65449d4..66b1017a5ed6 100644 --- a/src/jsc/bindings/BunString.h +++ b/src/jsc/bindings/BunString.h @@ -56,9 +56,12 @@ class UTF8View { } }; -WTF::String toCrossThreadShareable(const WTF::String& string); -// Always copies; the copy is pre-hashed and never atomized in place, so any -// number of threads may hold it and use it as a property key. -WTF::String isolatedCopyForSharing(WTF::StringImpl&); +// Pre-hashed and never atomized in place, so any number of threads may hold +// the result and use it as a property key. `isolatedCopyForSharing` always +// copies; `makeThreadShareable` copies only an atom/symbol/substring; +// `toCrossThreadShareable` also copies short strings so they stay atomizable. +Ref isolatedCopyForSharing(const WTF::StringImpl&); +Ref makeThreadShareable(WTF::StringImpl&); +WTF::String toCrossThreadShareable(const WTF::String&); } diff --git a/src/jsc/bindings/InternalForTesting.cpp b/src/jsc/bindings/InternalForTesting.cpp index 234f4766ad16..579424dc39c0 100644 --- a/src/jsc/bindings/InternalForTesting.cpp +++ b/src/jsc/bindings/InternalForTesting.cpp @@ -6,10 +6,12 @@ #include "JavaScriptCore/JSArrayBufferView.h" #include "headers-handwritten.h" #include "webcore/HTTPHeaderMap.h" +#include #include #include extern "C" BunString BunString__threadIsolatedCopy(const BunString* str); +extern "C" void BunString__makeThreadShareable(BunString* str); namespace Bun { @@ -59,27 +61,18 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_isASANEnabled, (JSC::JSGlobalObject * global #endif } -// Returns the net refcount change on the *original* StringImpl after a -// BunString owning one ref to it is passed through BunString__threadIsolatedCopy -// and then both are released. A correct implementation must return 0; a -// positive value means BunString__threadIsolatedCopy leaked a reference to the -// original StringImpl. +// Net refcount change on the *original* StringImpl after a BunString owning +// one ref to it goes through BunString__threadIsolatedCopy and both are +// released. 0 is correct; positive means the original was leaked. JSC_DEFINE_HOST_FUNCTION(jsFunction_BunString_threadIsolatedCopyRefCountDelta, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { - // Create a fresh, non-static, non-atom StringImpl with exactly one ref - // held by `original`. Ref original = WTF::String::fromLatin1("BunString__threadIsolatedCopy leak test").releaseImpl().releaseNonNull(); - const unsigned before = original->refCount(); - // Give the BunString its own ref, mirroring how a Rust-side bun.String - // owns one reference to the underlying StringImpl. original->ref(); BunString str = { BunStringTag::WTFStringImpl, { .wtf = original.ptr() } }; - BunString copy = BunString__threadIsolatedCopy(&str); - - ASSERT(copy.tag == BunStringTag::WTFStringImpl); + ASSERT(copy.tag == BunStringTag::WTFStringImpl && copy.impl.wtf != original.ptr()); copy.impl.wtf->deref(); str.impl.wtf->deref(); @@ -87,6 +80,24 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_BunString_threadIsolatedCopyRefCountDelta, ( return JSValue::encode(jsNumber(static_cast(after) - static_cast(before))); } +// Same for BunString__makeThreadShareable on an atom: it must swap in an +// isolated copy and release exactly the one ref the BunString held on the atom. +JSC_DEFINE_HOST_FUNCTION(jsFunction_BunString_makeThreadShareableRefCountDelta, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + Ref original = WTF::AtomStringImpl::add(WTF::String::fromLatin1("BunString__makeThreadShareable leak test").impl()).releaseNonNull(); + ASSERT(original->isAtom()); + const unsigned before = original->refCount(); + + original->ref(); + BunString str = { BunStringTag::WTFStringImpl, { .wtf = original.ptr() } }; + BunString__makeThreadShareable(&str); + ASSERT(str.tag == BunStringTag::WTFStringImpl && str.impl.wtf != original.ptr() && !str.impl.wtf->isAtom()); + str.impl.wtf->deref(); + + const unsigned after = original->refCount(); + return JSValue::encode(jsNumber(static_cast(after) - static_cast(before))); +} + extern "C" void Bun__MemoryPressure__emit(JSC::JSGlobalObject* global, int level); extern "C" bool Bun__MemoryPressure__isInstalled(JSC::JSGlobalObject* global); diff --git a/src/jsc/bindings/InternalForTesting.h b/src/jsc/bindings/InternalForTesting.h index 2b6c3bdfc4cc..462490b7d766 100644 --- a/src/jsc/bindings/InternalForTesting.h +++ b/src/jsc/bindings/InternalForTesting.h @@ -9,6 +9,7 @@ JSC_DECLARE_HOST_FUNCTION(jsFunction_arrayBufferViewHasBuffer); JSC_DECLARE_HOST_FUNCTION(jsFunction_hasReifiedStatic); JSC_DECLARE_HOST_FUNCTION(jsFunction_isASANEnabled); JSC_DECLARE_HOST_FUNCTION(jsFunction_BunString_threadIsolatedCopyRefCountDelta); +JSC_DECLARE_HOST_FUNCTION(jsFunction_BunString_makeThreadShareableRefCountDelta); JSC_DECLARE_HOST_FUNCTION(jsFunction_lowercaseHeaderNameSIMD); JSC_DECLARE_HOST_FUNCTION(jsFunction_emitMemoryPressure); JSC_DECLARE_HOST_FUNCTION(jsFunction_isMemoryPressureWatcherInstalled); diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index 11c8d69b0123..05b0b17c80e2 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -1251,9 +1251,9 @@ pub use self::array_buffer::JSTypedArrayBytesDeallocator; pub mod node_path; #[path = "webcore_types.rs"] pub mod webcore_types; -// RAII pair for `make_thread_shareable()`/`unprotect()` — re-exported at crate root +// RAII pair for `make_thread_isolated()`/`unprotect()` — re-exported at crate root // so `bun_runtime` callers don't reach through `node_path`. -pub use self::node_path::{ThreadShareable, Unprotect}; +pub use self::node_path::{ThreadIsolated, Unprotect}; /// `jsc.WebCore` (deprecated alias) — only the data-shape subset /// that was hoisted to this tier. Reach for `bun_runtime::webcore` for the diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index dc3037d174e2..8aa4f1334796 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -13,37 +13,37 @@ use bun_sys::Fd; use crate::array_buffer::MarkedArrayBuffer; // ────────────────────────────────────────────────────────────────────────── -// RAII for `protect()`/`unprotect()` pairs taken by `make_thread_shareable()`. +// RAII for `protect()`/`unprotect()` pairs taken by `make_thread_isolated()`. // -// The async-fs path calls `make_thread_shareable()` (which `JSValue::protect()`s +// The async-fs path calls `make_thread_isolated()` (which `JSValue::protect()`s // any borrowed JS-backed buffers so the work-pool thread may read them) and // must later release them. The "deinit" half is // already `Drop`; only the JS-side `unprotect()` needs an explicit hook, and // pairing it with the protect via a guard type removes the leak hazard on -// every early return between `make_thread_shareable` and the manual cleanup. +// every early return between `make_thread_isolated` and the manual cleanup. // ────────────────────────────────────────────────────────────────────────── -/// Undo the `JSValue::protect()` calls taken by [`make_thread_shareable`]( -/// PathLike::make_thread_shareable) (or an `args::*` type's `make_thread_shareable`). +/// Undo the `JSValue::protect()` calls taken by [`make_thread_isolated`]( +/// PathLike::make_thread_isolated) (or an `args::*` type's `make_thread_isolated`). /// /// Implementations release **only** the JS-GC protect refcount — owned Rust /// payloads (Vec, `Utf8WithString`, …) are freed by the type's own /// `Drop`, which runs immediately after when the value is held in a -/// [`ThreadShareable`]. +/// [`ThreadIsolated`]. pub trait Unprotect { fn unprotect(&mut self); } -/// RAII guard returned by `into_thread_shareable()`: a `T` whose JS-backed buffers +/// RAII guard returned by `into_thread_isolated()`: a `T` whose JS-backed buffers /// have been `protect()`ed. `Drop` calls [`Unprotect::unprotect`] then drops /// the inner `T` normally. /// /// `repr(transparent)` so identity-casts in the const-generic dispatch macros /// (see `node_fs.rs`'s `args_as!`) remain bit-exact. #[repr(transparent)] -pub struct ThreadShareable(T); +pub struct ThreadIsolated(T); -impl ThreadShareable { +impl ThreadIsolated { /// Wrap an **already-protected** `T`. Use when the protect was taken /// elsewhere (e.g. inside `from_js_maybe_async(.., Flavor::Async, ..)`). #[inline] @@ -56,9 +56,9 @@ impl ThreadShareable { // GC-protected for as long as it is held, so a pool job may read them (under // its `Ticket`, which keeps the VM alive); the job comes back to the JS // thread, where this is dropped and the protection released. -unsafe impl Send for ThreadShareable {} +unsafe impl Send for ThreadIsolated {} -impl core::ops::Deref for ThreadShareable { +impl core::ops::Deref for ThreadIsolated { type Target = T; #[inline] fn deref(&self) -> &T { @@ -66,14 +66,14 @@ impl core::ops::Deref for ThreadShareable { } } -impl core::ops::DerefMut for ThreadShareable { +impl core::ops::DerefMut for ThreadIsolated { #[inline] fn deref_mut(&mut self) -> &mut T { &mut self.0 } } -impl Drop for ThreadShareable { +impl Drop for ThreadIsolated { #[inline] fn drop(&mut self) { // The same argument types serve mini-loop threads (the shell's `cp` via @@ -87,14 +87,14 @@ impl Drop for ThreadShareable { } } -impl Default for ThreadShareable { +impl Default for ThreadIsolated { #[inline] fn default() -> Self { Self(T::default()) } } -// `ThreadShareable` crosses to the work-pool thread; auto-`Send` iff `T: Send`. +// `ThreadIsolated` crosses to the work-pool thread; auto-`Send` iff `T: Send`. /// `node.PathLike`. Parsed from JS it is `PathLike<'static>`; `Utf8` may /// instead borrow Rust-side bytes for a synchronous call ([`PathLike::borrowed`]). @@ -103,7 +103,7 @@ pub enum PathLike<'a> { /// Always shares its WTF string's bytes (built by `shared_or_utf8`); /// transcoded paths are `Utf8(Owned)`. String(Utf8WithString), - ThreadShareableString(Utf8WithString), + ThreadIsolatedString(Utf8WithString), Utf8(Utf8Bytes<'a>), } @@ -128,7 +128,7 @@ impl Clone for PathLike<'_> { pinned: false, }), Self::String(s) => Self::String(s.clone()), - Self::ThreadShareableString(s) => Self::ThreadShareableString(s.clone()), + Self::ThreadIsolatedString(s) => Self::ThreadIsolatedString(s.clone()), Self::Utf8(s) => Self::Utf8(s.clone()), } } @@ -143,7 +143,7 @@ impl Drop for PathLike<'_> { b.buffer.unpin(); } } - Self::String(_) | Self::ThreadShareableString(_) | Self::Utf8(_) => {} + Self::String(_) | Self::ThreadIsolatedString(_) | Self::Utf8(_) => {} } } } @@ -171,7 +171,7 @@ impl<'a> PathLike<'a> { pub fn slice(&self) -> &[u8] { match self { Self::Buffer(b) => b.slice(), - Self::String(s) | Self::ThreadShareableString(s) => s.slice(), + Self::String(s) | Self::ThreadIsolatedString(s) => s.slice(), Self::Utf8(s) => s.slice(), } } @@ -179,7 +179,7 @@ impl<'a> PathLike<'a> { pub(crate) fn estimated_size(&self) -> usize { match self { Self::Buffer(b) => b.slice().len(), - Self::String(_) | Self::ThreadShareableString(_) => 0, + Self::String(_) | Self::ThreadIsolatedString(_) => 0, Self::Utf8(s) => s.slice().len(), } } @@ -187,31 +187,47 @@ impl<'a> PathLike<'a> { impl PathLike<'static> { /// Promote any borrowed-JS - /// payload to a thread-shareable representation. For `Buffer` the variant is + /// payload to a thread-isolated representation. For `Buffer` the variant is /// kept and the backing JS value is `protect()`ed (paired with /// [`Unprotect::unprotect`]); the discriminant is preserved so callers /// matching on `Buffer` after this call see the same shape. /// - /// Called in place by the fs `args::*` types' `into_thread_shareable`, which - /// wrap the result in a [`ThreadShareable`] guard. - pub fn make_thread_shareable(&mut self) { + /// Called in place by the fs `args::*` types' `into_thread_isolated`, which + /// wrap the result in a [`ThreadIsolated`] guard. + pub fn make_thread_isolated(&mut self) { match self { Self::String(s) => { + s.make_thread_isolated(); + let owned = core::mem::take(s); + *self = Self::ThreadIsolatedString(owned); + } + Self::Buffer(b) => { + b.buffer.value.protect(); + } + Self::ThreadIsolatedString(_) | Self::Utf8(_) => {} + } + } + + /// As [`make_thread_isolated`](Self::make_thread_isolated), for a path a + /// `Blob` store keeps (read and dropped on any thread). + pub fn make_thread_shareable(&mut self) { + match self { + Self::String(s) | Self::ThreadIsolatedString(s) => { s.make_thread_shareable(); let owned = core::mem::take(s); - *self = Self::ThreadShareableString(owned); + *self = Self::ThreadIsolatedString(owned); } Self::Buffer(b) => { b.buffer.value.protect(); } - Self::ThreadShareableString(_) | Self::Utf8(_) => {} + Self::Utf8(_) => {} } } } impl Unprotect for PathLike<'static> { /// JS-side half of cleanup — undo - /// the `protect()` taken by [`Self::make_thread_shareable`] / + /// the `protect()` taken by [`Self::make_thread_isolated`] / /// `ArgumentsSlice::protect_eat`. Owned payloads are released by `Drop`. #[inline] fn unprotect(&mut self) { @@ -263,6 +279,12 @@ impl PathOrFileDescriptorSerializeTag { } impl PathOrFileDescriptor<'static> { + #[inline] + pub fn make_thread_isolated(&mut self) { + if let Self::Path(p) = self { + p.make_thread_isolated(); + } + } #[inline] pub fn make_thread_shareable(&mut self) { if let Self::Path(p) = self { diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index 667f84ca71f2..dc9ea8d67918 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -139,15 +139,11 @@ pub struct Blob { pub name: bun_ptr::JsCell, } -// SAFETY: a `Blob` reaches another thread only as an `ObjectURLRegistry` -// entry or standalone-graph template (null `global_this`, `name` made -// `String::make_thread_shareable()`d so dupes clone it into any VM; each dupe -// is stamped with the resolving thread's global) or via the work-pool -// read/write tasks that hand it back to the owning JS thread. `store` (`StoreRef`) and -// `content_type` are atomically refcounted; a shared `Store`'s `is_all_ascii` -// is atomic, a `Bytes` store is not otherwise written after construction, and -// a `File` store's stat fields (`seekable`, `max_size`, `mode`, -// `last_modified`) are lazily cached by whichever dupe stats it. +// SAFETY: `global_this` is dereferenced only on its owning JS thread; blobs +// that cross threads (object-URL registry entries, the standalone-graph +// template) carry a null global and a `make_thread_shareable()`d `name`; +// `store` (`RefPtr`) / `content_type` are atomically refcounted and +// `Store::is_all_ascii` is atomic. unsafe impl Send for Blob {} // SAFETY: concurrent `&Blob` access only occurs under `ObjectURLRegistry`'s // mutex, on the standalone-graph template (never written after its `OnceLock` @@ -727,14 +723,6 @@ pub mod store { } } - #[inline] - pub fn init_empty_with_name(name: Box<[u8]>) -> Bytes { - Bytes { - stored_name: name, - ..Default::default() - } - } - #[inline] pub fn allocator(&self) -> bun_alloc::StdAllocator { self.allocator diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 953e0d9c5b60..0b1152b6b6d6 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1878,19 +1878,9 @@ fn get_is_standalone_executable(global_this: &JSGlobalObject, _: &JSObject) -> J fn get_embedded_files(global_this: &JSGlobalObject, _: &JSObject) -> JsResult { use crate::webcore::blob::{Blob, BlobExt as _}; use bun_standalone_graph::{File as GraphFile, Graph as StandaloneModuleGraph}; - // SAFETY: bun_vm() returns the live thread-local VM for a Bun-owned global. - let vm = global_this.bun_vm(); - if vm.standalone_module_graph.is_none() { + let Some(graph) = StandaloneModuleGraph::get_ref() else { return JSValue::create_empty_array(global_this, 0); - } - // NOTE (layering): `VirtualMachine.standalone_module_graph` is - // type-erased to `&dyn bun_resolver::StandaloneModuleGraph` so `bun_jsc` - // doesn't depend on `bun_standalone_graph`. The concrete graph is the - // process singleton — `Graph::get()` returns the same instance the trait - // object was built from (`vm.standalone_module_graph.is_some()` ⇔ - // `Graph::get().is_some()`). - let graph: &StandaloneModuleGraph = StandaloneModuleGraph::get_ref() - .expect("vm.standalone_module_graph set ⇔ Graph singleton populated"); + }; let unsorted_files = graph.files.values(); let mut sort_indices: Vec = Vec::with_capacity(unsorted_files.len()); @@ -2775,8 +2765,8 @@ pub mod JSZstd { /// `Bun.zstdCompress` / `Bun.zstdDecompress` off the JS thread. pub(crate) struct ZstdJob { /// Created with `Flavor::Async` (JS-backed buffer protected); the - /// [`bun_jsc::ThreadShareable`] releases that with the job. - pub buffer: bun_jsc::ThreadShareable>, + /// [`bun_jsc::ThreadIsolated`] releases that with the job. + pub buffer: bun_jsc::ThreadIsolated>, pub is_compress: bool, pub level: i32, /// Filled in by `run`. @@ -2833,7 +2823,7 @@ pub mod JSZstd { jsc::Job::::schedule( &cx, ZstdJob { - buffer: bun_jsc::ThreadShareable::adopt(buffer), + buffer: bun_jsc::ThreadIsolated::adopt(buffer), is_compress, level, result: Ok(Box::default()), diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index 5e10e1f2fde3..966b8da0b353 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -78,12 +78,12 @@ pub mod js_bundler { while let Some((prop, property_value)) = files_iter.next()? { // Parse the value as BlobOrStringOrBuffer using async mode for thread safety. // Async mode `protect()`s any JS-backed buffer; adopt into a - // `ThreadShareable` so the guard unprotects + drops at end of iteration. + // `ThreadIsolated` so the guard unprotects + drops at end of iteration. let blob_or_string = match crate::node::BlobOrStringOrBuffer::from_js_async( global_this, property_value, )? { - Some(v) => bun_jsc::ThreadShareable::adopt(v), + Some(v) => bun_jsc::ThreadIsolated::adopt(v), None => { return Err(global_this.throw_invalid_arguments(format_args!("Expected file content to be a string, Blob, File, TypedArray, or ArrayBuffer"))); } diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index 01e588d41812..731825fb9d76 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -633,7 +633,7 @@ impl Config { /// into the owning `JSTranspiler`'s config (its `Transpiler` is bit-copied), /// which the job's Js side keeps alive and the pool borrow keeps valid. pub(crate) struct TransformTask { - pub input_code: bun_jsc::ThreadShareable>, + pub input_code: bun_jsc::ThreadIsolated>, pub output_code: BunString, pub transpiler: core::mem::ManuallyDrop>, pub log: bun_ast::Log, @@ -673,7 +673,7 @@ impl TransformTask { fn schedule( transpiler: &JSTranspiler, transpiler_js: JSValue, - input_code: bun_jsc::ThreadShareable>, + input_code: bun_jsc::ThreadIsolated>, global: &JSGlobalObject, loader: Loader, ) -> JSValue { @@ -1383,9 +1383,9 @@ impl JSTranspiler { code = StringOrBuffer::owned(bytes); } // `errdefer code.deinitAndUnprotect()` — `from_js_with_encoding_maybe_async` - // (`Flavor::Async`) already protected; adopt into a `ThreadShareable` so any + // (`Flavor::Async`) already protected; adopt into a `ThreadIsolated` so any // early-return drop unprotects. `TransformTask::create` takes the guard. - let code = bun_jsc::ThreadShareable::adopt(code); + let code = bun_jsc::ThreadIsolated::adopt(code); args.eat(); let loader: Option = 'brk: { diff --git a/src/runtime/api/standalone_graph_jsc.rs b/src/runtime/api/standalone_graph_jsc.rs index af1b49196de7..2d10277d25e3 100644 --- a/src/runtime/api/standalone_graph_jsc.rs +++ b/src/runtime/api/standalone_graph_jsc.rs @@ -14,9 +14,9 @@ use crate::webcore::blob::SizeType; use crate::webcore::blob::store::{Bytes, Data, IsAllAscii, Store}; use bun_standalone_graph::File; -/// The process-wide part of an embedded file's `Blob`: store, content type -/// and a shared `name`. `global_this` is null; `file_blob` stamps it per VM. -fn shared_blob(file: &File) -> NonNull { +/// The process-wide template of an embedded file's `Blob`: store, content +/// type and a shared `name`. `global_this` is null; `file_blob` stamps it per VM. +fn template_blob(file: &File) -> NonNull { *file.cached_blob.get_or_init(|| { // `contents` is a `'static` slice into the embedded executable // section — borrow it directly (no copy) and hand it to a `Bytes` @@ -27,7 +27,7 @@ fn shared_blob(file: &File) -> NonNull NonNull NonNull NonNull Blob { - // SAFETY: `shared_blob` returns the pointer from `Blob::new`, never + // SAFETY: `template_blob` returns the pointer from `Blob::new`, never // freed for the process lifetime; only read here. - let blob = unsafe { shared_blob(file).cast::().as_ref() }.dupe_with_content_type(true); + let blob = unsafe { template_blob(file).cast::().as_ref() }.dupe_with_content_type(true); blob.global_this.set(global); blob } diff --git a/src/runtime/bake/production.rs b/src/runtime/bake/production.rs index 2653ace4c53f..f9cf26b91072 100644 --- a/src/runtime/bake/production.rs +++ b/src/runtime/bake/production.rs @@ -743,12 +743,12 @@ fn build_with_vm(ctx: Context, cwd: &[u8], pt: &mut PerThread) -> crate::Result< if let Some(entry_point_index) = file.entry_point_index { if (entry_point_index as usize) < module_keys.len() { - let str = BunString::create_format(format_args!( + let mut str = BunString::create_format(format_args!( "bake:/{}", BStr::new(without_prefix) )); - module_keys[entry_point_index as usize] = - str.thread_isolated_copy(); + str.make_thread_shareable(); + module_keys[entry_point_index as usize] = str; } } diff --git a/src/runtime/crypto/PBKDF2.rs b/src/runtime/crypto/PBKDF2.rs index 202e578d46a5..5149c5b40028 100644 --- a/src/runtime/crypto/PBKDF2.rs +++ b/src/runtime/crypto/PBKDF2.rs @@ -61,7 +61,7 @@ impl PBKDF2 { // `password`/`salt` are `StringOrBuffer` whose `Drop` releases the // slice/WTF ref, so no explicit cleanup hook is needed — // dropping `PBKDF2` is sufficient for the sync path. The async path holds - // `ThreadShareable`, whose `Drop` additionally unprotects JS-rooted + // `ThreadIsolated`, whose `Drop` additionally unprotects JS-rooted // buffers via the `Unprotect` impl below. /// The second element is the validated callback on the `Async` flavor and @@ -267,8 +267,8 @@ impl bun_jsc::Unprotect for PBKDF2 { /// `crypto.pbkdf2` off the JS thread. pub(crate) struct Pbkdf2Job { /// `from_js(.., Flavor::Async)` protected the input buffers; the - /// [`bun_jsc::ThreadShareable`] releases that with the job. - pub pbkdf2: bun_jsc::ThreadShareable, + /// [`bun_jsc::ThreadIsolated`] releases that with the job. + pub pbkdf2: bun_jsc::ThreadIsolated, pub output: Vec, pub err: bool, } @@ -343,7 +343,7 @@ pub(crate) fn create_job(global_this: &JSGlobalObject, data: PBKDF2, callback: J &cx, Pbkdf2Job { // `from_js(.., Flavor::Async)` already protected — adopt, don't re-protect. - pbkdf2: bun_jsc::ThreadShareable::adopt(data), + pbkdf2: bun_jsc::ThreadIsolated::adopt(data), output: Vec::new(), err: false, }, diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 07e5e7799c69..70925e0b1d41 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -3748,21 +3748,18 @@ fn __bun_fetch_builtin_module( } // ── Standalone-module-graph probe ─────────────────────────────────── - // The VM field is the resolver's type-erased `&dyn StandaloneModuleGraph`; - // the concrete `Graph` is the sole implementor. - if jsc_vm.standalone_module_graph.is_some() { - let graph = bun_standalone_graph::Graph::get_ref() - .expect("vm.standalone_module_graph set ⇔ Graph singleton populated"); - // `files.get` (no virtual-root prefix check), not `find_ref`. - if let Some(file) = graph.files.get(spec) { - use bun_standalone_graph::StandaloneModuleGraph::ModuleFormat; - - if matches!(file.loader, Loader::Sqlite | Loader::SqliteEmbedded) { - // Distinct from - // [`SQLITE_MODULE_SOURCE`]: the standalone-binary path reads - // the embedded blob via `readFileSync(import.meta.path)` - // (resolved through the `/$bunfs/` virtual root). - const SQLITE_MODULE_SOURCE_STANDALONE: &[u8] = b"\ + // `files.get` (no virtual-root prefix check), not `find_ref`. + if let Some(file) = + bun_standalone_graph::Graph::get_ref().and_then(|graph| graph.files.get(spec)) + { + use bun_standalone_graph::StandaloneModuleGraph::ModuleFormat; + + if matches!(file.loader, Loader::Sqlite | Loader::SqliteEmbedded) { + // Distinct from + // [`SQLITE_MODULE_SOURCE`]: the standalone-binary path reads + // the embedded blob via `readFileSync(import.meta.path)` + // (resolved through the `/$bunfs/` virtual root). + const SQLITE_MODULE_SOURCE_STANDALONE: &[u8] = b"\ /* Generated code */ import {Database} from 'bun:sqlite'; import {readFileSync} from 'node:fs'; @@ -3771,48 +3768,47 @@ export const db = new Database(readFileSync(import.meta.path)); export const __esModule = true; export default db; "; - return Some(ResolvedSource { - source_code: bun_core::String::static_(SQLITE_MODULE_SOURCE_STANDALONE), - source_url: specifier.clone(), - ..ResolvedSource::default() - }); - } - - if file.is_text_module() { - // The bytes are already a string body (`encode_text_module`): - // the default export is a JSString over the section, no copy. - // Only a `Dead` string throws; `to_wtf_string` never yields one. - let value = file - .to_wtf_string() - .into_js(global) - .expect("embedded text module string is never dead"); - return Some(ResolvedSource { - jsvalue_for_export: value, - tag: bun_jsc::resolved_source::Tag::ExportDefaultObject, - ..ResolvedSource::default() - }); - } - - // SAFETY: `file.module_info`/`file.bytecode` are live subranges of - // the embedded section (set in `Graph::from_bytes`); - // `create_from_cached_record` copies out of it before returning. - let (module_info, bytecode) = unsafe { (&*file.module_info, &*file.bytecode) }; return Some(ResolvedSource { - source_code: file.to_wtf_string(), + source_code: bun_core::String::static_(SQLITE_MODULE_SOURCE_STANDALONE), source_url: specifier.clone(), - bytecode_origin_path: bun_core::String::from_bytes(file.bytecode_origin_path), - bytecode_cache: Bytecode::persistent(bytecode), - source_code_hash: file.source_hash, - module_info: if !module_info.is_empty() { - bun_bundler::analyze_transpiled_module::ModuleInfoDeserialized - ::create_from_cached_record(module_info) - } else { - None - }, - is_commonjs_module: file.module_format == ModuleFormat::Cjs, ..ResolvedSource::default() }); } + + if file.is_text_module() { + // The bytes are already a string body (`encode_text_module`): + // the default export is a JSString over the section, no copy. + // Only a `Dead` string throws; `to_wtf_string` never yields one. + let value = file + .to_wtf_string() + .into_js(global) + .expect("embedded text module string is never dead"); + return Some(ResolvedSource { + jsvalue_for_export: value, + tag: bun_jsc::resolved_source::Tag::ExportDefaultObject, + ..ResolvedSource::default() + }); + } + + // SAFETY: `file.module_info`/`file.bytecode` are live subranges of + // the embedded section (set in `Graph::from_bytes`); + // `create_from_cached_record` copies out of it before returning. + let (module_info, bytecode) = unsafe { (&*file.module_info, &*file.bytecode) }; + return Some(ResolvedSource { + source_code: file.to_wtf_string(), + source_url: specifier.clone(), + bytecode_origin_path: bun_core::String::from_bytes(file.bytecode_origin_path), + bytecode_cache: Bytecode::persistent(bytecode), + source_code_hash: file.source_hash, + module_info: if !module_info.is_empty() { + bun_bundler::analyze_transpiled_module::ModuleInfoDeserialized + ::create_from_cached_record(module_info) + } else { + None + }, + is_commonjs_module: file.module_format == ModuleFormat::Cjs, + ..ResolvedSource::default() + }); } None diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index 6388c005e19f..2efd3ad47fcf 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -755,9 +755,9 @@ pub mod random { // Scrypt // ─────────────────────────────────────────────────────────────────────────── pub(crate) struct Scrypt { - // Plain `StringOrBuffer` — NOT `ThreadShareable<_>`. The struct serves both + // Plain `StringOrBuffer` — NOT `ThreadIsolated<_>`. The struct serves both // `scryptSync` (no protect taken) and async `scrypt` (protect taken in - // `from_js_maybe_async(.., Flavor::Async, ..)`, adopted into a `ThreadShareable` + // `from_js_maybe_async(.., Flavor::Async, ..)`, adopted into a `ThreadIsolated` // by the job). password: StringOrBuffer<'static>, salt: StringOrBuffer<'static>, @@ -1087,7 +1087,7 @@ mod _impl { impl bun_jsc::Unprotect for Scrypt { /// Release the `protect()` taken by `from_js_maybe_async(.., Flavor::Async, ..)` - /// on the async path (via the job's `ThreadShareable`). The sync path never calls this. + /// on the async path (via the job's `ThreadIsolated`). The sync path never calls this. #[inline] fn unprotect(&mut self) { bun_jsc::Unprotect::unprotect(&mut self.password); @@ -1098,7 +1098,7 @@ mod _impl { /// `crypto.scrypt` off the JS thread: derives straight into the result /// ArrayBuffer's bytes under the job's ticket, which keeps their VM alive. pub(crate) struct ScryptJob { - params: bun_jsc::ThreadShareable, + params: bun_jsc::ThreadIsolated, result: JsPtr<[u8]>, err: Option, } @@ -1307,7 +1307,7 @@ mod _impl { fn scrypt(global: &JSGlobalObject, call_frame: &CallFrame) -> JsResult { let (ctx, callback) = Scrypt::from_js::(global, call_frame)?; // Protected by `from_js::`; released with the job wherever it ends. - let params = bun_jsc::ThreadShareable::adopt(ctx); + let params = bun_jsc::ThreadIsolated::adopt(ctx); if params.keylen as usize > jsc::virtual_machine::synthetic_allocation_limit() { return Err(global.throw_out_of_memory()); } diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 592d8c7045fe..aacaa6d4a342 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -17,7 +17,7 @@ use bun_jsc::AbortSignal; use bun_jsc::debugger::AsyncTaskTracker; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ - EventLoopHandle, JSGlobalObject, JSValue, JsResult, StringJsc as _, ThreadShareable, Unprotect, + EventLoopHandle, JSGlobalObject, JSValue, JsResult, StringJsc as _, ThreadIsolated, Unprotect, }; use bun_paths::{self as paths, OSPathBuffer, OSPathChar, OSPathSliceZ, PathBuffer}; use bun_sys::FdExt as _; @@ -640,8 +640,8 @@ mod _async_tasks { #[cfg(windows)] pub struct UVFSRequest { pub(crate) promise: JSPromiseStrong, - /// Wrapped in [`ThreadShareable`] so the paired `unprotect()` runs on drop. - pub args: ThreadShareable, + /// Wrapped in [`ThreadIsolated`] so the paired `unprotect()` runs on drop. + pub args: ThreadIsolated, pub(crate) global_object: bun_ptr::BackRef, pub(crate) req: uv::fs_t, pub(crate) result: Maybe, @@ -671,7 +671,7 @@ mod _async_tasks { ) -> JSValue { let task = Box::new(Self { promise: JSPromiseStrong::init(global_object), - args: task_args.into_thread_shareable(), + args: task_args.into_thread_isolated(), // Sentinel — overwritten by `uv_callback` (or the early-return arms // below) before any read on the JS thread. `Maybe` is // `Result` and may be niche-optimised for arbitrary @@ -704,7 +704,7 @@ mod _async_tasks { ($Args:ty) => {{ debug_assert_eq!(core::mem::size_of::(), core::mem::size_of::<$Args>()); // SAFETY: identity cast — `A == $Args` for this `F` (see `async_::*`). - // `ThreadShareable` is `repr(transparent)`; deref through it for the inner `A`. + // `ThreadIsolated` is `repr(transparent)`; deref through it for the inner `A`. unsafe { &*(&*task.args as *const A as *const $Args) } }}; } @@ -766,7 +766,7 @@ mod _async_tasks { let buf = &buf[..buf.len().min(args.length as usize)]; let bufs = [uv::uv_buf_t::init(buf)]; // SAFETY: libuv copies the iovec descriptor before return; the - // backing Buffer is JS-protected via `make_thread_shareable`. + // backing Buffer is JS-protected via `make_thread_isolated`. let rc = unsafe { uv::uv_fs_read( loop_, @@ -994,9 +994,9 @@ mod _async_tasks { // NewAsyncFSTask — runs a NodeFS method on the thread pool. // ────────────────────────────────────────────────────────────────────────── - /// Trait abstracting over Argument types' deinit/make_thread_shareable. + /// Trait abstracting over Argument types' deinit/make_thread_isolated. /// - /// Every Arguments struct defines `make_thread_shareable` (clone + /// Every Arguments struct defines `make_thread_isolated` (clone /// any borrowed JS-backed slices so the work-pool callback may run off-thread). /// The trait methods are **required** so missing impls are a compile error rather /// than a silent UAF/leak. @@ -1007,36 +1007,36 @@ mod _async_tasks { /// `from_js`; the trait forwards to it so the generic `Bindings` in /// `node_fs_binding.rs` can call it without per-type macro arms. fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult; - fn make_thread_shareable(&mut self); + fn make_thread_isolated(&mut self); /// Consume `self`, protect any JS-backed buffers, and return a guard that /// unprotects on drop — /// string/slice ownership is handled by each field's `Drop` (PathLike, /// StringOrBuffer, Vec); only the JS-side `unprotect()` needs the guard. #[inline] - fn into_thread_shareable(mut self) -> ThreadShareable { - self.make_thread_shareable(); - ThreadShareable::adopt(self) + fn into_thread_isolated(mut self) -> ThreadIsolated { + self.make_thread_isolated(); + ThreadIsolated::adopt(self) } fn signal(&self) -> Option<&AbortSignal> { None } } - /// Forward [`FsArgument`] to the inherent `from_js` / `make_thread_shareable` + /// Forward [`FsArgument`] to the inherent `from_js` / `make_thread_isolated` /// methods each `args::*` struct already defines. /// [`Unprotect`] is implemented per-type alongside. macro_rules! impl_fs_argument { ( $( $ty:ty ),+ $(,)? ) => { $( impl FsArgument for $ty { #[inline] fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { <$ty>::from_js(ctx, arguments) } - #[inline] fn make_thread_shareable(&mut self) { <$ty>::make_thread_shareable(self) } + #[inline] fn make_thread_isolated(&mut self) { <$ty>::make_thread_isolated(self) } } )+ }; - // Fd-only types — `make_thread_shareable` is a no-op (these hold only `FD`/scalars). + // Fd-only types — `make_thread_isolated` is a no-op (these hold only `FD`/scalars). ( @fd $( $ty:ty ),+ $(,)? ) => { $( impl FsArgument for $ty { #[inline] fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { <$ty>::from_js(ctx, arguments) } - #[inline] fn make_thread_shareable(&mut self) { <$ty>::make_thread_shareable(self) } + #[inline] fn make_thread_isolated(&mut self) { <$ty>::make_thread_isolated(self) } } impl Unprotect for $ty { #[inline] fn unprotect(&mut self) {} @@ -1085,8 +1085,8 @@ mod _async_tasks { args::ReadFile::from_js(ctx, arguments) } #[inline] - fn make_thread_shareable(&mut self) { - args::ReadFile::make_thread_shareable(self) + fn make_thread_isolated(&mut self) { + args::ReadFile::make_thread_isolated(self) } #[inline] fn signal(&self) -> Option<&AbortSignal> { @@ -1100,8 +1100,8 @@ mod _async_tasks { args::WriteFile::from_js(ctx, arguments) } #[inline] - fn make_thread_shareable(&mut self) { - args::WriteFile::make_thread_shareable(self) + fn make_thread_isolated(&mut self) { + args::WriteFile::make_thread_isolated(self) } #[inline] fn signal(&self) -> Option<&AbortSignal> { @@ -1116,8 +1116,8 @@ mod _async_tasks { .map(args::AppendFile) } #[inline] - fn make_thread_shareable(&mut self) { - self.0.make_thread_shareable(); + fn make_thread_isolated(&mut self) { + self.0.make_thread_isolated(); } #[inline] fn signal(&self) -> Option<&AbortSignal> { @@ -1227,13 +1227,13 @@ mod _async_tasks { } /// One `fs.promises.*` operation on the work pool. The arguments' JS-backed - /// buffers are protected (`ThreadShareable`) and read under the job's ticket. + /// buffers are protected (`ThreadIsolated`) and read under the job's ticket. pub struct AsyncFSTask { - pub args: ThreadShareable, + pub args: ThreadIsolated, pub(crate) result: Maybe, } // SAFETY: results are plain data / owned buffers / WTF strings built off - // thread for hand-off (`ret::*`); `ThreadShareable` is Send by its contract. + // thread for hand-off (`ret::*`); `ThreadIsolated` is Send by its contract. unsafe impl Send for AsyncFSTask {} /// The JS-thread half of an async fs operation. @@ -1331,7 +1331,7 @@ mod _async_tasks { bun_jsc::Job::::schedule( &global_object.js_thread(), Self { - args: args.into_thread_shareable(), + args: args.into_thread_isolated(), // Sentinel — overwritten by `run` before any read. `Maybe` // may be niche-optimised; never construct an all-zero `Result`. result: Err(sys::Error::default()), @@ -1356,8 +1356,8 @@ mod _async_tasks { pub struct NewAsyncCpTask { pub(crate) promise: JSPromiseStrong, - /// Wrapped in [`ThreadShareable`] so the paired `unprotect()` runs on drop. - pub args: ThreadShareable>, + /// Wrapped in [`ThreadIsolated`] so the paired `unprotect()` runs on drop. + pub args: ThreadIsolated>, /// Owning-thread uses (global object, keep-alive context). pub(crate) evtloop: EventLoopHandle, /// How the last subtask's thread delivers the completion (moved out @@ -1584,7 +1584,7 @@ mod _async_tasks { ) -> *mut Self { let mut task = Box::new(Self { promise, - args: cp_args.into_thread_shareable(), + args: cp_args.into_thread_isolated(), has_result: AtomicBool::new(false), // Sentinel — overwritten by `finish_concurrently` (gated by the // `has_result` CAS) before any read on the JS thread. @@ -1768,8 +1768,8 @@ mod _async_tasks { let ctx = event_loop_handle_to_ctx(task.evtloop); task.r#ref.unref(ctx); } - // `Drop for ThreadShareable` releases the `protect()` taken by - // `make_thread_shareable()` when `src`/`dest` are Buffers, so nothing leaks here. + // `Drop for ThreadIsolated` releases the `protect()` taken by + // `make_thread_isolated()` when `src`/`dest` are Buffers, so nothing leaks here. } /// Directory scanning + clonefile will block this thread, then each individual file copy (what the sync version @@ -2136,7 +2136,7 @@ mod _async_tasks { pub struct AsyncReaddirRecursiveTask { /// Protected arguments; their JS-backed path is not read off-thread /// (`root_path` is the owned copy). - pub args: ThreadShareable>, + pub args: ThreadIsolated>, pub(crate) tag: ret::ReaddirTag, pub(crate) encoding: Encoding, /// The completion token, finished by whichever subtask ends the scan. @@ -2169,7 +2169,7 @@ mod _async_tasks { pub(crate) pending_err_mutex: bun_threading::Mutex, } // SAFETY: shared by the pool subtasks through atomics / the lock-free - // queue / the mutex; `args` is Send by `ThreadShareable`'s contract; results + // queue / the mutex; `args` is Send by `ThreadIsolated`'s contract; results // are owned buffers and WTF strings built off-thread for hand-off. unsafe impl Send for AsyncReaddirRecursiveTask {} @@ -2360,7 +2360,7 @@ mod _async_tasks { bun_jsc::Job::::schedule( &global_object.js_thread(), AsyncReaddirRecursiveTask { - args: FsArgument::into_thread_shareable(args), + args: FsArgument::into_thread_isolated(args), tag, encoding, done: None, @@ -2607,12 +2607,12 @@ pub use _async_tasks::{ pub mod args { use super::*; - /// Derive the `Unprotect` impl + inherent `make_thread_shareable` for an `args::*` + /// Derive the `Unprotect` impl + inherent `make_thread_isolated` for an `args::*` /// struct whose only JS-backed state is one-or-more path-like fields. Each - /// listed `$field` must expose `.unprotect()` and `.make_thread_shareable()` (i.e. + /// listed `$field` must expose `.unprotect()` and `.make_thread_isolated()` (i.e. /// `PathLike` or `PathOrFileDescriptor`); the expansion is byte-identical to /// the hand-written boilerplate it replaces, so `impl_fs_argument!`'s - /// `<$ty>::make_thread_shareable(self)` forwarder and `ThreadShareable`'s drop-guard + /// `<$ty>::make_thread_isolated(self)` forwarder and `ThreadIsolated`'s drop-guard /// keep working unchanged. Structs with non-path JS state (`Read`, `Write`, /// `Writev`, `Readv`, `Exists`, `ReadFile`, `WriteFile`) keep bespoke impls. macro_rules! fs_args_path_forwarders { @@ -2621,7 +2621,7 @@ pub mod args { #[inline] fn unprotect(&mut self) { $( self.$field.unprotect(); )+ } } impl $ty<'static> { - pub fn make_thread_shareable(&mut self) { $( self.$field.make_thread_shareable(); )+ } + pub fn make_thread_isolated(&mut self) { $( self.$field.make_thread_isolated(); )+ } } }; } @@ -2699,7 +2699,7 @@ pub mod args { } } impl FdVectorIo { - pub(crate) fn make_thread_shareable(&mut self) { + pub(crate) fn make_thread_isolated(&mut self) { self.buffers.value.protect(); self.buffers.buffers = self.buffers.buffers.as_slice().to_vec(); } @@ -2737,7 +2737,7 @@ pub mod args { fn unprotect(&mut self) {} } impl FTruncate { - pub(crate) fn make_thread_shareable(&self) {} + pub(crate) fn make_thread_isolated(&self) {} pub fn from_js( ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice, @@ -2811,7 +2811,7 @@ pub mod args { pub(crate) gid: GidT, } impl Fchown { - pub(crate) fn make_thread_shareable(&self) {} + pub(crate) fn make_thread_isolated(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let uid: UidT = 'brk: { @@ -2927,7 +2927,7 @@ pub mod args { pub(crate) mode: Mode, } impl FChmod { - pub(crate) fn make_thread_shareable(&self) {} + pub(crate) fn make_thread_isolated(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let mode_arg = arguments.next().unwrap_or(JSValue::UNDEFINED); @@ -3015,7 +3015,7 @@ pub mod args { pub(crate) big_int: bool, } impl Fstat { - pub(crate) fn make_thread_shareable(&mut self) {} + pub(crate) fn make_thread_isolated(&mut self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let big_int = 'brk: { @@ -3223,8 +3223,8 @@ pub mod args { pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { Ok(Rm(RmDir::from_js_impl(ctx, arguments, true)?)) } - pub(crate) fn make_thread_shareable(&mut self) { - self.0.make_thread_shareable(); + pub(crate) fn make_thread_isolated(&mut self) { + self.0.make_thread_isolated(); } } @@ -3455,7 +3455,7 @@ pub mod args { pub(crate) fd: FD, } impl Close { - pub(crate) fn make_thread_shareable(&self) {} + pub(crate) fn make_thread_isolated(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; Ok(Close { fd }) @@ -3505,7 +3505,7 @@ pub mod args { pub(crate) mtime: TimeLike, } impl Futimes { - pub(crate) fn make_thread_shareable(&self) {} + pub(crate) fn make_thread_isolated(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; let atime = node::time_like_from_js( @@ -3583,8 +3583,8 @@ pub mod args { } } impl Write<'static> { - pub(crate) fn make_thread_shareable(&mut self) { - self.buffer.make_thread_shareable(); + pub(crate) fn make_thread_isolated(&mut self) { + self.buffer.make_thread_isolated(); } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; @@ -3736,7 +3736,7 @@ pub mod args { pub(crate) pinned: bool, } impl Read { - pub(crate) fn make_thread_shareable(&self) { + pub(crate) fn make_thread_isolated(&self) { self.buffer.buffer.value.protect(); } } @@ -3983,8 +3983,8 @@ pub mod args { } } impl ReadFile<'static> { - pub(crate) fn make_thread_shareable(&mut self) { - self.path.make_thread_shareable(); + pub(crate) fn make_thread_isolated(&mut self) { + self.path.make_thread_isolated(); } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { // `Drop` on `path` covers every @@ -4071,8 +4071,8 @@ pub mod args { } } impl WriteFile<'static> { - pub(crate) fn make_thread_shareable(&mut self) { - self.file.make_thread_shareable(); + pub(crate) fn make_thread_isolated(&mut self) { + self.file.make_thread_isolated(); } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { Self::from_js_with_default_flag(ctx, arguments, FileSystemFlags::W) @@ -4192,9 +4192,9 @@ pub mod args { } } impl Exists<'static> { - pub(crate) fn make_thread_shareable(&mut self) { + pub(crate) fn make_thread_isolated(&mut self) { if let Some(p) = &mut self.path { - p.make_thread_shareable(); + p.make_thread_isolated(); } } pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { @@ -4225,7 +4225,7 @@ pub mod args { pub(crate) fd: FD, } impl FdataSync { - pub(crate) fn make_thread_shareable(&self) {} + pub(crate) fn make_thread_isolated(&self) {} pub fn from_js( ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice, @@ -4318,7 +4318,7 @@ pub mod args { pub(crate) fd: FD, } impl Fsync { - pub(crate) fn make_thread_shareable(&self) {} + pub(crate) fn make_thread_isolated(&self) {} pub fn from_js(ctx: &JSGlobalObject, arguments: &mut ArgumentsSlice) -> JsResult { let fd = FD::from_js_required(ctx, arguments)?; Ok(Fsync { fd }) diff --git a/src/runtime/node/node_fs_binding.rs b/src/runtime/node/node_fs_binding.rs index b0fd5980b6c5..1297e143b2fa 100644 --- a/src/runtime/node/node_fs_binding.rs +++ b/src/runtime/node/node_fs_binding.rs @@ -89,7 +89,7 @@ fn parse_async_args( // `ManuallyDrop` keeps `slice` alive past return when ownership transfers // to the Task: dropped only on the early-return // error/abort branches; on the success path the Task owns `args` (whose - // protected JSValues are released by `Drop for ThreadShareable` when the + // protected JSValues are released by `Drop for ThreadIsolated` when the // Task completes), and `slice` is intentionally not dropped — its // `Drop`-unprotect would race that. diff --git a/src/runtime/node/node_process.rs b/src/runtime/node/node_process.rs index 019ea7d9bb16..18643dcb20ae 100644 --- a/src/runtime/node/node_process.rs +++ b/src/runtime/node/node_process.rs @@ -38,8 +38,8 @@ extern "C" fn get_exec_path(global_object: &JSGlobalObject) -> JSValue { } /// A worker's `argv`/`execArgv` strings live in its parent-thread -/// `WorkerOptions`; the worker thread gets its own copy (thread-affine -/// refcounts), and an empty one is spelled as `BunString::EMPTY`. +/// `WorkerOptions`; the worker thread gets its own copy (the parent may +/// atomize its impls), and an empty one is spelled as `BunString::EMPTY`. pub(crate) fn worker_option_string(wtf: bun_core::WTFStringImpl) -> bun_core::String { // SAFETY: non-null impl borrowed from the live `WorkerOptions`. let imp = unsafe { &*wtf }; diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 4f0508921964..030c4ae8a2f1 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -272,7 +272,7 @@ impl BlobOrStringOrBuffer { /// Rust-side bytes for a synchronous call ([`StringOrBuffer::borrowed`]). pub enum StringOrBuffer<'a> { String(Utf8WithString), - ThreadShareableString(Utf8WithString), + ThreadIsolatedString(Utf8WithString), Utf8(Utf8Bytes<'a>), Buffer(Buffer), } @@ -299,7 +299,7 @@ impl<'a> StringOrBuffer<'a> { pub(crate) fn slice(&self) -> &[u8] { match self { Self::String(str) => str.slice(), - Self::ThreadShareableString(str) => str.slice(), + Self::ThreadIsolatedString(str) => str.slice(), Self::Utf8(str) => str.slice(), Self::Buffer(str) => str.slice(), } @@ -309,7 +309,7 @@ impl<'a> StringOrBuffer<'a> { impl bun_jsc::Unprotect for BlobOrStringOrBuffer { /// JS-side half of cleanup — owned /// payloads are released by `Drop` (which runs next when held in a - /// [`bun_jsc::ThreadShareable`]). + /// [`bun_jsc::ThreadIsolated`]). #[inline] fn unprotect(&mut self) { if let Self::StringOrBuffer(sob) = self { @@ -320,7 +320,7 @@ impl bun_jsc::Unprotect for BlobOrStringOrBuffer { impl bun_jsc::Unprotect for StringOrBuffer<'static> { /// JS-side half of cleanup — undo the - /// `protect()` taken by [`StringOrBuffer::make_thread_shareable`] / + /// `protect()` taken by [`StringOrBuffer::make_thread_isolated`] / /// `from_js_maybe_async(.., Flavor::Async, ..)`. Owned slices are released /// by `Drop`. #[inline] @@ -338,7 +338,7 @@ impl bun_jsc::Unprotect for StringOrBuffer<'static> { impl StringOrBuffer<'_> { pub fn into_js(self, ctx: &JSGlobalObject) -> JsResult { match self { - Self::ThreadShareableString(str) | Self::String(str) => str.into_js(ctx), + Self::ThreadIsolatedString(str) | Self::String(str) => str.into_js(ctx), Self::Utf8(utf8) => bun_string_jsc::create_utf8_for_js(ctx, &utf8), Self::Buffer(mut buffer) => { if buffer.buffer.value != JSValue::ZERO { @@ -361,14 +361,14 @@ impl StringOrBuffer<'_> { } impl StringOrBuffer<'static> { - pub(crate) fn make_thread_shareable(&mut self) { + pub(crate) fn make_thread_isolated(&mut self) { match self { Self::String(s) => { - s.make_thread_shareable(); + s.make_thread_isolated(); let str = core::mem::take(s); - *self = Self::ThreadShareableString(str); + *self = Self::ThreadIsolatedString(str); } - Self::ThreadShareableString(_) | Self::Utf8(_) => {} + Self::ThreadIsolatedString(_) | Self::Utf8(_) => {} Self::Buffer(buffer) => { buffer.buffer.value.protect(); } @@ -421,8 +421,8 @@ impl StringOrBuffer<'static> { *out = if flavor == Flavor::Async { shared_or_utf8( global, - str.into_utf8_with_string_thread_shareable(), - Self::ThreadShareableString, + str.into_utf8_with_string_thread_isolated(), + Self::ThreadIsolatedString, Self::Utf8, ) } else { @@ -821,7 +821,7 @@ pub struct NameTooLong; /// `bun_runtime`-tier behaviour layered on `bun_jsc::node_path::PathLike`. /// -/// `make_thread_shareable` / `into_thread_shareable` / `slice` / `estimated_size` are +/// `make_thread_isolated` / `into_thread_isolated` / `slice` / `estimated_size` are /// inherent on the lower-tier type (see `bun_jsc::node_path`); this trait /// adds only the path-buffer slicers and JS-argument parsing that depend on /// `bun_runtime` types (`Valid`, `ArgumentsSlice` cursor flow). @@ -1233,7 +1233,7 @@ fn path_like_from_string( will_be_async: bool, ) -> JsResult> { let utf8 = if will_be_async { - str.into_utf8_with_string_thread_shareable() + str.into_utf8_with_string_thread_isolated() } else { str.into_utf8_with_string() }; @@ -1241,7 +1241,7 @@ fn path_like_from_string( Valid::path_null_bytes(utf8.slice(), global)?; let shared = if will_be_async { - PathLike::ThreadShareableString + PathLike::ThreadIsolatedString } else { PathLike::String }; diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 64223c13e319..1282afded664 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3561,16 +3561,10 @@ impl BlobExt for Blob { *path_or_fd = PathOrFileDescriptor::Path(PathLike::borrowed(b"\\\\.\\NUL")); } - // SAFETY: bun_vm() is live for the duration of a host call. - if global_this.bun_vm().standalone_module_graph.is_some() { - // `vm.standalone_module_graph` is a type-erased `&dyn` so - // `bun_jsc` doesn't depend on `bun_standalone_graph`; the - // concrete `Graph` is the sole implementor. - let graph = bun_standalone_graph::Graph::get_ref() - .expect("vm.standalone_module_graph set ⇔ Graph singleton populated"); - if let Some(file) = graph.find_ref(path_or_fd.path().slice()) { - return crate::api::standalone_graph_jsc::file_blob(file, global_this); - } + if let Some(file) = bun_standalone_graph::Graph::get_ref() + .and_then(|graph| graph.find_ref(path_or_fd.path().slice())) + { + return crate::api::standalone_graph_jsc::file_blob(file, global_this); } path_or_fd.make_thread_shareable(); @@ -5523,7 +5517,6 @@ pub(crate) fn jsdom_file_construct( callframe: &CallFrame, ) -> JsResult<*mut Blob> { jsc::mark_binding(); - let blob: Blob; let args = callframe.arguments(); if args.len() < 2 { @@ -5531,26 +5524,10 @@ pub(crate) fn jsdom_file_construct( "new File(bits, name) expects at least 2 arguments" ))); } - { - use bun_jsc::StringJsc as _; - let name_value_str = BunString::from_js(args[1], global_this)?; - - blob = Blob::get::(global_this, args[0])?; - if blob.store.get().is_some() { - // The store may be shared with the source blob (`dupe()`); never rename it. - blob.name.set(name_value_str); - } else if !name_value_str.is_empty() { - // not store but we have a name so we need a store - blob.store.set(Some(RefPtr::new(Store { - data: store::Data::Bytes(store::Bytes::init_empty_with_name( - name_value_str.to_owned_slice().into_boxed_slice(), - )), - ref_count: bun_ptr::ThreadSafeRefCount::init(), - mime_type: bun_http_types::MimeType::NONE, - is_all_ascii: store::IsAllAscii::default(), - }))); - } - } + let name = BunString::from_js(args[1], global_this)?; + let blob = Blob::get::(global_this, args[0])?; + // The store may be shared with the source blob (`dupe()`); never rename it. + blob.name.set(name); let mut set_last_modified = false; diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 76f12a688029..be2d5f72ce2b 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -463,9 +463,7 @@ impl FetchTasklet { self.url_proxy_buffer = Box::default(); } - if let Some(_hostname) = self.hostname.take() { - // dropped by Box - } + self.hostname = None; self.unix_socket_path = Box::default(); if let Some(certificate) = self.result.certificate_info.take() { diff --git a/test/internal/source-lints/vm-thread-door.inventory.json b/test/internal/source-lints/vm-thread-door.inventory.json index 4fa878aecbb2..ecb9ea232ce2 100644 --- a/test/internal/source-lints/vm-thread-door.inventory.json +++ b/test/internal/source-lints/vm-thread-door.inventory.json @@ -50,7 +50,7 @@ }, "src/jsc/node_path.rs": { "unsafe impl Send": [ - "ThreadShareable" + "ThreadIsolated" ] }, "src/jsc/web_worker.rs": { diff --git a/test/js/bun/util/bunstring-thread-isolated-copy.test.ts b/test/js/bun/util/bunstring-thread-shareable.test.ts similarity index 61% rename from test/js/bun/util/bunstring-thread-isolated-copy.test.ts rename to test/js/bun/util/bunstring-thread-shareable.test.ts index 0d2665ed4d14..cda6e08ab5a5 100644 --- a/test/js/bun/util/bunstring-thread-isolated-copy.test.ts +++ b/test/js/bun/util/bunstring-thread-shareable.test.ts @@ -1,27 +1,27 @@ -import { BunString_threadIsolatedCopyRefCountDelta } from "bun:internal-for-testing"; +import { + BunString_makeThreadShareableRefCountDelta, + BunString_threadIsolatedCopyRefCountDelta, +} from "bun:internal-for-testing"; import { expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; -// BunString__threadIsolatedCopy must not leak a ref on the original -// StringImpl: it returns a +1 isolated copy and leaves the source's -// refcount untouched. +// A positive delta means the original StringImpl's ref was leaked; negative, over-released. test("BunString__threadIsolatedCopy does not leak a ref on the original StringImpl", () => { - expect(typeof BunString_threadIsolatedCopyRefCountDelta).toBe("function"); - - // A correct implementation leaves the original StringImpl's refcount - // unchanged once both BunStrings are released. A positive delta means the - // original ref was leaked. for (let i = 0; i < 8; i++) { expect(BunString_threadIsolatedCopyRefCountDelta()).toBe(0); } }); -// Exercise the real callers (Bun.file / async fs.write) whose Zig-side -// Utf8WithString::make_thread_shareable wrappers were updated alongside the -// C++ fix. With ASAN this would crash on a double-deref if the two sides ever -// disagree on who owns the old StringImpl. -test("make_thread_shareable callers (Bun.file / fs.write) keep refcounts balanced", async () => { - using dir = tempDir("bunstring-thread-isolated-copy", { +test("BunString__makeThreadShareable on an atom releases exactly the ref it replaces", () => { + for (let i = 0; i < 8; i++) { + expect(BunString_makeThreadShareableRefCountDelta()).toBe(0); + } +}); + +// Bun.file(path) and async fs.write(fd, string) hand a JS string to another thread through +// Utf8WithString::make_thread_shareable / make_thread_isolated; under ASAN a ref imbalance there is a double-deref. +test("Bun.file / async fs.write string hand-off keeps refcounts balanced", async () => { + using dir = tempDir("bunstring-thread-shareable", { "target.txt": "", }); const src = ` @@ -40,7 +40,7 @@ test("make_thread_shareable callers (Bun.file / fs.write) keep refcounts balance } // Async fs.write(fd, string) routes the data string through - // StringOrBuffer::make_thread_shareable. + // StringOrBuffer::make_thread_isolated. const fd = fs.openSync(targetPath, "w"); const payload = Buffer.alloc(48, "p").toString(); let total = 0; From 150fe27407a1c7182b3d150571c5b00a5e9c38be Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 23:51:23 +0000 Subject: [PATCH 14/25] Readable::init: drop the Stdio it consumes (only the memfd is moved out); a Bun.file stdin's cloned path was leaked --- src/runtime/api/bun/spawn/stdio.rs | 10 ++++++++++ src/runtime/api/bun/subprocess/Readable.rs | 23 +++++++++++----------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/runtime/api/bun/spawn/stdio.rs b/src/runtime/api/bun/spawn/stdio.rs index 37a5ee09c8cf..1a9c3fc308d0 100644 --- a/src/runtime/api/bun/spawn/stdio.rs +++ b/src/runtime/api/bun/spawn/stdio.rs @@ -665,6 +665,16 @@ impl Stdio { } } +impl Stdio { + /// Move the memfd out (ownership passes to the caller); `self` becomes `Ignore`. + pub fn take_memfd(&mut self) -> Option { + let Stdio::Memfd(fd) = *self else { return None }; + // Don't run Drop on the old value: it would close `fd`. + let _ = core::mem::ManuallyDrop::new(core::mem::replace(self, Stdio::Ignore)); + Some(fd) + } +} + impl Drop for Stdio { fn drop(&mut self) { match self { diff --git a/src/runtime/api/bun/subprocess/Readable.rs b/src/runtime/api/bun/subprocess/Readable.rs index e81e9ce48b2d..d990f37428cd 100644 --- a/src/runtime/api/bun/subprocess/Readable.rs +++ b/src/runtime/api/bun/subprocess/Readable.rs @@ -92,20 +92,22 @@ impl Readable { ) -> Readable { super::assert_stdio_result!(result); - // Ownership of any resource inside `stdio` (notably `.memfd`) is being - // *transferred* into the returned `Readable`. `Stdio` has a `Drop` impl that - // would close the memfd, so suppress it here to avoid a double-close - // (EBADF) when the Readable later closes the same fd. - let stdio = mem::ManuallyDrop::new(stdio); + let mut stdio = stdio; + if let Some(memfd) = stdio.take_memfd() { + #[cfg(unix)] + return Readable::Memfd(memfd); + #[cfg(not(unix))] + return Readable::Ignore; + } #[cfg(unix)] { - if matches!(*stdio, Stdio::Pipe) { + if matches!(stdio, Stdio::Pipe) { let _ = bun_sys::set_nonblocking(result.unwrap()); } } - match &*stdio { + match &stdio { Stdio::Inherit => Readable::Inherit, Stdio::Ignore | Stdio::Ipc | Stdio::Path(..) => Readable::Ignore, Stdio::Fd(fd) => { @@ -119,15 +121,14 @@ impl Readable { Readable::Fd(*fd) } } - Stdio::Memfd(memfd) => { + Stdio::Memfd(_) => { #[cfg(unix)] { - Readable::Memfd(*memfd) + unreachable!() } #[cfg(not(unix))] { - let _ = memfd; - Readable::Ignore + unreachable!() } } Stdio::Dup2(dup2) => { From 2b81d4afc177bbee597f6a1b18ff8fdc62108c5e Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 25 Aug 2026 23:55:32 +0000 Subject: [PATCH 15/25] Readable::init: silence unused memfd on Windows No-Verification-Needed: cfg(windows) compile fix --- src/runtime/api/bun/subprocess/Readable.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/runtime/api/bun/subprocess/Readable.rs b/src/runtime/api/bun/subprocess/Readable.rs index d990f37428cd..021cc90c1f42 100644 --- a/src/runtime/api/bun/subprocess/Readable.rs +++ b/src/runtime/api/bun/subprocess/Readable.rs @@ -97,7 +97,10 @@ impl Readable { #[cfg(unix)] return Readable::Memfd(memfd); #[cfg(not(unix))] - return Readable::Ignore; + { + let _ = memfd; + return Readable::Ignore; + } } #[cfg(unix)] From eea22c75e65dda5f69859f67a9d6ff4fcb09f128 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 00:36:40 +0000 Subject: [PATCH 16/25] get_file_name: an empty name is not a filename (loader/Content-Disposition fall back to the store path or MIME type) --- src/jsc/webcore_types.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index dc9ea8d67918..5eb7dc5bdd1f 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -422,12 +422,13 @@ impl Blob { matches!(self.store.get().as_deref(), Some(s) if matches!(s.data, store::Data::File(_))) } - /// The name a `File` reports: `name` if set, else [`store_path`]. + /// A usable filename: a non-empty `name`, else [`store_path`]. (`file.name` + /// itself may be `""`; that is not a filename.) /// /// [`store_path`]: Self::store_path pub fn get_file_name(&self) -> Option> { let name = self.name.get(); - if name.tag() != bun_core::Tag::Dead { + if !name.is_empty() { return Some(name.to_utf8()); } self.store_path().map(bun_core::Utf8Bytes::Borrowed) From 3c98f82520210061712703757c056431a049b18a Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 00:51:09 +0000 Subject: [PATCH 17/25] =?UTF-8?q?Bun.file(path)/S3=20store=20paths:=20back?= =?UTF-8?q?=20to=20an=20isolated=20copy=20(main's=20behaviour;=20the=20sto?= =?UTF-8?q?re's=20copy=20is=20never=20handed=20to=20JS,=20so=20it=20was=20?= =?UTF-8?q?correct)=20=E2=80=94=20make=5Fthread=5Fshareable=20stays=20only?= =?UTF-8?q?=20where=20one=20impl=20is=20reachable=20from=20several=20VMs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/bun_core/string/mod.rs | 20 ++++++++++++-------- src/jsc/node_path.rs | 14 +++++++------- src/runtime/webcore/Blob.rs | 2 +- src/runtime/webcore/blob/Store.rs | 3 +-- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index b76ceba9d917..dce53d6478c2 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -1578,14 +1578,18 @@ impl Utf8WithString { self.string = String::EMPTY; } - /// For a holder that any thread may read or drop (a `Blob` store): as - /// [`make_thread_isolated`], and a kept `string` is made shareable so JS - /// can no longer atomize it in place. - /// - /// [`make_thread_isolated`]: Self::make_thread_isolated - pub fn make_thread_shareable(&mut self) { - self.make_thread_isolated(); - self.string.make_thread_shareable(); + /// For a holder that may be dropped on any thread (a `Blob` store): the + /// bytes end up owned by this value alone — either the transcoded `utf8` + /// or a [`String::thread_isolated_copy`] that is never handed to JS. + pub fn thread_isolated_copy(mut self) -> Self { + if self.string.tag == Tag::WTFStringImpl { + self.string = if self.utf8.is_some() { + String::EMPTY + } else { + self.string.thread_isolated_copy() + }; + } + self } } diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index 8aa4f1334796..e788c65f5dcc 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -208,13 +208,13 @@ impl PathLike<'static> { } } - /// As [`make_thread_isolated`](Self::make_thread_isolated), for a path a - /// `Blob` store keeps (read and dropped on any thread). - pub fn make_thread_shareable(&mut self) { + /// For a path a `Blob` store keeps (dropped on any thread): the string + /// becomes a private copy never handed to JS; a `Buffer` is protected as + /// in [`make_thread_isolated`](Self::make_thread_isolated). + pub fn make_thread_isolated_copy(&mut self) { match self { Self::String(s) | Self::ThreadIsolatedString(s) => { - s.make_thread_shareable(); - let owned = core::mem::take(s); + let owned = core::mem::take(s).thread_isolated_copy(); *self = Self::ThreadIsolatedString(owned); } Self::Buffer(b) => { @@ -286,9 +286,9 @@ impl PathOrFileDescriptor<'static> { } } #[inline] - pub fn make_thread_shareable(&mut self) { + pub fn make_thread_isolated_copy(&mut self) { if let Self::Path(p) = self { - p.make_thread_shareable(); + p.make_thread_isolated_copy(); } } } diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 1282afded664..d02a560b97a8 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3567,7 +3567,7 @@ impl BlobExt for Blob { return crate::api::standalone_graph_jsc::file_blob(file, global_this); } - path_or_fd.make_thread_shareable(); + path_or_fd.make_thread_isolated_copy(); core::mem::replace(path_or_fd, PathOrFileDescriptor::Path(PathLike::default())) } PathOrFileDescriptor::Fd(fd) => { diff --git a/src/runtime/webcore/blob/Store.rs b/src/runtime/webcore/blob/Store.rs index 337665f5fda1..c5128e86f93d 100644 --- a/src/runtime/webcore/blob/Store.rs +++ b/src/runtime/webcore/blob/Store.rs @@ -127,8 +127,7 @@ impl StoreExt for Store { credentials: S3Credentials, ) -> Result, crate::Error> { let mut path = pathlike; - // this actually protects/refs the pathlike - path.make_thread_shareable(); + path.make_thread_isolated_copy(); // Compute the extension-derived fallback before moving `path` into the // Store so we don't need to clone the owned PathLike. From 13bf71d65ce80254b77c663ba9ee587f74f6839c Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 00:58:50 +0000 Subject: [PATCH 18/25] bake module_keys: back to main's isolated copy (one VM today) --- src/runtime/bake/production.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/bake/production.rs b/src/runtime/bake/production.rs index f9cf26b91072..2653ace4c53f 100644 --- a/src/runtime/bake/production.rs +++ b/src/runtime/bake/production.rs @@ -743,12 +743,12 @@ fn build_with_vm(ctx: Context, cwd: &[u8], pt: &mut PerThread) -> crate::Result< if let Some(entry_point_index) = file.entry_point_index { if (entry_point_index as usize) < module_keys.len() { - let mut str = BunString::create_format(format_args!( + let str = BunString::create_format(format_args!( "bake:/{}", BStr::new(without_prefix) )); - str.make_thread_shareable(); - module_keys[entry_point_index as usize] = str; + module_keys[entry_point_index as usize] = + str.thread_isolated_copy(); } } From 0d976d1c842f987ac160ea2d0f04f68276736984 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 01:06:31 +0000 Subject: [PATCH 19/25] Utf8WithString::thread_isolated_copy: build the copy instead of mutating self No-Verification-Needed: equivalent reshape --- src/bun_core/string/mod.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index dce53d6478c2..a855376e451d 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -1581,15 +1581,19 @@ impl Utf8WithString { /// For a holder that may be dropped on any thread (a `Blob` store): the /// bytes end up owned by this value alone — either the transcoded `utf8` /// or a [`String::thread_isolated_copy`] that is never handed to JS. - pub fn thread_isolated_copy(mut self) -> Self { - if self.string.tag == Tag::WTFStringImpl { - self.string = if self.utf8.is_some() { - String::EMPTY - } else { - self.string.thread_isolated_copy() - }; + pub fn thread_isolated_copy(self) -> Self { + if self.string.tag != Tag::WTFStringImpl { + return self; + } + let string = if self.utf8.is_some() { + String::EMPTY + } else { + self.string.thread_isolated_copy() + }; + Self { + utf8: self.utf8, + string, } - self } } From 55b4156395edfd47c3f1b866a527431fb5caaf6b Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 01:13:48 +0000 Subject: [PATCH 20/25] Rename Bun::isolatedCopyForSharing -> Bun::threadShareableCopy No-Verification-Needed: rename --- src/CLAUDE.md | 2 +- src/jsc/bindings/BunString.cpp | 6 +++--- src/jsc/bindings/BunString.h | 4 ++-- src/jsc/bindings/webcore/SerializedScriptValue.cpp | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/CLAUDE.md b/src/CLAUDE.md index 1bcb4d735d68..5e7d653ec17c 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -395,7 +395,7 @@ computed hash/flags word is also unsynchronized. Rules: (`Box<[u8]>`, `clone_utf8` on arrival). - Letting several VMs reach one impl (process-global registry, one `SerializedScriptValue` with many receivers): `String::make_thread_shareable()` - (C++ `Bun::makeThreadShareable` / `isolatedCopyForSharing` / + (C++ `Bun::makeThreadShareable` / `threadShareableCopy` / `toCrossThreadShareable`) once — pre-hashed, never atomized in place, so each receiver's atom table takes its own copy — then hand out plain `clone()`s. Static strings already qualify. diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index 7a570b8f9245..7aee16bd19af 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -300,7 +300,7 @@ static constexpr unsigned int kMinCrossThreadShareableLength = 256; // the receivers race the lazy m_hashAndFlags update (debug: ASSERT(!hasHash()) // in setHash; e.g. two workers switch()ing on the same BroadcastChannel // message). Static strings are immortal, pre-hashed and safe to share as-is. -Ref isolatedCopyForSharing(const WTF::StringImpl& impl) +Ref threadShareableCopy(const WTF::StringImpl& impl) { Ref copy = impl.isolatedCopy(); if (!copy->isStatic()) { @@ -313,7 +313,7 @@ Ref isolatedCopyForSharing(const WTF::StringImpl& impl) Ref makeThreadShareable(WTF::StringImpl& impl) { if (impl.isAtom() || impl.isSymbol() || impl.isSubString()) - return isolatedCopyForSharing(impl); + return threadShareableCopy(impl); if (!impl.isStatic()) { impl.hash(); // Already set means other threads may hold this impl; don't write the flags word. @@ -329,7 +329,7 @@ WTF::String toCrossThreadShareable(const WTF::String& string) if (!impl) return string; if (impl->length() < kMinCrossThreadShareableLength) - return isolatedCopyForSharing(*impl); + return threadShareableCopy(*impl); return makeThreadShareable(*impl); } diff --git a/src/jsc/bindings/BunString.h b/src/jsc/bindings/BunString.h index 66b1017a5ed6..b4ee1b012b3d 100644 --- a/src/jsc/bindings/BunString.h +++ b/src/jsc/bindings/BunString.h @@ -57,10 +57,10 @@ class UTF8View { }; // Pre-hashed and never atomized in place, so any number of threads may hold -// the result and use it as a property key. `isolatedCopyForSharing` always +// the result and use it as a property key. `threadShareableCopy` always // copies; `makeThreadShareable` copies only an atom/symbol/substring; // `toCrossThreadShareable` also copies short strings so they stay atomizable. -Ref isolatedCopyForSharing(const WTF::StringImpl&); +Ref threadShareableCopy(const WTF::StringImpl&); Ref makeThreadShareable(WTF::StringImpl&); WTF::String toCrossThreadShareable(const WTF::String&); diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index cc5721d711be..a2845f470697 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/src/jsc/bindings/webcore/SerializedScriptValue.cpp @@ -4611,10 +4611,10 @@ ExceptionOr> SerializedScriptValue::create(JSGlobalOb objOk = false; return false; } - properties.append({ Bun::isolatedCopyForSharing(*entry.key()), + properties.append({ Bun::threadShareableCopy(*entry.key()), Bun::toCrossThreadShareable(stringValue) }); } else { - properties.append({ Bun::isolatedCopyForSharing(*entry.key()), propValue }); + properties.append({ Bun::threadShareableCopy(*entry.key()), propValue }); } return true; }); @@ -4693,10 +4693,10 @@ ExceptionOr> SerializedScriptValue::create(JSGlobalOb canUseObjectFastPath = false; return false; } - properties.append({ Bun::isolatedCopyForSharing(*entry.key()), Bun::toCrossThreadShareable(stringValue) }); + properties.append({ Bun::threadShareableCopy(*entry.key()), Bun::toCrossThreadShareable(stringValue) }); } else { // Primitive values are safe to share across threads. - properties.append({ Bun::isolatedCopyForSharing(*entry.key()), value }); + properties.append({ Bun::threadShareableCopy(*entry.key()), value }); } return true; From c0dc9c1408b23ef5072b225a0b55f2b03e13d2ca Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 01:16:51 +0000 Subject: [PATCH 21/25] Utf8WithString::thread_isolated_copy: exactly main's to_thread_safe (isolated copy of the string; transcoded bytes kept as they were) No-Verification-Needed: restores main behaviour --- src/bun_core/string/mod.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index a855376e451d..60c5599e4671 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -1578,21 +1578,12 @@ impl Utf8WithString { self.string = String::EMPTY; } - /// For a holder that may be dropped on any thread (a `Blob` store): the - /// bytes end up owned by this value alone — either the transcoded `utf8` - /// or a [`String::thread_isolated_copy`] that is never handed to JS. + /// For a holder that may be dropped on any thread (a `Blob` store): + /// `string` becomes a [`String::thread_isolated_copy`] never handed to JS. pub fn thread_isolated_copy(self) -> Self { - if self.string.tag != Tag::WTFStringImpl { - return self; - } - let string = if self.utf8.is_some() { - String::EMPTY - } else { - self.string.thread_isolated_copy() - }; Self { + string: self.string.thread_isolated_copy(), utf8: self.utf8, - string, } } } From 4cb843b9d2e961dc0d8e9b9b55f78b4d2982312e Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 01:18:07 +0000 Subject: [PATCH 22/25] Utf8WithString::thread_isolated_copy: when the bytes were already transcoded, drop the WTF backing instead of copying it too (one allocation instead of main's two for non-ASCII paths) --- src/bun_core/string/mod.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index 60c5599e4671..d9bb31700f96 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -1578,12 +1578,22 @@ impl Utf8WithString { self.string = String::EMPTY; } - /// For a holder that may be dropped on any thread (a `Blob` store): - /// `string` becomes a [`String::thread_isolated_copy`] never handed to JS. + /// For a holder that may be dropped on any thread (a `Blob` store): the + /// bytes end up owned by this value alone — the transcoded `utf8` if + /// there is one (the WTF backing is dropped), else a + /// [`String::thread_isolated_copy`] that is never handed to JS. pub fn thread_isolated_copy(self) -> Self { + if self.string.tag != Tag::WTFStringImpl { + return self; + } + let string = if self.utf8.is_some() { + String::EMPTY + } else { + self.string.thread_isolated_copy() + }; Self { - string: self.string.thread_isolated_copy(), utf8: self.utf8, + string, } } } From f5f11b8d29a97b5a0304de01d3afea89521833d6 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 01:20:47 +0000 Subject: [PATCH 23/25] Readable::init: handle Memfd inside the match (no pre-pass, no unreachable arm) --- src/runtime/api/bun/subprocess/Readable.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/runtime/api/bun/subprocess/Readable.rs b/src/runtime/api/bun/subprocess/Readable.rs index 021cc90c1f42..12ecccf4cdd7 100644 --- a/src/runtime/api/bun/subprocess/Readable.rs +++ b/src/runtime/api/bun/subprocess/Readable.rs @@ -93,16 +93,6 @@ impl Readable { super::assert_stdio_result!(result); let mut stdio = stdio; - if let Some(memfd) = stdio.take_memfd() { - #[cfg(unix)] - return Readable::Memfd(memfd); - #[cfg(not(unix))] - { - let _ = memfd; - return Readable::Ignore; - } - } - #[cfg(unix)] { if matches!(stdio, Stdio::Pipe) { @@ -125,13 +115,16 @@ impl Readable { } } Stdio::Memfd(_) => { + // Ownership of the fd moves into the Readable; `Stdio`'s Drop would close it. + let memfd = stdio.take_memfd().unwrap(); #[cfg(unix)] { - unreachable!() + Readable::Memfd(memfd) } #[cfg(not(unix))] { - unreachable!() + let _ = memfd; + Readable::Ignore } } Stdio::Dup2(dup2) => { From e04c24e623f712e51ee12f34709b5833ebfc7a0f Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 01:26:21 +0000 Subject: [PATCH 24/25] PathLike/PathOrFileDescriptor::thread_isolated_copy: by value, like String/Utf8WithString --- src/jsc/node_path.rs | 26 +++++++++++++------------- src/runtime/webcore/Blob.rs | 2 +- src/runtime/webcore/blob/Store.rs | 3 +-- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index e788c65f5dcc..568a9cbf6fc6 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -208,20 +208,19 @@ impl PathLike<'static> { } } - /// For a path a `Blob` store keeps (dropped on any thread): the string - /// becomes a private copy never handed to JS; a `Buffer` is protected as - /// in [`make_thread_isolated`](Self::make_thread_isolated). - pub fn make_thread_isolated_copy(&mut self) { - match self { + /// For a path a `Blob` store keeps (dropped on any thread): a JS-backed + /// string is replaced by a private copy never handed to JS; a `Buffer` is + /// protected as in [`make_thread_isolated`](Self::make_thread_isolated). + pub fn thread_isolated_copy(mut self) -> Self { + match &mut self { Self::String(s) | Self::ThreadIsolatedString(s) => { - let owned = core::mem::take(s).thread_isolated_copy(); - *self = Self::ThreadIsolatedString(owned); - } - Self::Buffer(b) => { - b.buffer.value.protect(); + let s = core::mem::take(s).thread_isolated_copy(); + self = Self::ThreadIsolatedString(s); } + Self::Buffer(b) => b.buffer.value.protect(), Self::Utf8(_) => {} } + self } } @@ -286,9 +285,10 @@ impl PathOrFileDescriptor<'static> { } } #[inline] - pub fn make_thread_isolated_copy(&mut self) { - if let Self::Path(p) = self { - p.make_thread_isolated_copy(); + pub fn thread_isolated_copy(self) -> Self { + match self { + Self::Path(p) => Self::Path(p.thread_isolated_copy()), + Self::Fd(fd) => Self::Fd(fd), } } } diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index d02a560b97a8..1f84288e6305 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3567,8 +3567,8 @@ impl BlobExt for Blob { return crate::api::standalone_graph_jsc::file_blob(file, global_this); } - path_or_fd.make_thread_isolated_copy(); core::mem::replace(path_or_fd, PathOrFileDescriptor::Path(PathLike::default())) + .thread_isolated_copy() } PathOrFileDescriptor::Fd(fd) => { if let Some(tag) = fd.stdio_tag() { diff --git a/src/runtime/webcore/blob/Store.rs b/src/runtime/webcore/blob/Store.rs index c5128e86f93d..b950a1adcd46 100644 --- a/src/runtime/webcore/blob/Store.rs +++ b/src/runtime/webcore/blob/Store.rs @@ -126,8 +126,7 @@ impl StoreExt for Store { mime_type: Option, credentials: S3Credentials, ) -> Result, crate::Error> { - let mut path = pathlike; - path.make_thread_isolated_copy(); + let path = pathlike.thread_isolated_copy(); // Compute the extension-derived fallback before moving `path` into the // Store so we don't need to clone the owned PathLike. From 2f2c06930e9fcbc7cfb4e77e1a49d2a98c0065df Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 26 Aug 2026 02:02:37 +0000 Subject: [PATCH 25/25] docs: PathLike::thread_isolated_copy name in CLAUDE.md No-Verification-Needed: doc-only --- src/CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CLAUDE.md b/src/CLAUDE.md index 5e7d653ec17c..c5bf0934ed14 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -124,7 +124,7 @@ string, or Rust-side bytes: `PathLike::borrowed(bytes)` lends `&'a [u8]` to a synchronous call, `PathLike::owned(vec)` when the value must own them), `Buffer`. Anything parsed from JS, stored, or sent to another thread (`make_thread_isolated` / `ThreadIsolated` for the fs `args::*<'static>` -async path, `PathLike::make_thread_shareable` for a `Blob` store) is `'static`. +async path, `PathLike::thread_isolated_copy` for a `Blob` store) is `'static`. `EncodedSlice<'a>` is the `{ptr, len}` + encoding-bits (Latin-1/UTF-8/UTF-16) borrowed view handed to C++. Constructors name the encoding of the bytes: