jsc: back JsRef::Weak with a real JSC::Weak handle

src/jsc/JSRef.rs

@@ -3,6 +3,7 @@
 // The methods used below (`get() -> Option`, `has()`, `try_swap()`) live on
 // the Optional wrapper, so import it under the local name `Strong`.
 use crate::strong::Optional as Strong;
+use crate::weak::Weak;
 use crate::{JSGlobalObject, JSValue};
 
 /// Holds a reference to a JSValue with lifecycle management.
@@ -61,9 +62,11 @@
 ///
 /// # States
 ///
-/// - **Weak**: Holds a JSValue directly. Does NOT prevent garbage collection.
-///   The JSValue may become invalid if the object is collected.
-///   Use `try_get()` to safely check if the value is still alive.
+/// - **Weak**: Holds a passive `JSC::Weak` handle registered against the
+///   value's cell. Does NOT prevent garbage collection. `try_get()` returns
+///   `None` from the moment GC reaps the referent — before the lazy sweep
+///   runs the wrapper's destructor (which is what flips the ref to
+///   `Finalized`) — so a deferred reader can never observe a dead cell.
 ///
 /// - **Strong**: Holds a Strong reference that prevents garbage collection.
 ///   The JavaScript object will stay alive as long as this reference exists.
@@ -95,24 +98,25 @@
 /// Common pattern: Start strong, downgrade to weak when idle, upgrade to strong when active.
 /// See ServerWebSocket, UDPSocket, MySQLConnection, and ValkeyClient for examples.
 ///
-/// `JsRef` is `!Send + !Sync` (transitively via `JSValue` and `Strong`): the
-/// `StrongRootBlock` slot backing `Strong` hangs off the per-VM JSVMClientData
-/// and must be dropped on the JS thread.
+/// `JsRef` is `!Send + !Sync` (transitively via `Weak` and `Strong`): the
+/// `StrongRootBlock` slot backing `Strong` hangs off the per-VM JSVMClientData,
+/// and the `JSC::Weak` handle backing `Weak` lives in the cell's `WeakSet`;
+/// both must be created and dropped on the JS thread.
 pub enum JsRef {
-    Weak(JSValue),
+    Weak(Weak<()>),
     Strong(Strong),
     Finalized,
 }
 
