Drop old PyObjectRef outside type lock to prevent deadlock

Dropping values inside with_type_lock can trigger weakref callbacks,
which may access attributes (LOAD_ATTR specialization) and re-acquire
the non-reentrant type mutex, causing deadlock.

Return old values from lock closures so they drop after lock release.

Author: youknowone

.github/workflows/cron-ci.yaml

@@ -12,7 +12,7 @@ on:
 name: Periodic checks/tasks
 
 env:
-  CARGO_ARGS: --no-default-features --features stdlib,importlib,encodings,ssl-rustls,jit
+  CARGO_ARGS: --no-default-features --features stdlib,importlib,stdio,encodings,ssl-rustls,jit,host_env
   PYTHON_VERSION: "3.14.3"
 
 jobs:
@@ -35,7 +35,7 @@ jobs:
           python-version: ${{ env.PYTHON_VERSION }}
       - run: sudo apt-get update && sudo apt-get -y install lcov
       - name: Run cargo-llvm-cov with Rust tests.
-        run: cargo llvm-cov --no-report --workspace --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher --verbose --no-default-features --features stdlib,importlib,encodings,ssl-rustls,jit
+        run: cargo llvm-cov --no-report --workspace --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher --verbose --no-default-features --features stdlib,importlib,stdio,encodings,ssl-rustls,jit,host_env
       - name: Run cargo-llvm-cov with Python snippets.
         run: python scripts/cargo-llvm-cov.py
         continue-on-error: true
@@ -48,7 +48,7 @@ jobs:
         if: ${{ github.event_name != 'pull_request' }}
         uses: codecov/codecov-action@v5
         with:
-          file: ./codecov.lcov
+          files: ./codecov.lcov
 
   testdata:
     name: Collect regression test data
@@ -170,12 +170,12 @@ jobs:
       - name: restructure generated files
         run: |
           cd ./target/criterion/reports
