Fix potential deadlocks by youknowone · Pull Request #6509 · RustPython/RustPython

Walkthrough

Three builtin and stdlib modules refactored to prevent deadlocks by cloning callable objects and values under read locks, then releasing locks before invoking Python code. Changes affect classmethod descriptor access, property getter/setter/deleter invocation, and partial function callable/representation logic.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• implement more property features  #5828: Modifies property.rs descr_get and related property-access logic with similar lock-release patterns for getter/setter/deleter access.
• fix pyexpat hang #6507: Implements the same deadlock prevention pattern — clone handler under read lock and release before invoking Python code.
• PyPayload::into_ref #3744: Modifies classmethod.rs descr_get callable access and PyBoundMethod construction similarly.

Poem

🐰 A lock held tight can cause a knot,
When Python calls within its slot.
Release it first, the deadlock flees—
Clone and run with gentle ease! ✨

• 📝 Generate docstrings

• X
• Mastodon
• Reddit
• LinkedIn

crates/vm/src/builtins/classmethod.rs (2)

142-158: Remaining potential deadlock: lock held during get_attr calls.

These getters still hold the lock while calling Python code, which could cause the same deadlock this PR aims to fix:

// Line 143 - lock held during get_attr
self.callable.lock().get_attr("__module__", vm)

Consider applying the same clone-and-release pattern:

🔎 Proposed fix

     #[pygetset]
     fn __module__(&self, vm: &VirtualMachine) -> PyResult {
-        self.callable.lock().get_attr("__module__", vm)
+        let callable = self.callable.lock().clone();
+        callable.get_attr("__module__", vm)
     }

     #[pygetset]
     fn __qualname__(&self, vm: &VirtualMachine) -> PyResult {
-        self.callable.lock().get_attr("__qualname__", vm)
+        let callable = self.callable.lock().clone();
+        callable.get_attr("__qualname__", vm)
     }

     #[pygetset]
     fn __name__(&self, vm: &VirtualMachine) -> PyResult {
-        self.callable.lock().get_attr("__name__", vm)
+        let callable = self.callable.lock().clone();
+        callable.get_attr("__name__", vm)
     }

     #[pygetset]
     fn __annotations__(&self, vm: &VirtualMachine) -> PyResult {
-        self.callable.lock().get_attr("__annotations__", vm)
+        let callable = self.callable.lock().clone();
+        callable.get_attr("__annotations__", vm)
     }

---

169-175: Remaining potential deadlock: lock held during set_attr.

The setter holds the lock while calling set_attr, which can invoke Python code.

🔎 Proposed fix

     #[pygetset(setter)]
     fn set___isabstractmethod__(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.callable
-            .lock()
-            .set_attr("__isabstractmethod__", value, vm)?;
+        let callable = self.callable.lock().clone();
+        callable.set_attr("__isabstractmethod__", value, vm)?;
         Ok(())
     }

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro