bun_ptr: add OwnedRef, an intrusive ref that is released on drop

[robobun]

Problem

• bun_ptr has no handle that can sit in a field or container and release its ref on drop. RefPtr<T> owns a ref but has no Drop, so every holder calls deref() by hand on every path; ScopedRef<T> releases on drop but cannot be dereferenced or stored.
• Code that parks a ref in a queue, a task, or an in-flight socket or timer therefore stores a bare NonNull<T> or *mut T and must release it at every site that removes it. Review on http_jsc: park the WebSocket upgrade client refs as OwnedRef fields (and Cell handshake state) #37594 and mysql: hold the request queue refs as OwnedRef<JSMySQLQuery> #37618 asked for an RAII type instead.
• RefPtr's debug-build holder tracker took &mut self on acquire and release, so a RefPtr used on one thread while another handle to the same object was released on a second thread would have formed a &mut over a count the other thread was reading.
• ptr: add OwnedRef<T>, RefPtr::to_owned, and AnyRefCounted::ref_guard #31173 proposed this type in May and never landed; this is the same type on current main, plus what the stacked consumers need.

Fix

• Adds OwnedRef<T>: one non-null pointer that takes a ref on construction (new allocates, acquire takes a ref, from_raw adopts one), derefs to &T, takes another ref on clone, and releases on drop. into_raw is the only way to give the ref up without releasing it. RefPtr::into_owned and ThisPtr::owned_ref convert existing handles.
• Property to check: every constructor pairs one taken or adopted ref with the one rc_deref in Drop, and into_raw is the only escape, so a holder cannot forget or double release without naming it. Those are the same calls the hand-written sites make, so there is no runtime cost. It gives up RefPtr's debug holder tracking, as ScopedRef already does.
• Send and Sync follow Arc's bounds (T: Send + Sync). Only a host with the atomic count can meet them, since RefCount and CellRefCounted are !Sync. So that a RefPtr on another thread is safe too, the debug tracker's methods now take &self and its tables sit behind the mutex it already had; debug builds only.
• Verification: nine new unit tests, two of them cross-thread, and cargo miri test -p bun_ptr passes all 27. No call sites outside bun_ptr change; cargo check -p bun_runtime confirms the derive output still compiles, and mysql: hold the request queue refs as OwnedRef<JSMySQLQuery> #37618, rebuilt on this branch, exercises it through the MySQL suites.

Background

• Intrusive refcount: the count is a field inside the object (RefCount, CellRefCounted, or the atomic ThreadSafeRefCount, added by derive macros). rc_ref / rc_deref bump it through a raw pointer, and the release that reaches zero runs the destructor and frees the allocation. AnyRefCounted is the derive-emitted trait that lets one generic handle work over all three counts.
• ScopedRef<T> takes a ref for one scope and releases it on drop; it keeps an object alive across a re-entrant call and cannot be dereferenced or moved. RefPtr<T> is the storable handle, released by an explicit deref(); its docs say dropping one leaks.
• ThisPtr<T> wraps the this pointer a callback is dispatched with and asserts it points at a live object. ref_guard() turns it into a ScopedRef; the new owned_ref() is the storable version.
• Debug holder tracking: in debug builds RefPtr records an id per holder in tables inside the count's DebugData, so a double release can be diagnosed. It is compiled out of release builds, which is why the locking change costs nothing there.
• Miri with Tree Borrows is how this crate's tests run. It rejects a destroying release made through a pointer derived from a shared borrow, which is why the constructors take *mut T rather than &T (the contract ScopedRef::new already has), and it checks the cross-thread tests for data races.

Original description

What

bun_ptr has two handles over an intrusive refcount, and neither is the one a struct field or a container element wants:

• RefPtr<T> owns a ref but has no Drop impl. Its own docs say that dropping one leaks, so every holder releases by hand on every path.
• ScopedRef<T> releases on drop, but cannot be dereferenced or stored; it brackets one re-entrant call.

So code that parks a ref somewhere (a request queue, a task, an in-flight socket or timer) stores a NonNull<T> or *mut T, remembers that it stands for a ref, and calls deref() / deref_nn() at every site that removes it. Review on #37594 and #37618 asked for an RAII type instead. #31173 proposed exactly this type in May and never landed; this PR is that type again, with the same name and constructor vocabulary (Jarred is credited as co-author), on current main and with the three things the reworks stacked on it need: Send / Sync for thread-safe hosts, as_non_null / this_ptr for the call sites that key on the pointer, and RefPtr::into_owned for migrating existing handles. If you would rather land #31173 itself I can restack the consumers on it instead; the consumers only use the shared surface.

