diff --git a/crates/vm/src/builtins/classmethod.rs b/crates/vm/src/builtins/classmethod.rs index 911960bf691..21f1ae4ba10 100644 --- a/crates/vm/src/builtins/classmethod.rs +++ b/crates/vm/src/builtins/classmethod.rs @@ -57,11 +57,11 @@ impl GetDescriptor for PyClassMethod { ) -> PyResult { let (zelf, _obj) = Self::_unwrap(&zelf, obj, vm)?; let cls = cls.unwrap_or_else(|| _obj.class().to_owned().into()); - let call_descr_get: PyResult = zelf.callable.lock().get_attr("__get__", vm); + // Clone and release lock before calling Python code to prevent deadlock + let callable = zelf.callable.lock().clone(); + let call_descr_get: PyResult = callable.get_attr("__get__", vm); match call_descr_get { - Err(_) => Ok(PyBoundMethod::new(cls, zelf.callable.lock().clone()) - .into_ref(&vm.ctx) - .into()), + Err(_) => Ok(PyBoundMethod::new(cls, callable).into_ref(&vm.ctx).into()), Ok(call_descr_get) => call_descr_get.call((cls.clone(), cls), vm), } } diff --git a/crates/vm/src/builtins/property.rs b/crates/vm/src/builtins/property.rs index 41b05a60049..7ea36d39768 100644 --- a/crates/vm/src/builtins/property.rs +++ b/crates/vm/src/builtins/property.rs @@ -55,7 +55,8 @@ impl GetDescriptor for PyProperty { let (zelf, obj) = Self::_unwrap(&zelf_obj, obj, vm)?; if vm.is_none(&obj) { Ok(zelf_obj) - } else if let Some(getter) = zelf.getter.read().as_ref() { + } else if let Some(getter) = zelf.getter.read().clone() { + // Clone and release lock before calling Python code to prevent deadlock getter.call((obj,), vm) } else { let error_msg = zelf.format_property_error(&obj, "getter", vm)?; @@ -70,12 +71,12 @@ impl PyProperty { // Returns the name if available, None if not found, or propagates errors fn get_property_name(&self, vm: &VirtualMachine) -> PyResult> { // First check if name was set via __set_name__ - if let Some(name) = self.name.read().as_ref() { - return Ok(Some(name.clone())); + if let Some(name) = self.name.read().clone() { + return Ok(Some(name)); } - let getter = self.getter.read(); - let Some(getter) = getter.as_ref() else { + // Clone and release lock before calling Python code to prevent deadlock + let Some(getter) = self.getter.read().clone() else { return Ok(None); }; @@ -105,7 +106,8 @@ impl PyProperty { let zelf = zelf.try_to_ref::(vm)?; match value { PySetterValue::Assign(value) => { - if let Some(setter) = zelf.setter.read().as_ref() { + // Clone and release lock before calling Python code to prevent deadlock + if let Some(setter) = zelf.setter.read().clone() { setter.call((obj, value), vm).map(drop) } else { let error_msg = zelf.format_property_error(&obj, "setter", vm)?; @@ -113,7 +115,8 @@ impl PyProperty { } } PySetterValue::Delete => { - if let Some(deleter) = zelf.deleter.read().as_ref() { + // Clone and release lock before calling Python code to prevent deadlock + if let Some(deleter) = zelf.deleter.read().clone() { deleter.call((obj,), vm).map(drop) } else { let error_msg = zelf.format_property_error(&obj, "deleter", vm)?; @@ -273,23 +276,24 @@ impl PyProperty { } }; + // Clone and release lock before calling Python code to prevent deadlock // Check getter - if let Some(getter) = self.getter.read().as_ref() - && is_abstract(getter)? + if let Some(getter) = self.getter.read().clone() + && is_abstract(&getter)? { return Ok(vm.ctx.new_bool(true).into()); } // Check setter - if let Some(setter) = self.setter.read().as_ref() - && is_abstract(setter)? + if let Some(setter) = self.setter.read().clone() + && is_abstract(&setter)? { return Ok(vm.ctx.new_bool(true).into()); } // Check deleter - if let Some(deleter) = self.deleter.read().as_ref() - && is_abstract(deleter)? + if let Some(deleter) = self.deleter.read().clone() + && is_abstract(&deleter)? { return Ok(vm.ctx.new_bool(true).into()); } @@ -299,7 +303,8 @@ impl PyProperty { #[pygetset(setter)] fn set___isabstractmethod__(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if let Some(getter) = self.getter.read().to_owned() { + // Clone and release lock before calling Python code to prevent deadlock + if let Some(getter) = self.getter.read().clone() { getter.set_attr("__isabstractmethod__", value, vm)?; } Ok(()) diff --git a/crates/vm/src/stdlib/functools.rs b/crates/vm/src/stdlib/functools.rs index 26dff8b4426..77f352c78fc 100644 --- a/crates/vm/src/stdlib/functools.rs +++ b/crates/vm/src/stdlib/functools.rs @@ -248,15 +248,24 @@ mod _functools { type Args = FuncArgs; fn call(zelf: &Py, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let inner = zelf.inner.read(); - let mut combined_args = inner.args.as_slice().to_vec(); + // Clone and release lock before calling Python code to prevent deadlock + let (func, stored_args, keywords) = { + let inner = zelf.inner.read(); + ( + inner.func.clone(), + inner.args.clone(), + inner.keywords.clone(), + ) + }; + + let mut combined_args = stored_args.as_slice().to_vec(); combined_args.extend_from_slice(&args.args); // Merge keywords from self.keywords and args.kwargs let mut final_kwargs = IndexMap::new(); // Add keywords from self.keywords - for (key, value) in &*inner.keywords { + for (key, value) in &*keywords { let key_str = key .downcast::() .map_err(|_| vm.new_type_error("keywords must be strings"))?; @@ -268,9 +277,7 @@ mod _functools { final_kwargs.insert(key, value); } - inner - .func - .call(FuncArgs::new(combined_args, KwArgs::new(final_kwargs)), vm) + func.call(FuncArgs::new(combined_args, KwArgs::new(final_kwargs)), vm) } } @@ -280,15 +287,24 @@ mod _functools { // Check for recursive repr let obj = zelf.as_object(); if let Some(_guard) = ReprGuard::enter(vm, obj) { - let inner = zelf.inner.read(); - let func_repr = inner.func.repr(vm)?; + // Clone and release lock before calling Python code to prevent deadlock + let (func, args, keywords) = { + let inner = zelf.inner.read(); + ( + inner.func.clone(), + inner.args.clone(), + inner.keywords.clone(), + ) + }; + + let func_repr = func.repr(vm)?; let mut parts = vec![func_repr.as_str().to_owned()]; - for arg in inner.args.as_slice() { + for arg in args.as_slice() { parts.push(arg.repr(vm)?.as_str().to_owned()); } - for (key, value) in inner.keywords.clone() { + for (key, value) in &*keywords { // For string keys, use them directly without quotes let key_part = if let Ok(s) = key.clone().downcast::() { s.as_str().to_owned()