-          find -type d -name cpython | xargs rm -rf
-          find -type d -name rustpython | xargs rm -rf
-          find -mindepth 2 -maxdepth 2 -name violin.svg | xargs rm -rf
-          find -type f -not -name violin.svg | xargs rm -rf
-          for file in $(find -type f -name violin.svg); do mv $file $(echo $file | sed -E "s_\./([^/]+)/([^/]+)/violin\.svg_./\1/\2.svg_"); done
-          find -mindepth 2 -maxdepth 2 -type d | xargs rm -rf
+          find . -type d -name cpython -print0 | xargs -0 rm -rf
+          find . -type d -name rustpython -print0 | xargs -0 rm -rf
+          find . -mindepth 2 -maxdepth 2 -name violin.svg -print0 | xargs -0 rm -rf
+          find . -type f -not -name violin.svg -print0 | xargs -0 rm -rf
+          find . -type f -name violin.svg -exec sh -c 'for file; do mv "$file" "$(echo "$file" | sed -E "s_\./([^/]+)/([^/]+)/violin\.svg_./\1/\2.svg_")"; done' _ {} +
+          find . -mindepth 2 -maxdepth 2 -type d -print0 | xargs -0 rm -rf
           cd ..
           mv reports/* .
           rmdir reports

Lib/test/test_class.py

@@ -614,6 +614,7 @@ def assertNotOrderable(self, a, b):
         with self.assertRaises(TypeError):
             a >= b
 
+    @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: 2582729627104 != 2582729622496")
     def testHashComparisonOfMethods(self):
         # Test comparison and hash of methods
         class A:

crates/vm/src/builtins/type.rs

@@ -1562,18 +1562,19 @@ impl PyType {
         drop(attrs);
 
         let none = vm.ctx.none();
-        Ok(Self::with_type_lock(vm, || {
+        let (result, _prev) = Self::with_type_lock(vm, || {
             let mut attrs = self.attributes.write();
             if let Some(annotate) = attrs.get(annotate_key).cloned() {
-                return annotate;
+                return (annotate, None);
             }
             if let Some(annotate) = attrs.get(annotate_func_key).cloned() {
-                return annotate;
+                return (annotate, None);
             }
             self.modified_inner();
-            attrs.insert(annotate_func_key, none.clone());
-            none
-        }))
+            let prev = attrs.insert(annotate_func_key, none.clone());
+            (none, prev)
+        });
+        Ok(result)
     }
 
     #[pygetset(setter)]
@@ -1596,13 +1597,16 @@ impl PyType {
             return Err(vm.new_type_error("__annotate__ must be callable or None"));
         }
 
-        Self::with_type_lock(vm, || {
+        let _prev_values = Self::with_type_lock(vm, || {
             self.modified_inner();
             let mut attrs = self.attributes.write();
-            if !vm.is_none(&value) {
-                attrs.swap_remove(identifier!(vm, __annotations_cache__));
-            }
-            attrs.insert(identifier!(vm, __annotate_func__), value);
+            let removed = if !vm.is_none(&value) {
+                attrs.swap_remove(identifier!(vm, __annotations_cache__))
+            } else {
+                None
+            };
+            let prev = attrs.insert(identifier!(vm, __annotate_func__), value);
+            (removed, prev)
         });
 
         Ok(())
@@ -1665,20 +1669,21 @@ impl PyType {
             vm.ctx.new_dict().into()
         };
 
-        Ok(Self::with_type_lock(vm, || {
+        let (result, _prev) = Self::with_type_lock(vm, || {
             let mut attrs = self.attributes.write();
             if let Some(existing) = attrs.get(annotations_key).cloned()
                 && !existing.class().is(vm.ctx.types.getset_type)
             {
-                return existing;
+                return (existing, None);
             }
             if let Some(existing) = attrs.get(annotations_cache_key).cloned() {
-                return existing;
+                return (existing, None);
             }
             self.modified_inner();
-            attrs.insert(annotations_cache_key, annotations.clone());
-            annotations
-        }))
+            let prev = attrs.insert(annotations_cache_key, annotations.clone());
+            (annotations, prev)
+        });
+        Ok(result)
     }
 
     #[pygetset(setter)]
@@ -1694,44 +1699,42 @@ impl PyType {
             )));
         }
 
-        Self::with_type_lock(vm, || {
+        let _prev_values = Self::with_type_lock(vm, || {
             self.modified_inner();
             let mut attrs = self.attributes.write();
             let has_annotations = attrs.contains_key(identifier!(vm, __annotations__));
 
+            let mut prev = Vec::new();
             match value {
                 crate::function::PySetterValue::Assign(value) => {
                     let key = if has_annotations {
                         identifier!(vm, __annotations__)
                     } else {
                         identifier!(vm, __annotations_cache__)
                     };
-                    attrs.insert(key, value);
+                    prev.extend(attrs.insert(key, value));
                     if has_annotations {
-                        attrs.swap_remove(identifier!(vm, __annotations_cache__));
+                        prev.extend(attrs.swap_remove(identifier!(vm, __annotations_cache__)));
                     }
                 }
                 crate::function::PySetterValue::Delete => {
                     let removed = if has_annotations {
-                        attrs
-                            .swap_remove(identifier!(vm, __annotations__))
-                            .is_some()
+                        attrs.swap_remove(identifier!(vm, __annotations__))
                     } else {
-                        attrs
-                            .swap_remove(identifier!(vm, __annotations_cache__))
-                            .is_some()
+                        attrs.swap_remove(identifier!(vm, __annotations_cache__))
                     };
-                    if !removed {
+                    if removed.is_none() {
                         return Err(vm.new_attribute_error("__annotations__"));
                     }
+                    prev.extend(removed);
                     if has_annotations {
-                        attrs.swap_remove(identifier!(vm, __annotations_cache__));
+                        prev.extend(attrs.swap_remove(identifier!(vm, __annotations_cache__)));
                     }
                 }
             }
-            attrs.swap_remove(identifier!(vm, __annotate_func__));
-            attrs.swap_remove(identifier!(vm, __annotate__));
-            Ok(())
+            prev.extend(attrs.swap_remove(identifier!(vm, __annotate_func__)));
+            prev.extend(attrs.swap_remove(identifier!(vm, __annotate__)));
+            Ok(prev)
         })?;
 
         Ok(())
@@ -1765,11 +1768,12 @@ impl PyType {
     #[pygetset(setter)]
     fn set___module__(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         self.check_set_special_type_attr(identifier!(vm, __module__), vm)?;
-        Self::with_type_lock(vm, || {
+        let _prev_values = Self::with_type_lock(vm, || {
             self.modified_inner();
-            self.attributes
-                .write()
-                .insert(identifier!(vm, __module__), value);
+            let mut attributes = self.attributes.write();
+            let removed = attributes.swap_remove(identifier!(vm, __firstlineno__));
+            let prev = attributes.insert(identifier!(vm, __module__), value);
+            (removed, prev)
         });
         Ok(())
     }
@@ -1896,9 +1900,9 @@ impl PyType {
         match value {
             PySetterValue::Assign(val) => {
                 self.check_set_special_type_attr(key, vm)?;
-                Self::with_type_lock(vm, || {
+                let _prev_value = Self::with_type_lock(vm, || {
                     self.modified_inner();
-                    self.attributes.write().insert(key, val.into());
+                    self.attributes.write().insert(key, val.into())
                 });
             }
             PySetterValue::Delete => {
@@ -1908,9 +1912,9 @@ impl PyType {
                         self.slot_name()
                     )));
                 }
-                Self::with_type_lock(vm, || {
+                let _prev_value = Self::with_type_lock(vm, || {
                     self.modified_inner();
-                    self.attributes.write().shift_remove(&key);
+                    self.attributes.write().shift_remove(&key)
                 });
             }
         }
@@ -2519,11 +2523,11 @@ impl Py<PyType> {
         // Check if we can set this special type attribute
         self.check_set_special_type_attr(identifier!(vm, __doc__), vm)?;
 
-        PyType::with_type_lock(vm, || {
+        let _prev_value = PyType::with_type_lock(vm, || {
             self.modified_inner();
             self.attributes
                 .write()
-                .insert(identifier!(vm, __doc__), value);
+                .insert(identifier!(vm, __doc__), value)
         });
 
         Ok(())
@@ -2586,14 +2590,17 @@ impl SetAttr for PyType {
         }
         let assign = value.is_assign();
 
-        Self::with_type_lock(vm, || {
+        // Drop old value OUTSIDE the type lock to avoid deadlock:
+        // dropping may trigger weakref callbacks → method calls →
+        // LOAD_ATTR specialization → version_for_specialization → type lock.
+        let _prev_value = Self::with_type_lock(vm, || {
             // Invalidate inline caches before modifying attributes.
             // This ensures other threads see the version invalidation before
             // any attribute changes, preventing use-after-free of cached descriptors.
             zelf.modified_inner();
 
             if let PySetterValue::Assign(value) = value {
-                zelf.attributes.write().insert(attr_name, value);
+                Ok(zelf.attributes.write().insert(attr_name, value))
             } else {
                 let prev_value = zelf.attributes.write().shift_remove(attr_name); // TODO: swap_remove applicable?
                 if prev_value.is_none() {
@@ -2603,8 +2610,8 @@ impl SetAttr for PyType {
                         attr_name,
                     )));
                 }
+                Ok(prev_value)
             }
-            Ok(())
         })?;
 
         if attr_name.as_wtf8().starts_with("__") && attr_name.as_wtf8().ends_with("__") {

crates/vm/src/stdlib/_ctypes/base.rs

@@ -2554,11 +2554,10 @@ fn make_fields(
         }
 
         let new_descr = super::PyCField::new_from_field(fdescr, index, offset);
-        cls.as_object().set_attr(
+        cls.set_attr(
             vm.ctx.intern_str(fname.as_wtf8()),
             new_descr.to_pyobject(vm),
-            vm,
-        )?;
+        );
     }
 
     Ok(())
@@ -2597,11 +2596,10 @@ pub(super) fn make_anon_fields(cls: &Py<PyType>, vm: &VirtualMachine) -> PyResul
 
         let mut new_descr = super::PyCField::new_from_field(descr, 0, 0);
         new_descr.set_anonymous(true);
-        cls.as_object().set_attr(
+        cls.set_attr(
             vm.ctx.intern_str(fname.as_wtf8()),
             new_descr.to_pyobject(vm),
-            vm,
-        )?;
+        );
 
         make_fields(cls, descr, descr.index, descr.offset, vm)?;
     }

crates/vm/src/stdlib/_ctypes/structure.rs

@@ -498,11 +498,7 @@ impl PyCStructType {
             };
 
             // Set the CField as a class attribute
-            cls.as_object().set_attr(
-                vm.ctx.intern_str(name.clone()),
-                c_field.to_pyobject(vm),
-                vm,
-            )?;
+            cls.set_attr(vm.ctx.intern_str(name.clone()), c_field.to_pyobject(vm));
 
             // Update tracking - don't advance offset for packed bitfields
             if field_advances_offset {

crates/vm/src/stdlib/_ctypes/union.rs

@@ -312,8 +312,7 @@ impl PyCUnionType {
                 PyCField::new(name.clone(), field_type_ref, 0, size as isize, index)
             };
 
-            cls.as_object()
-                .set_attr(vm.ctx.intern_str(name), c_field.to_pyobject(vm), vm)?;
+            cls.set_attr(vm.ctx.intern_str(name), c_field.to_pyobject(vm));
         }
 
         // Calculate total_align and aligned_size