pub struct OwnedRef<T: AnyRefCounted>(NonNull<T>);

impl OwnedRef<T> {
    pub fn new(value: T) -> Self;                 // allocates; owns the initial ref
    pub unsafe fn acquire(ptr: *mut T) -> Self;   // takes a ref (same contract as ScopedRef::new)
    pub unsafe fn from_raw(ptr: *mut T) -> Self;  // takes over a ref the caller owns
    pub fn into_raw(self) -> *mut T;              // hands the ref off; inverse of from_raw
    pub fn as_ptr(&self) -> *mut T;
    pub fn as_non_null(&self) -> NonNull<T>;
    pub fn this_ptr(&self) -> ThisPtr<T>;
}
impl Deref for OwnedRef<T> { type Target = T; }   // live by construction: it holds a ref
impl Clone for OwnedRef<T>                        // takes another ref
impl Drop for OwnedRef<T>                         // releases it
impl RefPtr<T> { pub fn into_owned(self) -> OwnedRef<T>; }
impl ThisPtr<T> { pub fn owned_ref(self) -> OwnedRef<T>; }
unsafe impl Send / Sync for OwnedRef<T> where T: Send + Sync

It is ScopedRef as a value: the same rc_ref on construction and rc_deref on drop, plus Deref, Clone and the hand-offs, so it works for all three count flavours (RefCounted, CellRefCounted, ThreadSafeRefCounted) through the AnyRefCounted bridge the derives already emit. It is one pointer with a null niche (Option<OwnedRef<T>> is pointer sized; there is a test for that). ThisPtr::owned_ref() sits next to ref_guard() as the safe way for a callback to take one. There is no conversion from OwnedRef back to RefPtr; nothing needs one.

Send / Sync follow Arc's bounds. That is what lets a work-pool task carry the ref it holds on a ThreadSafeRefCounted host and release it on the other thread, which is the shape the stat watcher (#37591) needs. The bounds are sound because the only count that can live inside a Sync host is the atomic one: RefCount and the Cell<u32> behind CellRefCounted are !Sync, so hosts embedding them never satisfy the bounds; and ThreadSafeRefCount's ref_ / deref are already the operations such hosts use from several threads today. The one thing that was not ready for this was RefPtr's debug holder tracking: its acquire / release took &mut self (and debug_data_ptr took &mut of the whole count), so a RefPtr driven on one thread while an OwnedRef to the same object was released on another would have formed a &mut over a count the other thread was reading, which review pointed out. Those methods now take &self and the two tables live behind the lock the struct already had (DebugData is debug-build only, so this costs nothing in release); noop_debug_data becomes a plain static. Two tests cover the threading: one clones an OwnedRef on a thread-safe host into four threads, drops the clones there and destroys it from a fifth, the other keeps a RefPtr recording and releasing holder entries on the main thread while four threads release OwnedRef clones of the same object. The whole test module runs under Miri, which also checks these for data races.

