weakref subclass payload, and the pickler's stale pinned hooks

pyre/extra_tests/parity_tests/weakref_ref_subclass_layout.py

@@ -0,0 +1,102 @@
+# CPython-suite gap: weakref subclass tests do not combine callbacks with all storage shapes.
+# parity-tests reason: this guards the translated W_Weakref payload and subclass carrier layout.
+
+import gc
+from weakref import WeakValueDictionary, ref
+
+
+class Value:
+    def __init__(self, value):
+        self.value = value
+
+    def __eq__(self, other):
+        return isinstance(other, Value) and self.value == other.value
+
+    def __hash__(self):
+        return hash(self.value)
+
+
+class PlainRef(ref):
+    pass
+
+
+class EmptyRef(ref):
+    __slots__ = ()
+
+
+class KeyedRef(ref):
+    __slots__ = ("key",)
+
+
+PRIVATE_FIELDS = ("w_obj_weak", "w_callable", "w_hash", "w_slots")
+
+
+def collect():
+    for _ in range(3):
+        gc.collect()
+
+
+def check_ref_subclass(ref_type):
+    callbacks = []
+
+    def callback(w_ref):
+        callbacks.append(w_ref() is None)
+
+    value = Value((ref_type.__name__, 42))
+    equal_value = Value((ref_type.__name__, 42))
+    w_ref = ref_type(value, callback)
+    equal_ref = ref(equal_value)
+
+    assert w_ref() is value
+    assert w_ref.__callback__ is callback
+    assert hash(w_ref) == hash(value)
+    assert w_ref == equal_ref
+
+    if ref_type is PlainRef:
+        w_ref.marker = "plain"
+        assert w_ref.__dict__ == {"marker": "plain"}
+    else:
+        assert not hasattr(w_ref, "__dict__")
+
+    if ref_type is KeyedRef:
+        w_ref.key = ("module", "name")
+        assert w_ref.key == ("module", "name")
+        w_ref.key = ("module", "renamed")
+        assert w_ref.key == ("module", "renamed")
+        assert w_ref() is value
+        del w_ref.key
+        try:
+            w_ref.key
+        except AttributeError:
+            pass
+        else:
+            raise AssertionError("deleted slot remained readable")
+
+    for name in PRIVATE_FIELDS:
+        assert not hasattr(w_ref, name)
+        if hasattr(w_ref, "__dict__"):
+            assert name not in w_ref.__dict__
+
+    del value
+    collect()
+    assert w_ref() is None
+    assert callbacks == [True]
+
+
+for ref_type in (PlainRef, EmptyRef, KeyedRef):
+    check_ref_subclass(ref_type)
+
+
+values = WeakValueDictionary()
+key = ("namespace", "spam")
+value = Value("live")
+values[key] = value
+assert values[key] is value
+stored_ref = values.data[key]
+assert stored_ref.key == key
+assert stored_ref() is value
+del value
+collect()
+assert key not in values
+
+print("OK")

pyre/pyre-interpreter/src/baseobjspace.rs

