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.
This commit is contained in:
Jeong, YunWon committed 2026-03-19 22:13:28 +09:00
1 parent e0886e2fb6
commit add34a2f19
1 file changed
+51 -44
+51 -44
View File
@@ -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,11 +1699,12 @@ 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 {
@@ -1706,32 +1712,29 @@ impl PyType {
} 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("__") {