Fix object.__getstate__ deadlock and panic on __slotnames__ (#8981)

object_getstate_default kept the __slotnames__ list read-locked while it
looked up each slot attribute. A getter or __getattr__ that changed the
list (as in test_descr.test_issue24097) then waited forever for the write
lock. A non-str entry in a cached __slotnames__ hit an unwrap and
panicked.

Take a reference to each name and drop the lock before the lookup, raise
TypeError for a non-str name, let errors other than AttributeError
propagate, and check the list size after each item, as CPython does.

Assisted-by: Claude Code:claude-opus-5-5

Lib/test/test_descr.py

@@ -5742,7 +5742,6 @@ class PicklingTests(unittest.TestCase):
                     objcopy2 = deepcopy(objcopy)
                     self._assert_is_copy(obj, objcopy2)
 
-    @unittest.skip("TODO: RUSTPYTHON")
     def test_issue24097(self):
         # Slot name is freed inside __getattr__ and is later used.
         class S(str):  # Not interned

crates/vm/src/builtins/object.rs

@@ -226,16 +226,23 @@ fn object_getstate_default(obj: &PyObject, required: bool, vm: &VirtualMachine)
         if slot_names_len > 0 {
             let slots = vm.ctx.new_dict();
             for i in 0..slot_names_len {
-                let borrowed_names = slot_names.borrow_vec();
-                // Check if slotnames changed during iteration
-                if borrowed_names.len() != slot_names_len {
+                // Release the list lock before the attribute lookup: a getter
+                // or `__getattr__` may mutate `__slotnames__`.
+                let name = slot_names.borrow_vec()[i].clone();
+                let name = name.downcast::<PyStr>().map_err(|name| {
+                    vm.new_type_error(format!(
+                        "attribute name must be string, not '{}'",
+                        name.class().name()
+                    ))
+                })?;
+                if let Some(value) = vm.get_attribute_opt(obj, &name)? {
+                    slots.set_item(name.as_wtf8(), value, vm)?;
+                }
+                // The list is stored on the class, so it may change while we
+                // iterate over it.
+                if slot_names.borrow_vec().len() != slot_names_len {
                     return Err(vm.new_runtime_error("__slotnames__ changed size during iteration"));
                 }
-                let name = borrowed_names[i].downcast_ref::<PyStr>().unwrap();
-                let Ok(value) = obj.get_attr(name, vm) else {
-                    continue;
-                };
-                slots.set_item(name.as_wtf8(), value, vm).unwrap();
             }
 
             if !slots.is_empty() {

extra_tests/snippets/builtin_object.py

@@ -193,3 +193,46 @@ with assert_raises(TypeError):
     raw_instance.value
 RawGetDescriptor.__get__ = lambda self, instance, owner: (self, instance, owner)
 assert raw_instance.value == (raw_descriptor, raw_instance, RawGetOwner)
+
+
+# object.__getstate__ reads names from a cached __slotnames__ list
+class NonStrSlotName:
+    __slotnames__ = [1]
+
+
+with assert_raises(TypeError) as caught:
+    NonStrSlotName().__getstate__()
+assert str(caught.exception) == "attribute name must be string, not 'int'"
+
+
+class GrowingSlotNames:
+    __slotnames__ = ["a", "b"]
+
+    def __getattr__(self, name):
+        if name == "a":
+            GrowingSlotNames.__slotnames__.append("c")
+            return 1
+        raise AttributeError(name)
+
+
+with assert_raises(RuntimeError):
+    GrowingSlotNames().__getstate__()
+
+
+class RaisingSlot:
+    __slotnames__ = ["x"]
+
+    def __getattr__(self, name):
+        raise ValueError("boom")
+
+
+with assert_raises(ValueError):
+    RaisingSlot().__getstate__()
+
+
+class MissingSlot:
+    __slotnames__ = ["x", "y"]
+    y = 2
+
+
+assert MissingSlot().__getstate__() == (None, {"y": 2})