The constructors take *mut T rather than &T on purpose. A first version took &T; Miri (Tree Borrows, which is how this crate's tests run) rejected it, because a ref taken through a pointer derived from a shared borrow may be the one whose release destroys the object, and the destroying release writes to the count's debug data and frees the allocation. ScopedRef::new and RefPtr::init_ref have the same *mut T contract for the same reason, so the per-type ref_guard(&self) style helpers that already exist for ScopedRef are where the one audited cast belongs (the first consumer, #37618, adds JSMySQLQuery::owned_ref(&self) next to its ref_guard). new(value) needs no unsafe at all and replaces the Box plus initial-count hand-off that types like the proxy tunnel do today.

What it gives up relative to RefPtr (and to the #31173 version) is the debug-build holder tracking, which ScopedRef does not have either; in the current port that tracker only records ids, and it is not written for concurrent use, so being able to send the ref is worth more than keeping it. RefPtr itself gains into_owned, the &self tracker described above and a doc pointer here.

No call sites outside bun_ptr change (the derives' generated code calls debug_data_ptr through a reference, which now auto-borrows shared; bun_runtime, which has thread-safe hosts, was checked). The PR adds the type, its docs and nine unit tests (take/release, last one out destroys, new / into_raw / from_raw round trip, Cell<Option<_>>::take releases exactly once, RefPtr::into_owned leaves the count alone, ThisPtr::owned_ref outlives the callback, the two cross-thread tests, the niche).

Why

With the ref represented as a value, "this field holds a ref" is stated by its type, releasing it is = None, take(), or popping it out of a container, and a path that forgets to release, or releases twice, does not compile unless it names into_raw. The follow-ups that use it are the reworks requested in review: #37618 (the MySQL request queue becomes a fifo of OwnedRef<JSMySQLQuery>, removing the five deref_nn sites), #37591 (the stat watcher's tasks carry the ref instead of the callbacks adopting it) and #37594 (the WebSocket upgrade client's socket, C++ owner and tunnel refs become Option<OwnedRef<Self>> fields); they are stacked on this branch. There is no runtime cost relative to the hand-written pairs: construction and Drop are the same rc_ref / rc_deref calls the hand-written sites make, inlined.

Part of a series of small type-system hardening changes.

Verification

cargo check, cargo clippy and cargo fmt --check are clean for bun_ptr. cargo miri test -p bun_ptr (Tree Borrows, as bun run rust:miri runs it): 27 passed, including the 9 new tests. cargo check -p bun_runtime confirms the derive-generated code is unaffected by the &self change; #37618, rebuilt on top of this branch, exercises it through the MySQL suites.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a2715232-73e4-4fe1-859e-f77aa3845e3a

📥 Commits

Reviewing files that changed from the base of the PR and between 8d10147 and bca06d7.

📒 Files selected for processing (2)

• src/ptr/lib.rs
• src/ptr/ref_count.rs

---

Walkthrough

Changes

The PR adds OwnedRef<T> as an owning RAII intrusive-reference handle. It integrates OwnedRef with ThisPtr and RefPtr, updates debug tracking for shared concurrent access, and adds ownership and cross-thread tests.

Owned reference lifecycle

Layer / File(s)	Summary
OwnedRef API and pointer integration src/ptr/ref_count.rs, src/ptr/lib.rs	OwnedRef now supports ownership, cloning, raw-pointer transfer, dereferencing, and automatic release. RefPtr::into_owned, ThisPtr::owned_ref, and the public re-export are added.
Concurrent debug tracking src/ptr/ref_count.rs	Debug tracking uses shared access and mutex-protected live and released-holder tables.
Ownership and thread-safety validation src/ptr/ref_count.rs	Tests cover transfers, drops, Option, lifetime extension, null-pointer layout, and cross-thread ownership.

Suggested reviewers: alii, jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the primary change: adding an intrusive OwnedRef that releases its reference on drop.
Description check	✅ Passed	The description explains the problem, implementation, design decisions, testing, and verification, including the requested behavior and limitations.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 12:05 AM PT - Aug 12th, 2026}

@robobun, your commit bca06d7 is building: #92785

[claude]

I re-reviewed this and found no bugs. My earlier note about the unsafe impl Sync SAFETY comment doesn't apply to this revision — OwnedRef has no Send/Sync impl here (it stays !Send/!Sync via the inner RefPtr's NonNull), and the doc comment states that. Given this adds a new public unsafe-backed primitive to the foundational bun_ptr crate that follow-up PRs will build on, a human sign-off on the API shape is still worthwhile.

What was reviewed:

• The two ManuallyDrop::take exits (Drop and into_ref_ptr) are mutually exclusive — into_ref_ptr wraps self in ManuallyDrop first, so no double-take.
• new/adopt/Clone/Deref forward to existing RefPtr methods with the same safety contracts as ScopedRef; debug tracking is preserved.
• Option<OwnedRef<T>> niche and the Cell<Option<_>>::take release-once pattern are covered by the Miri tests.

Extended reasoning...

Overview

Adds OwnedRef<T> to src/ptr/ref_count.rs (a ManuallyDrop<RefPtr<T>> newtype with a Drop impl that calls RefPtr::deref()), re-exports it from src/ptr/lib.rs, adds ThisPtr::owned_ref() alongside ref_guard(), updates a few doc comments to point at the new type, and adds seven Miri-run unit tests plus a count() test helper. No existing callers change; the type is not yet used anywhere in the tree.

Change since my prior comment

My earlier inline finding referenced an unsafe impl Sync for OwnedRef<T> and its SAFETY comment at lines ~1193-1195. That impl is not present in this revision (commit 16d2d0f titled "OwnedRef stays !Send/!Sync like RefPtr"), and the type's doc comment now states it is !Send/!Sync for the same reason RefPtr is. So that concern is resolved — there is no thread-safety obligation on the debug tracker introduced here.

Security risks

None user-facing. The risk surface is memory-safety soundness of a new unsafe-backed smart pointer. The two unsafe blocks in the type body (ManuallyDrop::take in Drop and in into_ref_ptr) are exclusive by construction; the unsafe fn constructors forward the exact caller contracts of RefPtr::init_ref/from_raw and ScopedRef::new/adopt. Tests pass under Miri (Tree Borrows) per the PR description.

Level of scrutiny

High. bun_ptr is a T0 foundation crate; this is a new public API that three named follow-up PRs will adopt for request queues, timers, and socket self-refs. The implementation is small and mechanically forwards to RefPtr, but the API shape (new vs adopt, into_raw/into_ref_ptr, whether Deref is appropriate for a shared-ownership handle, staying !Send) is a design decision a maintainer should ratify before dependents stack on it.

Other factors

The comment-cop bot flagged ~13 lines; those are doc comments and required SAFETY comments on a public unsafe API, not workaround justifications, so I did not treat them as blocking findings — but they are outstanding automated comments the author may want to address or dismiss. The bug-hunting pass this run found nothing.

[robobun]

Revised in d817546, in case anyone read the first version: OwnedRef is now a plain NonNull<T> that calls rc_ref / rc_deref itself (the same pair ScopedRef uses) instead of wrapping a RefPtr, and it is Send / Sync under Arc's bounds. The reason is the stat watcher rework (#37591): its refs are taken on the JS thread and released on the work pool, and RefPtr's debug holder tracking is not written for that, so the first version could not have been sent across threads. The trade is losing that debug tracking for OwnedRef holders (ScopedRef never had it either). Description updated; a cross-thread test was added and the module still passes under Miri (26 tests).

[claude]

The latest push implements the fix from the third review round: debug_data_ptr and DebugDataOps::acquire/release now take &self, the holder tables live behind bun_core::Mutex<Holders>, and noop_debug_data is a plain static — so no path from a RefPtr on one thread forms a &mut to the count while an OwnedRef on another thread reads it. The new ref_ptr_tracking_coexists_with_owned_refs_on_other_threads test exercises exactly that shape, and the Send SAFETY comment now covers it. That closes the concern at line 866; I found nothing further this round.

Given this adds a new public handle type to a T0 foundation crate with hand-written unsafe impl Send + Sync and reshapes DebugData's locking, a maintainer sign-off would still be worthwhile before it lands.

Also checked: alloc_id moving outside the lock is fine (AtomicU32::fetch_add); deinit keeping &mut self is fine (last-release only, uses get_mut()); the derive at bun_core_macros/lib.rs:479 now auto-refs to &ThreadSafeRefCount instead of &mut.

Extended reasoning...

Overview

Adds OwnedRef<T> to bun_ptr — an RAII intrusive-ref handle that releases on Drop, with Deref/Clone, Send + Sync under Arc's bounds, plus RefPtr::into_owned and ThisPtr::owned_ref bridges. To make the Send/Sync impls sound in debug builds, this revision also refactors DebugData: debug_data_ptr and DebugDataOps::acquire/release now take &self, and the live/frees maps move into a Holders struct behind bun_core::Mutex. Eight new unit tests, two of them cross-thread; all run under Miri.

Security risks

None user-facing. This is a memory-safety primitive; the risk class is soundness of the unsafe impl Send + Sync. Three prior review rounds identified aliasing routes through the debug tracker's &mut self methods; the current revision closes them at the layer that owned the invariant (the tracker itself) rather than by removing bridges one at a time, which was the fix recommended in each round. I re-traced the previous scenario against the new code: both the RefPtr side (rc_debug_data → debug_data_ptr(&self) → acquire/release(&self) → holders.lock()) and the OwnedRef side (rc_ref/rc_deref → &*get_ref_count(...)) now form only shared borrows of ThreadSafeRefCount/DebugData, with the mutex serialising the map writes. bun_core::Mutex wraps std::sync::Mutex, so it is a real lock. deinit still takes &mut self but only runs after the atomic 1→0 transition, which is pre-existing behaviour.

Level of scrutiny

High. This is new public API in the T0 bun_ptr foundation crate, with hand-written unsafe impl Send + Sync, intended as the base for stacked follow-ups (#37591, #37594, #37618). REVIEW.md asks for maintainer agreement on new cross-cutting abstractions; while #31173 and prior review requests establish that the type is wanted, the Send/Sync surface and the DebugData locking refactor warrant a human look.

Other factors

The comment-cop bot's five unresolved flags on this revision are the same false positives as prior rounds (rustdoc on public API, a doc comment on DebugDataOps::acquire, and a note in the test module). No runtime code uses OwnedRef yet, so behavioural risk is limited to the DebugData refactor, which is behaviour-preserving (same insert/remove semantics; alloc_id was already atomic and just moved outside the lock).