From bee12a08d9a11c0cee50a37c3b691534bc0d2728 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 14 Aug 2026 19:55:26 +0000 Subject: [PATCH 1/3] valkey: reach the client through &JSValkeyClient, not through &SubscriptionCtx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SubscriptionCtx` is three plain bools, so rustc passes `&SubscriptionCtx` as `noalias readonly dereferenceable(3)`; its methods recovered the surrounding client with `impl_field_parent!` (container_of on `&self`) and wrote through the result — refcount bumps, poll refs — which is UB, and in release builds LLVM is entitled to delete those stores. It does once `ScopedRef::new` gets inlined into `upsert_receive_handler`: the +1 of `on_new_subscription_callback_insert`'s guard disappears while the out-of-line -1 in its drop stays, so a `subscribe()` issued from an `onclose`/rejection continuation drops the client's count by one and the client is freed under its live wrapper (heap-use-after-free in `js_disconnect` / `finalize`, or `deinit` with a timer ref still held). Whether it bites depends only on inlining decisions, which is why it surfaced with unrelated changes elsewhere in the crate. The bookkeeping that needs the client (its `this` value, poll ref, refcount) now lives on `JSValkeyClient`; `SubscriptionCtx` keeps only its flags and `init`, and its container_of accessor is gone. --- src/runtime/valkey_jsc/js_valkey.rs | 41 ++++++------- src/runtime/valkey_jsc/js_valkey_functions.rs | 57 +++++++------------ src/runtime/valkey_jsc/valkey.rs | 2 +- 3 files changed, 36 insertions(+), 64 deletions(-) diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index c40332f9b48a..37c8889bc703 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -53,10 +53,6 @@ pub struct SubscriptionCtx { /// free-fns plus `to_js`/`from_js`. Re-exported here as `Js`. pub use crate::generated_classes::js_RedisClient as Js; -// SAFETY: `SubscriptionCtx` lives at `JSValkeyClient._subscription_ctx` -// (intrusive backref). `JsCell` is `#[repr(transparent)]`. -bun_core::impl_field_parent! { SubscriptionCtx => JSValkeyClient._subscription_ctx; fn parent; } - impl SubscriptionCtx { pub(crate) fn init(valkey_parent: &JSValkeyClient) -> JsResult { let callback_map = JSMap::create(&valkey_parent.global_object); @@ -82,14 +78,14 @@ impl SubscriptionCtx { is_subscriber: false, }) } +} +/// The subscription bookkeeping that needs the client (its `this` value, poll +/// ref, refcount) lives on the client: `SubscriptionCtx` itself is three flags, +/// and a `&SubscriptionCtx` carries no right to reach the client around it. +impl JSValkeyClient { fn subscription_callback_map(&self) -> &mut JSMap { - let parent_this = self - .parent() - .this_value - .get() - .try_get() - .expect("unreachable"); + let parent_this = self.this_value.get().try_get().expect("unreachable"); let value_js = Js::subscription_callback_map_get_cached(parent_this).unwrap(); // `JSMap` is an `opaque_ffi!` ZST — `opaque_mut` is the safe deref. // `from_js` returns a non-null heap cell when the slot was set by @@ -183,7 +179,7 @@ impl SubscriptionCtx { ) -> JsResult<()> { // `BackRef` (Copy + Deref) detaches the borrow so the guard closure is // safe even though intervening JS may re-enter `&self`. - let parent_br = BackRef::new(self.parent()); + let parent_br = BackRef::new(self); let _guard = scopeguard::guard(parent_br, |p| { p.on_new_subscription_callback_insert(); }); @@ -264,7 +260,7 @@ impl SubscriptionCtx { // The user may, for example, unsubscribe in the callbacks, or even stop the client. // `BackRef` (Copy + Deref) detaches the borrow so the guard closure is // safe even though intervening JS may re-enter `&self`. - let parent_br = BackRef::new(self.parent()); + let parent_br = BackRef::new(self); let _update = scopeguard::guard(parent_br, |p| p.update_poll_ref()); // If callbacks is an array, iterate and call each one @@ -279,19 +275,17 @@ impl SubscriptionCtx { Ok(()) } - fn is_deletable(&self) -> bool { + fn subscription_ctx_is_deletable(&self) -> bool { // The user may request .close(), in which case we can dispose of the subscription object. // If that is the case, finalized will be true. Otherwise, we should treat the object as // disposable if there are no active subscriptions. - self.parent().client.get().flags.finalized || !self.has_subscriptions() + self.client.get().flags.finalized || !self.has_subscriptions() } - // Cannot be `Drop` — takes a `global_object` param. Exposed as explicit - // `close` per PORTING.md (never expose `pub fn deinit`). - pub fn close(&self, global_object: &JSGlobalObject) { - debug_assert!(self.is_deletable()); + pub fn close_subscription_ctx(&self, global_object: &JSGlobalObject) { + debug_assert!(self.subscription_ctx_is_deletable()); - if let Some(parent_this) = self.parent().this_value.get().try_get() { + if let Some(parent_this) = self.this_value.get().try_get() { Js::subscription_callback_map_set_cached( parent_this, global_object, @@ -949,12 +943,12 @@ impl JSValkeyClient { pub(crate) fn remove_subscription(&self) { debug!( "removeSubscription: entering, has subscriptions: {}", - self._subscription_ctx.get().has_subscriptions() + self.has_subscriptions() ); let _guard = self.ref_scope(); // This is the last subscription, restore original flags - if !self._subscription_ctx.get().has_subscriptions() { + if !self.has_subscriptions() { let (q, p) = { let s = self._subscription_ctx.get(); ( @@ -1332,8 +1326,6 @@ impl JSValkeyClient { // Invoke callbacks for this channel with message and channel as arguments if self - ._subscription_ctx - .get() .invoke_callbacks( &global_object, channel_value, @@ -1655,8 +1647,7 @@ impl JSValkeyClient { // in `subscriptionCallbackMap()` because `this_value.tryGet()` returns // null for a finalized ref. Short-circuit here: a finalized client has // no subscriptions by definition. - let subs_deletable: bool = - self.client.get().flags.finalized || !self._subscription_ctx.get().has_subscriptions(); + let subs_deletable: bool = self.client.get().flags.finalized || !self.has_subscriptions(); let has_activity = has_pending_commands || !subs_deletable || self.client.get().flags.is_reconnecting; diff --git a/src/runtime/valkey_jsc/js_valkey_functions.rs b/src/runtime/valkey_jsc/js_valkey_functions.rs index 41e8b945bc8b..468ba9104e57 100644 --- a/src/runtime/valkey_jsc/js_valkey_functions.rs +++ b/src/runtime/valkey_jsc/js_valkey_functions.rs @@ -1801,11 +1801,7 @@ impl JSValkeyClient { // This is less-than-ideal, still, because this assumes a happy path. What happens if // the SUBSCRIBE command fails? We have no way to roll back the addition of the // handler. - this._subscription_ctx.get().upsert_receive_handler( - global, - channel_arg, - handler_callback, - )?; + this.upsert_receive_handler(global, channel_arg, handler_callback)?; } } else if channel_or_many.is_string() { // It is a single string channel @@ -1814,11 +1810,7 @@ impl JSValkeyClient { }; redis_channels.push(channel); - this._subscription_ctx.get().upsert_receive_handler( - global, - channel_or_many, - handler_callback, - )?; + this.upsert_receive_handler(global, channel_or_many, handler_callback)?; } else { return Err(global.throw_invalid_argument_type( "subscribe", @@ -1836,9 +1828,7 @@ impl JSValkeyClient { Ok(p) => p, Err(err) => { // If we catch an error, we need to clean up any handlers we may have added and fall out of subscription mode - this._subscription_ctx - .get() - .clear_all_receive_handlers(global)?; + this.clear_all_receive_handlers(global)?; return send_err_to_js(global, "Failed to send SUBSCRIBE command", &err); } }; @@ -1885,9 +1875,7 @@ impl JSValkeyClient { // If no arguments, unsubscribe from all channels if args_view.is_empty() { - this._subscription_ctx - .get() - .clear_all_receive_handlers(global)?; + this.clear_all_receive_handlers(global)?; return Self::send_unsubscribe_request_and_cleanup( this, frame.this(), @@ -1935,22 +1923,19 @@ impl JSValkeyClient { }; redis_channels.push(ch); - let remaining_listeners = match this._subscription_ctx.get().remove_receive_handler( - global, - channel, - listener_cb, - ) { - Ok(Some(n)) => n, - Ok(None) => { - // Listeners weren't present in the first place, so we can return a - // resolved promise. - return Ok(JSPromise::resolved_promise_value( - global, - JSValue::UNDEFINED, - )); - } - Err(e) => return Err(e), - }; + let remaining_listeners = + match this.remove_receive_handler(global, channel, listener_cb) { + Ok(Some(n)) => n, + Ok(None) => { + // Listeners weren't present in the first place, so we can return a + // resolved promise. + return Ok(JSPromise::resolved_promise_value( + global, + JSValue::UNDEFINED, + )); + } + Err(e) => return Err(e), + }; // In this case, we only want to send the unsubscribe command to redis if there are no more listeners for this // channel. @@ -1992,9 +1977,7 @@ impl JSValkeyClient { }; redis_channels.push(channel); // Clear the handlers for this channel - this._subscription_ctx - .get() - .clear_receive_handlers(global, channel_arg)?; + this.clear_receive_handlers(global, channel_arg)?; } } else if channel_or_many.is_string() { // It is a single string channel @@ -2003,9 +1986,7 @@ impl JSValkeyClient { }; redis_channels.push(channel); // Clear the handlers for this channel - this._subscription_ctx - .get() - .clear_receive_handlers(global, channel_or_many)?; + this.clear_receive_handlers(global, channel_or_many)?; } else { return Err(global.throw_invalid_argument_type( "unsubscribe", diff --git a/src/runtime/valkey_jsc/valkey.rs b/src/runtime/valkey_jsc/valkey.rs index 4172cf89a086..3339ffdc33b6 100644 --- a/src/runtime/valkey_jsc/valkey.rs +++ b/src/runtime/valkey_jsc/valkey.rs @@ -915,7 +915,7 @@ impl ValkeyClient { } RESPValue::Push(push) => { let p = self.parent(); - let sub_count = p._subscription_ctx.get().channels_subscribed_to_count(); + let sub_count = p.channels_subscribed_to_count(); let is_pattern_or_sharded = protocol::SubscriptionPushMessage::is_reply_kind(&push.kind); From b34d49df1f92849c3571c0b571ef29c6144d9d82 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 14 Aug 2026 20:54:49 +0000 Subject: [PATCH 2/3] impl_field_parent!: no parent access through a `&self` that LLVM may treat as readonly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `impl_field_parent!` recovers a struct's parent from one of its fields (`@fieldParentPtr`). Its `&self -> &Parent` form is unsound for a `Freeze` child: rustc passes such a `&self` as `noalias readonly`, so stores made through the recovered parent (refcount bumps, flag sets) can be deleted by LLVM depending on inlining — which is how `SubscriptionCtx` over-released `JSValkeyClient` in release builds. - The macro now offers `&mut self -> &Parent` / `-> *mut Parent` arms (sound under Tree Borrows and LLVM: the parent pointer is based on the argument), and `shared`/`raw` `&self` arms that fail to compile when the child is `Freeze` (`assert_not_freeze!`, exported for hand-rolled sites too). - ValkeyClient, MySQLConnection, CapturedWriter, ByteBlobLoader, Assets and Execution move to the `&mut self` arms; CapturedWriter's `is_done` moves to `PipeReader`, and Assets drops two debug-only reads through `&self`. - FileReader/ByteStream keep `&self` access via the checked arms; raw `*mut Context` sites use the new `NewSource::from_context_ptr` instead. - `container_of` docs spell out what each derivation permits; `bun_ptr` gains Miri (Tree Borrows) tests for every arm. --- src/bun_core/lib.rs | 204 +++++++++++++------- src/ptr/lib.rs | 128 +++++++++++- src/runtime/bake/dev_server/assets.rs | 6 +- src/runtime/bake/dev_server/mod.rs | 23 +-- src/runtime/shell/subproc.rs | 92 ++++----- src/runtime/test_runner/Execution.rs | 6 +- src/runtime/timer/timer_object_internals.rs | 1 + src/runtime/valkey_jsc/valkey.rs | 17 +- src/runtime/webcore/ByteBlobLoader.rs | 4 +- src/runtime/webcore/ByteStream.rs | 7 +- src/runtime/webcore/FileReader.rs | 14 +- src/runtime/webcore/ReadableStream.rs | 21 +- src/runtime/webcore/fetch/FetchTasklet.rs | 2 +- src/sql_jsc/mysql/MySQLConnection.rs | 11 +- 14 files changed, 361 insertions(+), 175 deletions(-) diff --git a/src/bun_core/lib.rs b/src/bun_core/lib.rs index c69c6a85b16b..78466c987380 100644 --- a/src/bun_core/lib.rs +++ b/src/bun_core/lib.rs @@ -1,6 +1,7 @@ #![feature(allocator_api)] #![feature(adt_const_params)] #![feature(thread_local)] // bare `__thread` slot for `thread_id::current()` cache +#![feature(freeze)] // `impl_field_parent!`'s `shared` arm rejects `Freeze` children at compile time #![allow(non_snake_case, non_camel_case_types, non_upper_case_globals)] // bun_core is the T0 foundation crate that bun_threading, bun_sys, and // bun_collections depend on; importing any of them to satisfy the disallowed-* @@ -651,11 +652,22 @@ pub use util::*; /// `core::mem::offset_of!` so the field name is type-checked. /// /// # Safety -/// - `field` must have been derived from a live `P` via -/// `addr_of!((*p).field)` / `addr_of_mut!` (or equivalent), so its -/// provenance covers the entire `P` allocation — a `&mut field` reborrow -/// does **not** suffice. -/// - `offset` must equal `offset_of!(P, )`. +/// - `field` must point at the `F` field of a live `P`, and +/// `offset == offset_of!(P, )`. +/// - What may be done with the result depends on where `field` came from: +/// - a raw place projection of a whole-`P` pointer (`&raw mut (*p).field`, +/// an intrusive node handed back by C): anything the original `p` allowed. +/// - `ptr::from_mut(field_ref)` for a `&mut F`: reads and writes of `P` are +/// fine (Tree Borrows initialises out-of-range locations lazily for a +/// `&mut`-derived tag; LLVM sees the result as based on the argument), for +/// as long as that `&mut F` could itself be used. +/// - `ptr::from_ref(field_ref)` for a `&F`: reads of `P` are fine. Writes +/// (including `Cell::set` on a `P` field) are only defined when `F` is +/// **not** [`Freeze`](core::marker::Freeze): rustc passes a `&F` to a +/// `Freeze` `F` as `noalias readonly`, so a store through anything derived +/// from it is UB that release builds do exploit — the store is deleted. +/// Use [`impl_field_parent!`]'s `shared` arm instead of open-coding this +/// case; it rejects `Freeze` children at compile time. #[inline(always)] pub const unsafe fn container_of(field: *const F, offset: usize) -> *mut P { // SAFETY: per fn contract — `field` is interior to a `P`; `byte_sub` @@ -698,8 +710,9 @@ pub unsafe fn callback_ctx<'a, T>(ctx: *mut core::ffi::c_void) -> &'a mut T { /// /// Type-checked wrapper over [`container_of`]: expands to /// `container_of::(ptr, offset_of!(Parent, field))`. The call is -/// `unsafe` (caller asserts `ptr` points at `Parent.field` with whole-`Parent` -/// provenance) and must appear inside an `unsafe` block. +/// `unsafe` (caller asserts `ptr` points at `Parent.field`; see +/// [`container_of`] for what the result may be used for depending on how +/// `ptr` was derived) and must appear inside an `unsafe` block. #[macro_export] macro_rules! from_field_ptr { ($Parent:ty, $field:ident, $ptr:expr $(,)?) => { @@ -708,109 +721,160 @@ macro_rules! from_field_ptr { } /// Stamp container-of-style back-reference accessors on a child type that -/// is **only ever constructed as the `$field` field of `$Parent`**. +/// is **only ever constructed as the `$field` field of `$Parent`** (a port of +/// Zig's `@fieldParentPtr`). /// -/// Five forms (mix-and-match is not supported; pick the one matching the call -/// site's receiver/return contract): /// ```ignore -/// // (1) ref + raw-mut pair (&self -> &P ; &mut self -> *mut P) -/// bun_core::impl_field_parent! { Assets => DevServer.assets; pub fn owner; fn owner_mut; } +/// bun_core::impl_field_parent! { ValkeyClient => JSValkeyClient.client; +/// fn parent; // (&mut self) -> &JSValkeyClient +/// fn mut parent_ptr; // (&mut self) -> *mut JSValkeyClient +/// } +/// bun_core::impl_field_parent! { FileReader => Source.context; +/// pub fn shared parent_const; // (&self) -> &Source — only compiles while FileReader: !Freeze +/// pub fn raw parent; // (&self) -> *mut Source — same check +/// } +/// ``` /// -/// // (2) ref-only (&self -> &P) -/// bun_core::impl_field_parent! { SubscriptionCtx => JSValkeyClient._subscription_ctx; fn parent; } +/// Any subset of the four arms, in any order. /// -/// // (3) mut-only (&mut self -> *mut P) -/// bun_core::impl_field_parent! { DirectoryWatchStore => DevServer.directory_watchers; fn mut owner; } +/// ## Why the receivers are what they are /// -/// // (4) nonnull (&mut self -> NonNull

) -/// bun_core::impl_field_parent! { Execution => BunTest.execution; fn nonnull bun_test; } +/// The parent pointer is computed from `self`, so it is only as capable as +/// the reference it came from: /// -/// // (5) raw (&self -> *mut P) -/// bun_core::impl_field_parent! { FileReader => Source.context; pub fn raw parent; } -/// ``` +/// - From `&mut self` (the plain and `mut` arms) the whole parent may be read +/// and written for as long as that `&mut self` is usable. Under Tree Borrows +/// the locations outside `self` are initialised lazily for the `&mut`'s tag, +/// and LLVM sees the parent pointer as *based on* the argument, so parent +/// code that comes back and touches `self`'s bytes (through a `JsCell`, say) +/// is correctly treated as aliasing. This is what makes the container-of +/// form preferable to a stored back-pointer for `&mut self` methods: a +/// pointer loaded from a field is *not* based on the argument, and `&mut +/// self` is `noalias`. +/// - From `&self` (the `shared` and `raw` arms) the parent may be read and +/// written only because `$Child` is not [`Freeze`](core::marker::Freeze). +/// For a `Freeze` child rustc passes `&self` as `noalias readonly` and LLVM +/// deletes stores made through anything derived from it — refcount bumps, +/// flag sets — depending purely on inlining. Both arms therefore fail to +/// compile for a `Freeze` `$Child`. If that assertion fires, do not add a +/// `Cell` to placate it: move the methods that need the parent onto +/// `&$Parent`, or take `&mut self`. `shared` gives `&$Parent` (so only its +/// interior-mutable fields are writable); `raw` gives the pointer for +/// `&self` methods that must call `&mut $Parent` methods or set plain +/// fields, under the usual no-other-live-reference rule. +/// +/// No arm hands out `&mut $Parent`: `self` is inside it, so that would be two +/// live `&mut` to the same bytes. The pointer-returning arms are dereferenced +/// at the point of use. /// -/// The mut accessor returns `*mut $Parent` (NOT `&mut`) because `self` is a -/// field of `$Parent` — materializing `&mut $Parent` while `&mut self` is live -/// would alias. Callers dereference under `unsafe` and must only touch fields -/// disjoint from `$field`. +/// None of this is defined under Stacked Borrows, which is why +/// `scripts/rust-miri.ts` runs Tree Borrows; `bun_ptr`'s tests exercise both +/// arms under Miri. /// /// # Safety /// Expanding this macro asserts that **every** `$Child` instance lives at /// `$Parent.$field` for its entire lifetime. If `$Child` can exist /// standalone, the generated accessors are unsound; keep a hand-rolled -/// `pub unsafe fn` instead. +/// `unsafe fn(*mut $Child) -> *mut $Parent` over [`from_field_ptr!`] instead. #[macro_export] macro_rules! impl_field_parent { - // ref + raw-mut pair - ($Child:ty => $Parent:ident . $field:ident ; $v:vis fn $ref_name:ident ; $vm:vis fn $mut_name:ident ;) => { + ($Child:ty => $Parent:ident . $field:ident ; $($arms:tt)+) => { + $crate::impl_field_parent!(@arm [$Child] [$Parent] [$field] $($arms)+); + }; + (@arm [$Child:ty] [$Parent:ident] [$field:ident]) => {}; + // (&mut self) -> *mut $Parent + (@arm [$Child:ty] [$Parent:ident] [$field:ident] $v:vis fn mut $name:ident ; $($rest:tt)*) => { impl $Child { #[inline] - $v fn $ref_name(&self) -> &$Parent { - // SAFETY: macro contract — `self` is the `$field` field of a - // live `$Parent`; recovering the parent and reborrowing as `&` - // for the lifetime of `&self` is sound. - unsafe { &*$crate::from_field_ptr!($Parent, $field, ::core::ptr::from_ref(self)) } - } - #[inline] - $vm fn $mut_name(&mut self) -> *mut $Parent { - // SAFETY: macro contract — pointer arithmetic only; no - // reference is formed here. + $v fn $name(&mut self) -> *mut $Parent { + // SAFETY: macro contract — `self` is `$Parent.$field`. unsafe { $crate::from_field_ptr!($Parent, $field, ::core::ptr::from_mut(self)) } } } + $crate::impl_field_parent!(@arm [$Child] [$Parent] [$field] $($rest)*); }; - // ref-only - ($Child:ty => $Parent:ident . $field:ident ; $v:vis fn $ref_name:ident ;) => { + // (&self) -> &$Parent, `$Child: !Freeze` enforced + (@arm [$Child:ty] [$Parent:ident] [$field:ident] $v:vis fn shared $name:ident ; $($rest:tt)*) => { + $crate::assert_not_freeze!($Child, $Parent); impl $Child { #[inline] - $v fn $ref_name(&self) -> &$Parent { - // SAFETY: macro contract — see two-arm form above. + $v fn $name(&self) -> &$Parent { + // SAFETY: macro contract — `self` is `$Parent.$field`; the + // assertion above guarantees `&self` carries no `readonly`. unsafe { &*$crate::from_field_ptr!($Parent, $field, ::core::ptr::from_ref(self)) } } } + $crate::impl_field_parent!(@arm [$Child] [$Parent] [$field] $($rest)*); }; - // mut-only: (&mut self) -> *mut $Parent - ($Child:ty => $Parent:ident . $field:ident ; $v:vis fn mut $name:ident ;) => { - impl $Child { - #[inline] - $v fn $name(&mut self) -> *mut $Parent { - // SAFETY: macro contract — pointer arithmetic only. - unsafe { $crate::from_field_ptr!($Parent, $field, ::core::ptr::from_mut(self)) } - } - } - }; - // nonnull: (&mut self) -> NonNull<$Parent> - ($Child:ty => $Parent:ident . $field:ident ; $v:vis fn nonnull $name:ident ;) => { + // (&self) -> *mut $Parent, `$Child: !Freeze` enforced. For `&self` + // methods that must reach non-cell parent state (`&mut $Parent` methods, + // plain fields): going through the `shared` arm's `&$Parent` first would + // freeze those bytes, so this hands back the pointer underived. + (@arm [$Child:ty] [$Parent:ident] [$field:ident] $v:vis fn raw $name:ident ; $($rest:tt)*) => { + $crate::assert_not_freeze!($Child, $Parent); impl $Child { #[inline] - $v fn $name(&mut self) -> ::core::ptr::NonNull<$Parent> { - // SAFETY: macro contract — `self` is non-null, so the - // recovered parent pointer is too. + $v fn $name(&self) -> *mut $Parent { + // SAFETY: macro contract — `self` is `$Parent.$field`. unsafe { - ::core::ptr::NonNull::new_unchecked( - $crate::from_field_ptr!($Parent, $field, ::core::ptr::from_mut(self)), - ) + $crate::from_field_ptr!($Parent, $field, ::core::ptr::from_ref(self).cast_mut()) } } } + $crate::impl_field_parent!(@arm [$Child] [$Parent] [$field] $($rest)*); }; - // raw: (&self) -> *mut $Parent (read-only receiver, raw out — for FFI - // callback shapes that round-trip through `*const Self` but need a - // `*mut Parent` without forming an aliased `&mut`) - ($Child:ty => $Parent:ident . $field:ident ; $v:vis fn raw $name:ident ;) => { + // (&mut self) -> &$Parent + (@arm [$Child:ty] [$Parent:ident] [$field:ident] $v:vis fn $name:ident ; $($rest:tt)*) => { impl $Child { #[inline] - $v fn $name(&self) -> *mut $Parent { - // SAFETY: macro contract — pointer arithmetic only; the - // returned pointer is not dereferenced here. - unsafe { - $crate::from_field_ptr!($Parent, $field, ::core::ptr::from_ref(self).cast_mut()) - } + $v fn $name(&mut self) -> &$Parent { + // SAFETY: macro contract — `self` is `$Parent.$field`; the + // returned borrow keeps `self` mutably borrowed, so no `&mut` + // to any part of `$Parent` is usable while it lives. + unsafe { &*$crate::from_field_ptr!($Parent, $field, ::core::ptr::from_mut(self)) } } } + $crate::impl_field_parent!(@arm [$Child] [$Parent] [$field] $($rest)*); + }; +} + +/// Compile-time `assert!($Child: !Freeze)`, for code that reaches a parent +/// through a pointer derived from `&$Child` (see [`container_of`]). rustc +/// passes a `&T` to a [`Freeze`](core::marker::Freeze) `T` as `noalias +/// readonly`, so any store through such a pointer is deleted in release +/// builds; this turns "someone removed the last `Cell` from `$Child`" into a +/// build error at the site that depends on it instead of a heap corruption. +#[macro_export] +macro_rules! assert_not_freeze { + ($Child:ty, $Parent:ty $(,)?) => { + const _: () = { + #[allow(unused_imports)] + use $crate::__NotFreeze as _; + ::core::assert!( + !<$crate::__IsFreeze<$Child>>::IS_FREEZE, + concat!( + "`", stringify!($Child), "` is Freeze, so `&self` is passed `noalias readonly` ", + "and writes to `", stringify!($Parent), "` through a pointer derived from it ", + "would be miscompiled. Put the methods that need the parent on `&", + stringify!($Parent), "` instead, or derive the pointer from `&mut self`." + ), + ); + }; }; } +#[doc(hidden)] +pub struct __IsFreeze(core::marker::PhantomData); +#[doc(hidden)] +pub trait __NotFreeze { + const IS_FREEZE: bool = false; +} +impl __NotFreeze for __IsFreeze {} +impl __IsFreeze { + // Inherent consts shadow trait consts, so this wins exactly when `T: Freeze`. + pub const IS_FREEZE: bool = true; +} + // ─── IntrusiveField ────────────────────────────────────────────────────── /// Declares that `Self` embeds exactly one intrusive `F` field at byte diff --git a/src/ptr/lib.rs b/src/ptr/lib.rs index 69a20c83394b..3740609d9a42 100644 --- a/src/ptr/lib.rs +++ b/src/ptr/lib.rs @@ -59,7 +59,8 @@ pub use weak_ptr::WeakPtr; // (lowest tier, every crate can reach them); re-exported here so callers can // spell `bun_ptr::container_of` / `bun_ptr::from_field_ptr!`. pub use bun_core::{ - IntrusiveField, container_of, from_field_ptr, impl_field_parent, intrusive_field, + IntrusiveField, assert_not_freeze, container_of, from_field_ptr, impl_field_parent, + intrusive_field, }; // C-callback `void *user_data` → `&mut T` recovery — same tiering rationale @@ -780,3 +781,128 @@ pub trait AsCtxPtr { } } impl AsCtxPtr for T {} + +#[cfg(test)] +mod container_of_tests { + //! The shapes `container_of` / `impl_field_parent!` are used in, kept + //! Miri-clean under Tree Borrows (`bun run rust:miri`). Stacked Borrows + //! rejects every reference-derived form by design; only + //! `raw_place_projection` is defined under both models. + use core::cell::{Cell, UnsafeCell}; + + struct Parent { + count: Cell, + plain: u32, + by_mut: ByMut, + by_shared: UnsafeCell, + } + /// A `Freeze` child reached through `&mut self`. + struct ByMut { + hits: u32, + } + /// A `!Freeze` child reached through `&self`, itself sitting in an + /// interior-mutable slot the parent writes through (the `JsCell` shape). + struct ByShared { + hits: Cell, + } + + bun_core::impl_field_parent! { ByMut => Parent.by_mut; fn parent; fn mut parent_ptr; } + bun_core::impl_field_parent! { ByShared => Parent.by_shared; fn shared parent; } + + impl Parent { + fn boxed() -> *mut Parent { + bun_core::heap::into_raw(Box::new(Parent { + count: Cell::new(0), + plain: 0, + by_mut: ByMut { hits: 0 }, + by_shared: UnsafeCell::new(ByShared { hits: Cell::new(0) }), + })) + } + /// Parent method that reaches back into the child it was called from. + fn bump_shared_child(&self) { + // SAFETY: single-threaded test; no `&mut ByShared` is live. + unsafe { (*self.by_shared.get()).hits.set((*self.by_shared.get()).hits.get() + 1) }; + self.count.set(self.count.get() + 1); + } + } + + impl ByMut { + fn touch(&mut self) { + self.hits += 1; + self.parent().count.set(10); + // SAFETY: `plain` is disjoint from `by_mut`; deref at point of use. + unsafe { (*self.parent_ptr()).plain = 20 }; + self.hits += 1; + } + } + + impl ByShared { + fn touch(&self) { + self.hits.set(1); + self.parent().count.set(30); + self.parent().bump_shared_child(); + assert_eq!(self.hits.get(), 2); + } + } + + #[test] + fn raw_place_projection() { + let p = Parent::boxed(); + // SAFETY: `p` is live; the field pointer keeps whole-`Parent` provenance. + unsafe { + let field = &raw mut (*p).by_mut; + let back: *mut Parent = bun_core::from_field_ptr!(Parent, by_mut, field); + assert!(core::ptr::eq(back, p)); + (*back).plain = 1; + (*back).by_mut.hits = 1; + assert_eq!(((*p).plain, (*p).by_mut.hits), (1, 1)); + drop(bun_core::heap::take(p)); + } + } + + /// `&mut self` arms, called the way the runtime calls them: through a + /// `&mut Parent` that is a protected function argument. + #[test] + fn mut_receiver_under_parent_borrow() { + fn drive(parent: &mut Parent) { + parent.by_mut.touch(); + assert_eq!((parent.count.get(), parent.plain, parent.by_mut.hits), (10, 20, 2)); + } + let p = Parent::boxed(); + // SAFETY: `p` is live and uniquely owned here. + unsafe { + drive(&mut *p); + drop(bun_core::heap::take(p)); + } + } + + /// `shared` arm: `&self` on a `!Freeze` child, writing the parent's cells + /// and letting the parent write back into the child mid-call. + #[test] + fn shared_receiver_with_reentrant_parent() { + fn drive(parent: &Parent) { + // SAFETY: single-threaded test; no `&mut ByShared` is live. + unsafe { (*parent.by_shared.get()).touch() }; + assert_eq!(parent.count.get(), 31); + } + let p = Parent::boxed(); + // SAFETY: `p` is live. + unsafe { + drive(&*p); + drop(bun_core::heap::take(p)); + } + } + + #[test] + fn freeze_detection() { + use bun_core::__NotFreeze as _; + const { + assert!(>::IS_FREEZE); + assert!(!>::IS_FREEZE); + assert!(!>::IS_FREEZE); + // Interior mutability behind a pointer does not count. + assert!(>>>::IS_FREEZE); + } + bun_core::assert_not_freeze!(ByShared, Parent); + } +} diff --git a/src/runtime/bake/dev_server/assets.rs b/src/runtime/bake/dev_server/assets.rs index 2667f7b77d99..1057d6f3e7dd 100644 --- a/src/runtime/bake/dev_server/assets.rs +++ b/src/runtime/bake/dev_server/assets.rs @@ -30,11 +30,10 @@ pub struct Assets { // SAFETY: `Assets` is only ever constructed as the `assets` field of // `DevServer` (which is `Box`-allocated and never moved post-init). -bun_core::impl_field_parent! { Assets => DevServer.assets; pub(super) fn owner; fn owner_mut; } +bun_core::impl_field_parent! { Assets => DevServer.assets; fn owner; fn mut owner_mut; } impl Assets { pub(crate) fn get_hash(&self, path: &[u8]) -> Option { - debug_assert!(self.owner().magic == Magic::Valid); self.path_map .get(path) .map(|idx| self.files.keys()[idx.get_usize()]) @@ -69,8 +68,6 @@ impl Assets { bstr::BStr::new(&*mime_type.value), ); - // Captured up-front so borrows of `self.files` / `self.path_map` below don't - // overlap with `owner()` (`&self`) calls. let server = self.owner().server; debug_assert!(server.is_some()); @@ -193,7 +190,6 @@ impl Assets { /// Look up a `StaticRoute` by content hash. pub(crate) fn get(&self, content_hash: u64) -> Option<&StaticRouteRef> { - debug_assert!(self.owner().magic == Magic::Valid); debug_assert_eq!(self.files.count(), self.refs.len()); self.files.get(&content_hash) } diff --git a/src/runtime/bake/dev_server/mod.rs b/src/runtime/bake/dev_server/mod.rs index 11a81c24178c..71a4951daf9b 100644 --- a/src/runtime/bake/dev_server/mod.rs +++ b/src/runtime/bake/dev_server/mod.rs @@ -1029,24 +1029,13 @@ pub struct DirectoryWatchStore { /// Dependencies cannot be re-ordered. This list tracks what indexes are free. pub(crate) dependencies_free_list: Vec, } -impl DirectoryWatchStore { - /// Intrusive backref: recover `*mut DevServer`. - /// Returns a raw ptr (not `&mut DevServer`) because `&mut self` is live; - /// callers must scope their borrow of fields disjoint from - /// `directory_watchers` to avoid aliasing UB. - #[inline] - fn owner(&mut self) -> *mut DevServer { - // SAFETY: `DirectoryWatchStore` is only ever the `directory_watchers` - // field of a heap-allocated `DevServer` (never moved post-init). - unsafe { - bun_core::from_field_ptr!( - DevServer, - directory_watchers, - std::ptr::from_mut::(self) - ) - } - } +// SAFETY: `DirectoryWatchStore` is only ever the `directory_watchers` field of +// a heap-allocated `DevServer` (never moved post-init). Raw ptr (not `&mut +// DevServer`) because `&mut self` is live; callers scope their borrows to +// fields disjoint from `directory_watchers`. +bun_core::impl_field_parent! { DirectoryWatchStore => DevServer.directory_watchers; fn mut owner; } +impl DirectoryWatchStore { /// Safe sibling-projection: borrow the owning [`DevServer`]'s /// `bun_watcher` while holding `&mut self`. The two fields are disjoint, /// so the returned `&mut Watcher` does not alias `self`. diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 2eee13052d29..2a934c93e0b5 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1636,7 +1636,7 @@ impl Default for CapturedWriter { } } -bun_core::impl_field_parent! { CapturedWriter => PipeReader.captured_writer; pub fn parent; fn parent_mut; } +bun_core::impl_field_parent! { CapturedWriter => PipeReader.captured_writer; fn parent; fn mut parent_mut; } impl CapturedWriter { pub(crate) fn do_write(&mut self, chunk: &[u8]) { @@ -1644,12 +1644,14 @@ impl CapturedWriter { return; } + let addr = std::ptr::from_mut(self) as usize; + let parent = self.parent(); log!( "CapturedWriter(0x{:x}, {}) doWrite len={} parent_amount={}", - std::ptr::from_mut(self) as usize, - out_kind_str(self.parent().out_type), + addr, + out_kind_str(parent.out_type), chunk.len(), - self.parent().buffered_output.len() + parent.buffered_output.len() ); // `dead == false` ⇒ writer.is_some() (set in PipeReader::create). let writer = self @@ -1661,47 +1663,29 @@ impl CapturedWriter { // `io_writer::ChildPtr::subproc_capture` / `WriterTag::Subproc`. let child = io_writer::ChildPtr::subproc_capture(std::ptr::from_mut(self).cast::()); let y = writer.enqueue(child, None, chunk); - // `parent()` recovers the enclosing `PipeReader` via the same - // `from_field_ptr!` projection (encapsulated once there). The `&mut - // self` access above is finished, so the shared `&PipeReader` is fine. self.parent().run_yield(y); } - pub(crate) fn is_done(&self, just_written: usize) -> bool { - log!( - "CapturedWriter(0x{:x}, {}) isDone(has_err={}, parent_state={}, written={}, parent_amount={})", - std::ptr::from_ref(self) as usize, - out_kind_str(self.parent().out_type), - self.err.is_some(), - <&'static str>::from(&self.parent().state), - self.written, - self.parent().buffered_output.len() - ); - if self.dead || self.err.is_some() { - return true; - } - let p = self.parent(); - if matches!(p.state, PipeReaderState::Pending) { - return false; - } - self.written + just_written >= self.parent().buffered_output.len() - } - pub(crate) fn on_iowriter_chunk(&mut self, amount: usize, err: Option) -> Yield { + let addr = std::ptr::from_mut(self) as usize; + let written = self.written + amount; + let parent = self.parent(); log!( "CapturedWriter({:x}, {}) onWrite({}, has_err={}) total_written={} total_to_write={}", - std::ptr::from_mut(self) as usize, - out_kind_str(self.parent().out_type), + addr, + out_kind_str(parent.out_type), amount, err.is_some(), - self.written + amount, - self.parent().buffered_output.len() + written, + parent.buffered_output.len() ); - self.written += amount; + let all_written = written >= parent.buffered_output.len() + && !matches!(parent.state, PipeReaderState::Pending); + self.written = written; if let Some(e) = err { log!( "CapturedWriter(0x{:x}, {}) onWrite errno={} errmsg={} errfd={:?} syscall={}", - std::ptr::from_mut(self) as usize, + addr, out_kind_str(self.parent().out_type), e.errno, e.message, @@ -1709,17 +1693,35 @@ impl CapturedWriter { e.syscall ); self.err = Some(e); - // SAFETY: `parent_mut` recovers the embedding `PipeReader` via - // `container_of`; raw-ptr form per `try_signal_done_to_cmd` - // contract (no `&mut PipeReader` held across the Cmd re-entry). - return unsafe { PipeReader::try_signal_done_to_cmd(self.parent_mut()) }; - } else if self.written >= self.parent().buffered_output.len() - && !matches!(self.parent().state, PipeReaderState::Pending) - { - // SAFETY: as above. - return unsafe { PipeReader::try_signal_done_to_cmd(self.parent_mut()) }; + } else if !all_written { + return Yield::Suspended; } - Yield::Suspended + // SAFETY: `parent_mut` recovers the embedding `PipeReader`; raw-ptr + // form per `try_signal_done_to_cmd` contract (no `&mut PipeReader` + // held across the Cmd re-entry). + unsafe { PipeReader::try_signal_done_to_cmd(self.parent_mut()) } + } +} + +impl PipeReader { + fn captured_writer_done(&self, just_written: usize) -> bool { + let cw = &self.captured_writer; + log!( + "CapturedWriter(0x{:x}, {}) isDone(has_err={}, parent_state={}, written={}, parent_amount={})", + std::ptr::from_ref(cw) as usize, + out_kind_str(self.out_type), + cw.err.is_some(), + <&'static str>::from(&self.state), + cw.written, + self.buffered_output.len() + ); + if cw.dead || cw.err.is_some() { + return true; + } + if matches!(self.state, PipeReaderState::Pending) { + return false; + } + cw.written + just_written >= self.buffered_output.len() } } @@ -1745,12 +1747,12 @@ impl PipeReader { std::ptr::from_ref(self) as usize, out_kind_str(self.out_type), <&'static str>::from(&self.state), - self.captured_writer.is_done(0) + self.captured_writer_done(0) ); if matches!(self.state, PipeReaderState::Pending) { return false; } - self.captured_writer.is_done(0) + self.captured_writer_done(0) } /// Drive a `Yield` from inside an async I/O callback. Mirrors diff --git a/src/runtime/test_runner/Execution.rs b/src/runtime/test_runner/Execution.rs index eaa66cf9f051..a864c8f8b17b 100644 --- a/src/runtime/test_runner/Execution.rs +++ b/src/runtime/test_runner/Execution.rs @@ -268,11 +268,11 @@ impl Result { } } -// Recover the parent `BunTest` from `&mut self`. Returns `NonNull` (not +// Recover the parent `BunTest` from `&mut self`. Returns `*mut BunTest` (not // `&mut BunTest`) because `self` *is* `BunTest.execution`, so materializing a // `&mut BunTest` while `&mut self` is live would be aliased-`&mut` UB. Callers // must dereference at point-of-use into disjoint fields only. -bun_core::impl_field_parent! { Execution => BunTest.execution; fn nonnull bun_test; } +bun_core::impl_field_parent! { Execution => BunTest.execution; fn mut bun_test; } impl Execution { pub(crate) fn init() -> Execution { @@ -329,7 +329,7 @@ impl Execution { let buntest = self.bun_test(); // SAFETY: deref parent at point-of-use; `self` is not accessed while this `&mut BunTest` is live. - unsafe { (*buntest.as_ptr()).add_result(RefDataValue::Start) }; + unsafe { (*buntest).add_result(RefDataValue::Start) }; Ok(()) } diff --git a/src/runtime/timer/timer_object_internals.rs b/src/runtime/timer/timer_object_internals.rs index 78eb00beff77..039f7b610e36 100644 --- a/src/runtime/timer/timer_object_internals.rs +++ b/src/runtime/timer/timer_object_internals.rs @@ -100,6 +100,7 @@ impl TimerObjectInternals { /// lives in exactly one place. #[inline] fn parent_ptr(&self) -> TimerParent { + bun_core::assert_not_freeze!(TimerObjectInternals, TimerParent); let this = std::ptr::from_ref::(self).cast_mut(); match self.flags.get().kind() { // SAFETY: `kind == SetImmediate` ⇒ `self` is the `internals` field diff --git a/src/runtime/valkey_jsc/valkey.rs b/src/runtime/valkey_jsc/valkey.rs index 3339ffdc33b6..68ab3d8cbd8b 100644 --- a/src/runtime/valkey_jsc/valkey.rs +++ b/src/runtime/valkey_jsc/valkey.rs @@ -326,11 +326,10 @@ fn reader_pos(reader: &protocol::ValkeyReader<'_>) -> usize { reader.pos() } -// SAFETY: `ValkeyClient` lives at `JSValkeyClient.client` (intrusive embed via -// `container_of`). `JsCell` is `#[repr(transparent)]`, so the -// field offset is unchanged. R-2: shared `&` only — every `JSValkeyClient` -// method this reaches is `&self`. -bun_core::impl_field_parent! { ValkeyClient => JSValkeyClient.client; fn parent; } +// SAFETY: `ValkeyClient` lives at `JSValkeyClient.client` (intrusive embed). +// `JsCell` is `#[repr(transparent)]`, so the field offset is +// unchanged. Every `JSValkeyClient` method this reaches is `&self`. +bun_core::impl_field_parent! { ValkeyClient => JSValkeyClient.client; fn parent; fn mut parent_ptr; } impl ValkeyClient { /// Clean up resources used by the Valkey client @@ -914,8 +913,7 @@ impl ValkeyClient { Ok(SubscribeHandled::Handled) } RESPValue::Push(push) => { - let p = self.parent(); - let sub_count = p.channels_subscribed_to_count(); + let sub_count = self.parent().channels_subscribed_to_count(); let is_pattern_or_sharded = protocol::SubscriptionPushMessage::is_reply_kind(&push.kind); @@ -929,7 +927,7 @@ impl ValkeyClient { Ok(SubscribeHandled::Handled) } protocol::SubscriptionPushMessage::Subscribe => { - p.add_subscription(); + self.parent().add_subscription(); self.on_valkey_subscribe(value); // For SUBSCRIBE responses, only resolve the promise for the first channel confirmation @@ -1527,11 +1525,10 @@ impl ValkeyClient { } pub fn deref(&mut self) { - let parent = std::ptr::from_ref(self.parent()).cast_mut(); // SAFETY: only called in balanced `ref_()`/`deref()` pairs // (`on_auto_flush`, `on_writable`), so the count stays > 0 and the // outer `&mut self` protector is never invalidated by deallocation. - unsafe { JSValkeyClient::deref(parent) }; + unsafe { JSValkeyClient::deref(self.parent_ptr()) }; } #[inline] diff --git a/src/runtime/webcore/ByteBlobLoader.rs b/src/runtime/webcore/ByteBlobLoader.rs index 2c5d6e2c8066..28c426ff8b4d 100644 --- a/src/runtime/webcore/ByteBlobLoader.rs +++ b/src/runtime/webcore/ByteBlobLoader.rs @@ -69,7 +69,7 @@ impl readable_stream::SourceContext for ByteBlobLoader { } } -bun_core::impl_field_parent! { ByteBlobLoader => Source.context; pub fn parent_const; pub fn parent; } +bun_core::impl_field_parent! { ByteBlobLoader => Source.context; fn parent; } impl ByteBlobLoader { pub(crate) fn setup(&mut self, blob: &Blob, user_chunk_size: blob::SizeType) { @@ -166,7 +166,7 @@ impl ByteBlobLoader { blob.content_type.set(ct); } - self.parent_const().is_closed.set(true); + self.parent().is_closed.set(true); Some(blob::Any::Blob(blob)) } diff --git a/src/runtime/webcore/ByteStream.rs b/src/runtime/webcore/ByteStream.rs index a117314b6775..d4374e065fb3 100644 --- a/src/runtime/webcore/ByteStream.rs +++ b/src/runtime/webcore/ByteStream.rs @@ -103,10 +103,9 @@ impl readable_stream::SourceContext for ByteStream { } // SAFETY: `ByteStream` is always the `context` field of a `Source` -// (ReadableStream.NewSource); never constructed standalone. `parent` returns -// `*mut Source` (not `&mut`) — retained for the `finalize` (GC-teardown) path -// only; all host-fn-reachable callers use `parent_const`. -bun_core::impl_field_parent! { ByteStream => Source.context; pub fn parent_const; pub fn parent; } +// (ReadableStream.NewSource); never constructed standalone. Everything it +// touches on the `Source` is a `Cell`, so the `&Source` arm suffices. +bun_core::impl_field_parent! { ByteStream => Source.context; pub fn shared parent_const; } impl ByteStream { #[inline] diff --git a/src/runtime/webcore/FileReader.rs b/src/runtime/webcore/FileReader.rs index bb983f38be73..8c3ca7555f23 100644 --- a/src/runtime/webcore/FileReader.rs +++ b/src/runtime/webcore/FileReader.rs @@ -1262,14 +1262,12 @@ impl FileReader { pub type Source = readable_stream::NewSource; -// Intrusive backref: `self` is always the `context` field of a heap-allocated -// `Source`. Returns `*mut Source` -// (NOT `&mut Source`) because `self` IS the `context` field — materializing -// `&mut Source` would alias the live `&self` borrow. Callers deref in a tight -// `unsafe { (*ptr).method() }` scope and never hold `&mut Source` across other -// `self.*` accesses. -bun_core::impl_field_parent! { FileReader => Source.context; pub fn raw parent; } -bun_core::impl_field_parent! { FileReader => Source.context; pub fn parent_const; } +// SAFETY: `FileReader` is always the `context` field of a heap-allocated +// `Source`. `parent` is the `raw` arm because the ref-count pin +// (`increment_count`/`decrement_count`) and `global_this` are plain `Source` +// fields; callers deref in a tight `unsafe { (*ptr).method() }` scope and never +// hold `&mut Source` across other `self.*` accesses. +bun_core::impl_field_parent! { FileReader => Source.context; pub fn raw parent; pub fn shared parent_const; } impl readable_stream::SourceContext for FileReader { const NAME: &'static str = "File"; diff --git a/src/runtime/webcore/ReadableStream.rs b/src/runtime/webcore/ReadableStream.rs index 799c4be50c09..8765510123f4 100644 --- a/src/runtime/webcore/ReadableStream.rs +++ b/src/runtime/webcore/ReadableStream.rs @@ -255,11 +255,11 @@ impl ReadableStream { // this will resolve any pending promises to done: true match self.ptr { // SAFETY: ptrs came from ReadableStreamTag__tagged; valid while stream alive. - Source::Blob(source) => unsafe { (*(*source).parent()).cancel() }, + Source::Blob(source) => unsafe { (*NewSource::from_context_ptr(source)).cancel() }, // SAFETY: ptr came from ReadableStreamTag__tagged; valid while stream alive. - Source::File(source) => unsafe { (*(*source).parent()).cancel() }, + Source::File(source) => unsafe { (*NewSource::from_context_ptr(source)).cancel() }, // SAFETY: ptr came from ReadableStreamTag__tagged; valid while stream alive. - Source::Bytes(source) => unsafe { (*(*source).parent()).cancel() }, + Source::Bytes(source) => unsafe { (*NewSource::from_context_ptr(source)).cancel() }, _ => {} } self.detach_if_possible(global_this); @@ -846,8 +846,9 @@ pub struct NewSource { /// `Finalized` so [`Self::on_js_close`] reads `None` instead of a /// dead-but-unswept cell. pub this_jsvalue: jsc::JsRef, - /// R-2: written by `&self` context methods (`ByteStream::to_any_blob`, - /// `ByteBlobLoader::to_any_blob`) via `parent_const()`, so interior-mutable. + /// R-2: written by context methods (`ByteStream::to_any_blob`, + /// `ByteBlobLoader::to_any_blob`) through their parent accessor, so + /// interior-mutable. pub is_closed: Cell, } @@ -1035,6 +1036,16 @@ impl NewSource { bun_core::heap::into_raw(Box::new(init)) } + /// Inverse of the `*mut Self as *mut C` cast [`ReadableStream::from_js`] + /// performs: `ctx` must be that pointer (whole-allocation provenance), not + /// one derived from a `&C`/`&mut C` — use the context's + /// `impl_field_parent!` accessors for those. + #[inline] + pub unsafe fn from_context_ptr(ctx: *mut C) -> *mut Self { + // SAFETY: caller contract. + unsafe { bun_core::from_field_ptr!(Self, context, ctx) } + } + /// [`Self::new`] returning the leaked allocation as an unbounded `&mut`. /// /// Every call site of `new()` immediately did `unsafe { &mut *p }` to set diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 1589e4820a85..8d351c221a15 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -1601,7 +1601,7 @@ impl FetchTasklet { // SAFETY: the stream (held Strong above) owns a live ByteStream embedded in // its Source; JS thread. unsafe { - let source = (*bytes).parent(); + let source = crate::webcore::readable_stream::NewSource::from_context_ptr(bytes); (*source).increment_count(); this.response_stream_source = NonNull::new(source); } diff --git a/src/sql_jsc/mysql/MySQLConnection.rs b/src/sql_jsc/mysql/MySQLConnection.rs index b1e82173e9df..12c656587966 100644 --- a/src/sql_jsc/mysql/MySQLConnection.rs +++ b/src/sql_jsc/mysql/MySQLConnection.rs @@ -131,7 +131,7 @@ impl Default for MySQLConnection { // SAFETY: `MySQLConnection` is the `connection` field embedded inside // `JSMySQLConnection`; never constructed standalone. -bun_core::impl_field_parent! { MySQLConnection => JSMySQLConnection.connection; fn js_connection_ref; fn get_js_connection; } +bun_core::impl_field_parent! { MySQLConnection => JSMySQLConnection.connection; fn js_connection_ref; fn mut get_js_connection; } impl MySQLConnection { pub(crate) fn init( @@ -169,13 +169,16 @@ impl MySQLConnection { } pub(crate) fn can_pipeline(&mut self) -> bool { - self.queue.can_pipeline(self.js_connection_ref()) + let js_connection = self.js_connection_ref(); + js_connection.connection.get().queue.can_pipeline(js_connection) } pub(crate) fn can_prepare_query(&mut self) -> bool { - self.queue.can_prepare_query(self.js_connection_ref()) + let js_connection = self.js_connection_ref(); + js_connection.connection.get().queue.can_prepare_query(js_connection) } pub(crate) fn can_execute_query(&mut self) -> bool { - self.queue.can_execute_query(self.js_connection_ref()) + let js_connection = self.js_connection_ref(); + js_connection.connection.get().queue.can_execute_query(js_connection) } #[inline] From a8b11247d0de6189971d8d3dc30c48582304717d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:57:31 +0000 Subject: [PATCH 3/3] [autofix.ci] apply automated fixes --- src/bun_core/lib.rs | 11 ++++++++--- src/ptr/lib.rs | 11 +++++++++-- src/sql_jsc/mysql/MySQLConnection.rs | 18 +++++++++++++++--- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/bun_core/lib.rs b/src/bun_core/lib.rs index 78466c987380..4ea12a56ac4c 100644 --- a/src/bun_core/lib.rs +++ b/src/bun_core/lib.rs @@ -853,10 +853,15 @@ macro_rules! assert_not_freeze { ::core::assert!( !<$crate::__IsFreeze<$Child>>::IS_FREEZE, concat!( - "`", stringify!($Child), "` is Freeze, so `&self` is passed `noalias readonly` ", - "and writes to `", stringify!($Parent), "` through a pointer derived from it ", + "`", + stringify!($Child), + "` is Freeze, so `&self` is passed `noalias readonly` ", + "and writes to `", + stringify!($Parent), + "` through a pointer derived from it ", "would be miscompiled. Put the methods that need the parent on `&", - stringify!($Parent), "` instead, or derive the pointer from `&mut self`." + stringify!($Parent), + "` instead, or derive the pointer from `&mut self`." ), ); }; diff --git a/src/ptr/lib.rs b/src/ptr/lib.rs index 3740609d9a42..3b9c53388d60 100644 --- a/src/ptr/lib.rs +++ b/src/ptr/lib.rs @@ -821,7 +821,11 @@ mod container_of_tests { /// Parent method that reaches back into the child it was called from. fn bump_shared_child(&self) { // SAFETY: single-threaded test; no `&mut ByShared` is live. - unsafe { (*self.by_shared.get()).hits.set((*self.by_shared.get()).hits.get() + 1) }; + unsafe { + (*self.by_shared.get()) + .hits + .set((*self.by_shared.get()).hits.get() + 1) + }; self.count.set(self.count.get() + 1); } } @@ -866,7 +870,10 @@ mod container_of_tests { fn mut_receiver_under_parent_borrow() { fn drive(parent: &mut Parent) { parent.by_mut.touch(); - assert_eq!((parent.count.get(), parent.plain, parent.by_mut.hits), (10, 20, 2)); + assert_eq!( + (parent.count.get(), parent.plain, parent.by_mut.hits), + (10, 20, 2) + ); } let p = Parent::boxed(); // SAFETY: `p` is live and uniquely owned here. diff --git a/src/sql_jsc/mysql/MySQLConnection.rs b/src/sql_jsc/mysql/MySQLConnection.rs index 12c656587966..423bd998bb80 100644 --- a/src/sql_jsc/mysql/MySQLConnection.rs +++ b/src/sql_jsc/mysql/MySQLConnection.rs @@ -170,15 +170,27 @@ impl MySQLConnection { pub(crate) fn can_pipeline(&mut self) -> bool { let js_connection = self.js_connection_ref(); - js_connection.connection.get().queue.can_pipeline(js_connection) + js_connection + .connection + .get() + .queue + .can_pipeline(js_connection) } pub(crate) fn can_prepare_query(&mut self) -> bool { let js_connection = self.js_connection_ref(); - js_connection.connection.get().queue.can_prepare_query(js_connection) + js_connection + .connection + .get() + .queue + .can_prepare_query(js_connection) } pub(crate) fn can_execute_query(&mut self) -> bool { let js_connection = self.js_connection_ref(); - js_connection.connection.get().queue.can_execute_query(js_connection) + js_connection + .connection + .get() + .queue + .can_execute_query(js_connection) } #[inline]