@@ -4725,6 +4725,9 @@ pub(crate) fn native_slot_get(
     if unsafe { pyre_object::is_list(obj) } {
         return Ok(unsafe { pyre_object::listobject::w_list_slot_get(obj, index as usize) });
     }
+    if unsafe { pyre_object::weakref::is_typed_weakref(obj) } {
+        return Ok(unsafe { pyre_object::weakref::w_weakref_object_slot_get(obj, index as usize) });
+    }
     let w_dict = getdict(obj)?;
     if w_dict.is_null() {
         return Ok(None);
@@ -4766,6 +4769,10 @@ pub(crate) fn native_slot_set(
         unsafe { pyre_object::listobject::w_list_slot_set(obj, index as usize, value) };
         return Ok(true);
     }
+    if unsafe { pyre_object::weakref::is_typed_weakref(obj) } {
+        unsafe { pyre_object::weakref::w_weakref_object_slot_set(obj, index as usize, value) };
+        return Ok(true);
+    }
     let w_dict = getdict(obj)?;
     if w_dict.is_null() {
         return Ok(false);
@@ -4798,6 +4805,9 @@ pub(crate) fn native_slot_del(obj: PyObjectRef, name: &str, index: u32) -> Resul
     if unsafe { pyre_object::is_list(obj) } {
         return Ok(unsafe { pyre_object::listobject::w_list_slot_del(obj, index as usize) });
     }
+    if unsafe { pyre_object::weakref::is_typed_weakref(obj) } {
+        return Ok(unsafe { pyre_object::weakref::w_weakref_object_slot_del(obj, index as usize) });
+    }
     let w_dict = getdict(obj)?;
     if w_dict.is_null() {
         return Ok(false);

pyre/pyre-interpreter/src/module/_weakref/interp__weakref.rs

@@ -78,10 +78,9 @@ fn write_attr(obj: PyObjectRef, name: &str, value: PyObjectRef) {
     }
 }
 
-/// Field bridge for the exact typed `W_Weakref` and the still-generated
-/// mapdict layout of a Python `weakref.ref` subclass.  PyPy presents the same
-/// `W_WeakrefBase` fields on both translated classes; only their physical
-/// composition differs (`typedef.py:174-227`).
+/// Field bridge for typed `W_Weakref` and mapdict `W_AbstractProxy` objects.
+/// `W_WeakrefBase` (interp__weakref.py:160-202) supplies the same fields to
+/// both; only their physical composition differs.
 #[inline]
 fn weakref_obj_weak(obj: PyObjectRef) -> PyObjectRef {
     if unsafe { pyre_object::weakref::is_typed_weakref(obj) } {
@@ -128,12 +127,12 @@ fn weakref_set_hash(obj: PyObjectRef, value: PyObjectRef) {
 }
 
 /// Scoped GC root for a freshly-allocated instance still held only in a
-/// Rust local. The weakref / proxy constructors allocate the instance,
-/// then allocate a `GcWeakrefBox` (an `rweakref` `Weakref` via
-/// `try_gc_alloc`) and grow the instance dict — each is a nursery
-/// allocation that can drive a minor collection. Without a root the
-/// not-yet-reachable instance is swept (its header zeroed), and a later
-/// `write_attr` / `typedef::r#type` dereferences the dangling pointer.
+/// Rust local. The proxy constructors allocate the instance, then allocate a
+/// `GcWeakrefBox` (an `rweakref` `Weakref` via `try_gc_alloc`) and grow the
+/// instance dict — each is a nursery allocation that can drive a minor
+/// collection. Without a root the not-yet-reachable instance is swept (its
+/// header zeroed), and a later `write_attr` / `typedef::r#type` dereferences
+/// the dangling pointer.
 /// Registering the slot keeps the instance alive and relocates the local
 /// in place when the collector promotes it, mirroring `FrameLocalsRoot`.
 struct InstanceRoot {
@@ -647,67 +646,41 @@ pub fn W_Weakref_new(
     w_obj: PyObjectRef,
     w_callable: PyObjectRef,
 ) -> PyObjectRef {
-    use pyre_object::objectobject::w_instance_new;
     // typedef.py:519 generic_new_descr → space.allocate_instance(W_Type, w_subtype)
     let actual_type = if w_subtype.is_null() {
         weakref_type()
     } else {
         w_subtype
     };
-    // typeobject.py `allocate_instance`: a subclass whose Layout adds no
-    // storage keeps the builtin W_Weakref layout and changes only `w_class`.
-    // In particular `class R(ref): __slots__ = ()` has neither mapdict nor
-    // member slots in which the three interpreter-owned fields could live.
+    // `W_Weakref` (interp__weakref.py:195-257) / `descr__new__weakref`
+    // (interp__weakref.py:259-269): the three fields are interpreter-owned, so
+    // every subtype keeps the builtin payload. Its Python-level `__dict__` and
+    // slots use the same tagged carrier as other builtin subclasses.
     let exact_type = std::ptr::eq(actual_type, weakref_type());
-    let shares_base_layout = !exact_type
-        && unsafe {
-            !pyre_object::w_type_get_hasdict(actual_type)
-                && pyre_object::w_type_get_nslots(actual_type)
-                    == pyre_object::w_type_get_nslots(weakref_type())
-        };
-    if exact_type || shares_base_layout {
-        let _roots = pyre_object::gc_roots::push_roots();
-        let root_base = pyre_object::gc_roots::shadow_stack_len();
-        pyre_object::gc_roots::pin_root(w_obj);
-        pyre_object::gc_roots::pin_root(w_callable);
-        let w_obj_weak = pyre_object::weakref::w_gc_weakref_box_new_or_strong(
-            pyre_object::gc_roots::shadow_stack_get(root_base),
-        );
-        pyre_object::gc_roots::pin_root(w_obj_weak);
-        let callable = pyre_object::gc_roots::shadow_stack_get(root_base + 1);
-        let callable = if !callable.is_null() && !unsafe { pyre_object::is_none(callable) } {
-            callable
-        } else {
-            pyre_object::PY_NULL
-        };
-        let weakref = pyre_object::weakref::w_weakref_object_new(
-            pyre_object::gc_roots::shadow_stack_get(root_base + 2),
-            callable,
-            pyre_object::PY_NULL,
-        );
-        return if exact_type {
-            weakref
-        } else {
-            crate::typedef::tag_subclass_instance(weakref, actual_type)
-        };
-    }
-
-    let mut obj = w_instance_new(actual_type);
-    let _root = InstanceRoot::new(&mut obj);
-    // W_WeakrefBase.__init__: self.w_obj_weak = weakref.ref(w_obj).
-    // Compute the box before `write_attr`: the allocation must not happen
-    // while a stale `obj` is captured as the call's receiver argument.
-    let w_obj_weak = pyre_object::weakref::w_gc_weakref_box_new_or_strong(w_obj);
-    write_attr(obj, ATTR_W_OBJ_WEAK, w_obj_weak);
-    let w_callable = if !w_callable.is_null() && !unsafe { pyre_object::is_none(w_callable) } {
-        w_callable
+    let _roots = pyre_object::gc_roots::push_roots();
+    let root_base = pyre_object::gc_roots::shadow_stack_len();
+    pyre_object::gc_roots::pin_root(w_obj);
+    pyre_object::gc_roots::pin_root(w_callable);
+    let w_obj_weak = pyre_object::weakref::w_gc_weakref_box_new_or_strong(
+        pyre_object::gc_roots::shadow_stack_get(root_base),
+    );
+    pyre_object::gc_roots::pin_root(w_obj_weak);
+    let callable = pyre_object::gc_roots::shadow_stack_get(root_base + 1);
+    let callable = if !callable.is_null() && !unsafe { pyre_object::is_none(callable) } {
+        callable
     } else {
-        pyre_object::w_none()
+        pyre_object::PY_NULL
     };
-    write_attr(obj, ATTR_W_CALLABLE, w_callable);
-    // W_Weakref.__init__: self.w_hash = None
-    write_attr(obj, ATTR_W_HASH, pyre_object::w_none());
-    obj
+    let weakref = pyre_object::weakref::w_weakref_object_new(
+        pyre_object::gc_roots::shadow_stack_get(root_base + 2),
+        callable,
+        pyre_object::PY_NULL,
+    );
+    if exact_type {
+        weakref
+    } else {
+        crate::typedef::tag_subclass_instance(weakref, actual_type)
+    }
 }
 
 #[allow(non_snake_case)]

pyre/pyre-jit/src/eval.rs

@@ -3906,8 +3906,9 @@ fn build_gc() -> Box<MiniMarkGC> {
     // `interp__weakref.py:193-205 W_Weakref` exact builtin payload. Like the
     // lifeline above, its allocation is selected by its translated GC layout;
     // Python class identity remains in the header's `w_class`. Append it after
-    // the lifeline so the already-published lifeline tid stays stable. The
-    // separate generated user-subclass layout remains mapdict-backed.
+    // the lifeline so the already-published lifeline tid stays stable. Every
+    // `weakref.ref` subclass instance carries this same payload, so its
+    // `w_slots` tail is traced here too.
     let weakref_descr =
         <pyre_object::weakref::W_Weakref as pyre_object::lltype::PyreClassPyTypeOf>::DESCRIPTOR;
     let weakref_object_tid = gc.register_type(TypeInfo::with_gc_ptrs(

pyre/pyre-object/src/weakref.rs

@@ -107,6 +107,8 @@ pub struct W_Weakref {
     pub w_callable: PyObjectRef,
     /// `W_Weakref.__init__: self.w_hash = None`.
     pub w_hash: PyObjectRef,
+    /// Native-subclass `__slots__` storage indexed by `Member.index`.
+    pub w_slots: PyObjectRef,
 }
 
 /// Allocate the exact builtin W_Weakref payload. The caller has already
@@ -122,6 +124,7 @@ pub fn w_weakref_object_new(
         w_obj_weak,
         w_callable,
         w_hash,
+        w_slots: PY_NULL,
     })
 }
 
@@ -157,6 +160,35 @@ pub unsafe fn w_weakref_object_set_hash(obj: PyObjectRef, value: PyObjectRef) {
     crate::gc_hook::try_gc_write_barrier(obj as *mut u8);
 }
 
+/// Address of `W_Weakref::w_slots` for the shared slot helpers.
+///
+/// # Safety
+/// `obj` must point to a valid `W_Weakref`.
+unsafe fn weakref_slots_field(obj: PyObjectRef) -> *mut PyObjectRef {
+    unsafe { &mut (*(obj as *mut W_Weakref)).w_slots }
+}
+
+/// # Safety
+/// The caller must uphold every validity, runtime-type, aliasing, and lifetime
+/// invariant required by the object and pointer arguments for the entire call.
+pub unsafe fn w_weakref_object_slot_get(obj: PyObjectRef, index: usize) -> Option<PyObjectRef> {
+    unsafe { crate::slots::slot_get(obj, index, weakref_slots_field) }
+}
+
+/// # Safety
+/// The caller must uphold every validity, runtime-type, aliasing, and lifetime
+/// invariant required by the object and pointer arguments for the entire call.
+pub unsafe fn w_weakref_object_slot_set(obj: PyObjectRef, index: usize, value: PyObjectRef) {
+    unsafe { crate::slots::slot_set(obj, index, value, weakref_slots_field) }
+}
+
+/// # Safety
+/// The caller must uphold every validity, runtime-type, aliasing, and lifetime
+/// invariant required by the object and pointer arguments for the entire call.
+pub unsafe fn w_weakref_object_slot_del(obj: PyObjectRef, index: usize) -> bool {
+    unsafe { crate::slots::slot_del(obj, index, weakref_slots_field) }
+}
+
 /// GC type id for the WEAKREF GcStruct. Registered by
 /// `pyre-jit::eval::init` after `W_INT_MUTABLE_CELL` and before the
 /// per-exception kind loop. A `debug_assert_eq!` in the registration
@@ -550,13 +582,24 @@ mod tests {
         }
         assert_eq!(unsafe { w_weakref_object_hash(weakref) }, hash);
         assert!(unsafe { w_weakref_object_callable(weakref) }.is_null());
+
+        let slot_value = 0x7000_usize as PyObjectRef;
+        unsafe { w_weakref_object_slot_set(weakref, 2, slot_value) };
+        assert_eq!(
+            unsafe { w_weakref_object_slot_get(weakref, 2) },
+            Some(slot_value)
+        );
+        assert!(unsafe { w_weakref_object_slot_del(weakref, 2) });
+        assert_eq!(unsafe { w_weakref_object_slot_get(weakref, 2) }, None);
+
         assert_eq!(
             W_Weakref::DESCRIPTOR.ptr_offsets,
             &[
                 std::mem::offset_of!(W_Weakref, ob) + std::mem::offset_of!(PyObject, w_class),
                 std::mem::offset_of!(W_Weakref, w_obj_weak),
                 std::mem::offset_of!(W_Weakref, w_callable),
                 std::mem::offset_of!(W_Weakref, w_hash),
+                std::mem::offset_of!(W_Weakref, w_slots),
             ]
         );
     }