-// Belt-and-suspenders: JSValue and Strong are already !Send/!Sync, but make it
+// Belt-and-suspenders: Weak and Strong are already !Send/!Sync, but make it
 // explicit so a future refactor of those types cannot accidentally make JsRef
 // sendable.
 const _: PhantomData<*const ()> = PhantomData;
 
 impl JsRef {
     pub fn init_weak(value: JSValue) -> Self {
         debug_assert!(!value.is_empty_or_undefined_or_null());
-        JsRef::Weak(value)
+        JsRef::Weak(Weak::create_passive(value))
     }
 
     pub fn init_strong(value: JSValue, global: &JSGlobalObject) -> Self {
@@ -121,18 +125,12 @@
     }
 
     pub fn empty() -> Self {
-        JsRef::Weak(JSValue::UNDEFINED)
+        JsRef::Weak(Weak::default())
     }
 
     pub fn try_get(&self) -> Option<JSValue> {
         match self {
-            JsRef::Weak(weak) => {
-                if weak.is_empty_or_undefined_or_null() {
-                    None
-                } else {
-                    Some(*weak)
-                }
-            }
+            JsRef::Weak(weak) => weak.get(),
             JsRef::Strong(strong) => strong.get(),
             JsRef::Finalized => None,
         }
@@ -148,17 +146,12 @@
 
     pub fn set_weak(&mut self, value: JSValue) {
         debug_assert!(!value.is_empty_or_undefined_or_null());
-        match self {
-            JsRef::Weak(_) => {}
-            JsRef::Strong(_) => {
-                // `Strong`'s `Drop` releases the block slot when `*self` is
-                // overwritten below, so no explicit deinit is needed.
-            }
-            JsRef::Finalized => {
-                return;
-            }
+        if matches!(self, JsRef::Finalized) {
+            return;
         }
-        *self = JsRef::Weak(value);
+        // Overwriting `*self` drops the prior variant (a `Strong`'s `Drop`
+        // releases its block slot; a `Weak`'s `Drop` frees its handle).
+        *self = JsRef::Weak(Weak::create_passive(value));
     }
 
     pub fn set_strong(&mut self, value: JSValue, global: &JSGlobalObject) {
@@ -173,9 +166,13 @@
     pub fn upgrade(&mut self, global: &JSGlobalObject) {
         match self {
             JsRef::Weak(weak) => {
-                debug_assert!(!weak.is_empty_or_undefined_or_null());
-                let weak = *weak;
-                *self = JsRef::Strong(Strong::create(weak, global));
+                // A reaped referent cannot be resurrected: stay `Weak` (dead)
+                // so readers keep seeing `None`. This also makes an upgrade
+                // that races wrapper collection a skip instead of a re-root
+                // of a dead cell.
+                if let Some(value) = weak.get() {
+                    *self = JsRef::Strong(Strong::create(value, global));
+                }
             }
             JsRef::Strong(_) => {}
             JsRef::Finalized => {
@@ -188,28 +185,30 @@
         match self {
             JsRef::Weak(_) => {}
             JsRef::Strong(strong) => {
-                let value = strong.try_swap().unwrap_or(JSValue::UNDEFINED);
-                value.ensure_still_alive();
-                // The old `Strong` is dropped by the assignment below; the
-                // `strong` borrow ends at its last use above, permitting
-                // reassignment of `*self`.
-                *self = JsRef::Weak(value);
+                // Register the weak handle while the `Strong` still roots the
+                // referent, so there is no window in which the value is
+                // unrooted; the old `Strong` is dropped by the assignment.
+                let weak = match strong.get() {
+                    Some(value) => Weak::create_passive(value),
+                    None => Weak::default(),
+                };
+                *self = JsRef::Weak(weak);
             }
             JsRef::Finalized => {}
         }
     }
 
     pub fn is_empty(&self) -> bool {
         match self {
-            JsRef::Weak(weak) => weak.is_empty_or_undefined_or_null(),
+            JsRef::Weak(weak) => weak.get().is_none(),
             JsRef::Strong(strong) => !strong.has(),
             JsRef::Finalized => true,
         }
     }
 
     pub fn is_not_empty(&self) -> bool {
         match self {
-            JsRef::Weak(weak) => !weak.is_empty_or_undefined_or_null(),
+            JsRef::Weak(weak) => weak.get().is_some(),
             JsRef::Strong(strong) => strong.has(),
             JsRef::Finalized => false,
         }
@@ -229,9 +228,9 @@
 
     pub fn update(&mut self, global: &JSGlobalObject, value: JSValue) {
         match self {
-            JsRef::Weak(weak) => {
+            JsRef::Weak(_) => {
                 debug_assert!(!value.is_empty_or_undefined_or_null());
-                *weak = value;
+                *self = JsRef::Weak(Weak::create_passive(value));
             }
             JsRef::Strong(strong) => {
                 if strong.get() != Some(value) {

src/jsc/Weak.rs

@@ -2,7 +2,7 @@ use core::ffi::c_void;
 use core::marker::PhantomData;
 use core::ptr::NonNull;
 
-use crate::{JSGlobalObject, JSValue};
+use crate::JSValue;
 
 #[repr(u32)]
 #[derive(Copy, Clone, Eq, PartialEq)]
@@ -19,13 +19,11 @@ bun_opaque::opaque_ffi! {
 
 impl WeakImpl {
     fn init(
-        global_this: &JSGlobalObject,
         value: JSValue,
         ref_type: WeakRefType,
         ctx: Option<NonNull<c_void>>,
     ) -> NonNull<WeakImpl> {
         NonNull::new(Bun__WeakRef__new(
-            global_this,
             value,
             ref_type,
             ctx.map_or(core::ptr::null_mut(), |p| p.as_ptr()),
@@ -60,14 +58,15 @@ impl WeakImpl {
 
 // `WeakImpl` is an opaque `UnsafeCell`-backed ZST handle (`&WeakImpl` is
 // ABI-identical to non-null `*const WeakImpl`; C++ slot mutation is interior).
-// `new` is `safe fn`: `&JSGlobalObject` is the non-null handle proof, and `ctx`
-// is an opaque round-trip pointer C++ only stores and forwards to the finalizer
+// `new` is `safe fn`: the weak handle registers against `value`'s own cell
+// (callers pass a live object value — enforced by the `is_object()` guard in
+// the constructors below, which implies a readable cell header), and `ctx` is
+// an opaque round-trip pointer C++ only stores and forwards to the finalizer
 // (never dereferenced as Rust data) — same contract as `JSC__VM__holdAPILock`.
 // `delete` consumes the allocation and so stays `unsafe fn`.
 unsafe extern "C" {
     fn Bun__WeakRef__delete(this: *mut WeakImpl);
     safe fn Bun__WeakRef__new(
-        global: &JSGlobalObject,
         value: JSValue,
         ref_type: WeakRefType,
         ctx: *mut c_void,
@@ -91,28 +90,25 @@ impl<T> Default for Weak<T> {
 }
 
 impl<T> Weak<T> {
-    /// A weak handle with no finalize callback. `get()` reads `None` from the
-    /// moment GC reaps the referent, before any sweep runs cell destructors.
-    pub fn create_passive(value: JSValue, global_this: &JSGlobalObject) -> Self {
-        if value.is_empty() {
+    /// A weak handle with no finalize callback, registered against `value`'s
+    /// own cell (no global needed). `get()` reads `None` from the moment GC
+    /// reaps the referent, before any sweep runs cell destructors.
+    ///
+    /// Non-object values produce an empty handle (`get()` is `None`).
+    pub fn create_passive(value: JSValue) -> Self {
+        if !value.is_object() {
             return Self::default();
         }
         Self {
-            r#ref: Some(WeakImpl::init(global_this, value, WeakRefType::None, None)),
+            r#ref: Some(WeakImpl::init(value, WeakRefType::None, None)),
             _ctx: PhantomData,
         }
     }
 
-    pub fn create(
-        value: JSValue,
-        global_this: &JSGlobalObject,
-        ref_type: WeakRefType,
-        ctx: &mut T,
-    ) -> Self {
-        if !value.is_empty() {
+    pub fn create(value: JSValue, ref_type: WeakRefType, ctx: &mut T) -> Self {
+        if value.is_object() {
             return Self {
                 r#ref: Some(WeakImpl::init(
-                    global_this,
                     value,
                     ref_type,
                     Some(NonNull::from(ctx).cast::<c_void>()),

src/jsc/bindings/Weak.cpp

@@ -77,10 +77,13 @@ class WeakRef {
     WTF_MAKE_TZONE_ALLOCATED(WeakRef);
 
 public:
-    WeakRef(JSC::VM& vm, JSC::JSValue value, WeakRefType kind, void* ctx = nullptr)
+    // The weak handle is registered against the cell's own WeakSet, so no
+    // VM/globalObject is needed. Non-object values produce an empty handle.
+    WeakRef(JSC::JSValue value, WeakRefType kind, void* ctx = nullptr)
     {
-
         JSC::JSObject* object = value.getObject();
+        if (!object) [[unlikely]]
+            return;
         if (object->type() == JSC::JSType::GlobalProxyType)
             object = uncheckedDowncast<JSC::JSGlobalProxy>(object)->target();
 
@@ -107,9 +110,9 @@ extern "C" void Bun__WeakRef__delete(Bun::WeakRef* weakRef)
     delete weakRef;
 }
 
-extern "C" Bun::WeakRef* Bun__WeakRef__new(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue, Bun::WeakRefType kind, void* ctx)
+extern "C" Bun::WeakRef* Bun__WeakRef__new(JSC::EncodedJSValue encodedValue, Bun::WeakRefType kind, void* ctx)
 {
-    return new Bun::WeakRef(globalObject->vm(), JSC::JSValue::decode(encodedValue), kind, ctx);
+    return new Bun::WeakRef(JSC::JSValue::decode(encodedValue), kind, ctx);
 }
 
 extern "C" JSC::EncodedJSValue Bun__WeakRef__get(Bun::WeakRef* weakRef)

src/runtime/server/mod.rs

@@ -549,11 +549,13 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
         self.js_value.try_get().expect("js_value alive")
     }
 
-    /// Returns the wrapper while it is alive (`Strong` or `Weak`), or `None`
-    /// once `finalize()` has set `Finalized`. `Weak` means the wrapper cell is
-    /// still live (its WriteBarrier slots still root the handlers); only
-    /// `Finalized` means the slots are gone and the `config` shadows may point
-    /// at freed cells. Dispatch trampolines answer 503+close on `None`.
+    /// Returns the wrapper while it is alive (`Strong`, or `Weak` with the
+    /// cell still reachable), or `None` once GC has reaped the cell or
+    /// `finalize()` has set `Finalized`. While the wrapper is alive its
+    /// WriteBarrier slots still root the handlers; once it is reaped the
+    /// `config` shadows may point at dead cells, and the `Weak` read goes
+    /// `None` before the sweep runs `finalize()`, so no dispatch can observe
+    /// that window. Dispatch trampolines answer 503+close on `None`.
     pub(crate) fn js_value_for_dispatch(&self) -> Option<JSValue> {
         self.js_value.try_get()
     }
@@ -1750,10 +1752,10 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
             && !self.has_listener()
             && !self.has_active_web_sockets()
         {
-            // Make the wrapper collectible. Dispatch still works while it is
-            // `Weak` (its WriteBarrier slots still root the handlers); the
-            // `js_value_for_dispatch` gate only trips once the wrapper is
-            // actually finalized.
+            // Make the wrapper collectible. Dispatch still works while the
+            // wrapper is alive (its WriteBarrier slots still root the
+            // handlers); the `js_value_for_dispatch` gate trips as soon as GC
+            // reaps the wrapper, before its finalizer runs.
             self.js_value.downgrade();
             if let Some(ws) = self.config.websocket.as_mut() {
                 ws.handler.app = None;

src/runtime/webcore/Body.rs

@@ -1717,7 +1717,7 @@ pub(crate) trait BodyMixin: BodyOwnerJs + Sized {
                 if let Some(stream) = locked.readable.get(global_object) {
                     stream.value.ensure_still_alive();
                     Self::stream_set_cached(js_value, global_object, stream.value);
-                    locked.readable.downgrade(global_object);
+                    locked.readable.downgrade();
                 }
             }
         }

src/runtime/webcore/ReadableStream.rs

@@ -77,9 +77,9 @@ impl Strong {
     /// Release the GC root while keeping the stream readable via `Weak`. The
     /// owning wrapper's `m_stream` `WriteBarrier` keeps the stream alive from
     /// here.
-    pub(crate) fn downgrade(&mut self, global: &JSGlobalObject) {
+    pub(crate) fn downgrade(&mut self) {
         if let Self::Held(s) = self {
-            *self = Self::Weak(bun_jsc::Weak::create_passive(s.get(), global));
+            *self = Self::Weak(bun_jsc::Weak::create_passive(s.get()));
         }
     }
 
@@ -103,7 +103,7 @@ impl Strong {
             match self {
                 Self::Held(s) => s.set(global, first.value),
                 Self::Weak(_) | Self::Empty => {
-                    *self = Self::Weak(bun_jsc::Weak::create_passive(first.value, global))
+                    *self = Self::Weak(bun_jsc::Weak::create_passive(first.value))
                 }
             }
             return Ok(Some(second));

src/runtime/webcore/fetch/FetchTasklet.rs

@@ -1890,12 +1890,8 @@ impl FetchTasklet {
         // SAFETY: `response` is freshly allocated above; ownership transfers to JSC.
         let response_js = Response::make_maybe_pooled(&global_this, response);
         response_js.ensure_still_alive();
-        self.response = jsc::Weak::<FetchTasklet>::create(
-            response_js,
-            &global_this,
-            jsc::WeakRefType::FetchResponse,
-            self,
-        );
+        self.response =
+            jsc::Weak::<FetchTasklet>::create(response_js, jsc::WeakRefType::FetchResponse, self);
         // Response is intrusively refcounted; bump for native_response.
         // SAFETY: `response` is the live heap allocation owned by JSC after
         // `make_maybe_pooled`; `ref_` bumps the intrusive refcount.