From 471fe551fa67ef1962d19f1a37a71f1dbe43e7d4 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 8 Mar 2026 21:30:59 +0900 Subject: [PATCH 01/10] Align BINARY_OP_EXTEND with CPython descriptor cache model --- crates/vm/src/frame.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index d8e3d35f9e3..d503c64f1de 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -1051,7 +1051,7 @@ struct ExecutingFrame<'a> { } #[inline] -fn specialization_compact_int_value(i: &PyInt, vm: &VirtualMachine) -> Option { +fn cpython_compact_int_value(i: &PyInt, vm: &VirtualMachine) -> Option { // _PyLong_IsCompact(): a one-digit PyLong (base 2^30), // i.e. abs(value) <= 2^30 - 1. const CPYTHON_COMPACT_LONG_ABS_MAX: i64 = (1i64 << 30) - 1; @@ -1066,7 +1066,7 @@ fn specialization_compact_int_value(i: &PyInt, vm: &VirtualMachine) -> Option Option { obj.downcast_ref_if_exact::(vm) - .and_then(|i| specialization_compact_int_value(i, vm)) + .and_then(|i| cpython_compact_int_value(i, vm)) } #[inline] @@ -1075,7 +1075,7 @@ fn exact_float_from_obj(obj: &PyObject, vm: &VirtualMachine) -> Option { } #[inline] -fn specialization_nonnegative_compact_index(i: &PyInt, vm: &VirtualMachine) -> Option { +fn cpython_nonnegative_compact_index(i: &PyInt, vm: &VirtualMachine) -> Option { // _PyLong_IsNonNegativeCompact(): a single base-2^30 digit. const CPYTHON_COMPACT_LONG_MAX: u64 = (1u64 << 30) - 1; let v = i.try_to_primitive::(vm).ok()?; @@ -1176,7 +1176,7 @@ long_float_action!(compactlong_float_subtract, -); long_float_action!(compactlong_float_multiply, *); long_float_action!(compactlong_float_true_div, /); -static BINARY_OP_EXTEND_DESCRIPTORS: &[BinaryOpExtendSpecializationDescr] = &[ +static BINARY_OP_EXTEND_DESCRS: &[BinaryOpExtendSpecializationDescr] = &[ // long-long arithmetic BinaryOpExtendSpecializationDescr { oparg: bytecode::BinaryOperator::Or, @@ -3999,7 +3999,7 @@ impl ExecutingFrame<'_> { let value = self.pop_value(); if let Some(list) = obj.downcast_ref_if_exact::(vm) && let Some(int_idx) = idx.downcast_ref_if_exact::(vm) - && let Some(i) = specialization_nonnegative_compact_index(int_idx, vm) + && let Some(i) = cpython_nonnegative_compact_index(int_idx, vm) { let mut vec = list.borrow_vec_mut(); if i < vec.len() { @@ -4099,7 +4099,7 @@ impl ExecutingFrame<'_> { if let (Some(list), Some(idx)) = ( a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), - ) && let Some(i) = specialization_nonnegative_compact_index(idx, vm) + ) && let Some(i) = cpython_nonnegative_compact_index(idx, vm) { let vec = list.borrow_vec(); if i < vec.len() { @@ -4119,7 +4119,7 @@ impl ExecutingFrame<'_> { if let (Some(tuple), Some(idx)) = ( a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), - ) && let Some(i) = specialization_nonnegative_compact_index(idx, vm) + ) && let Some(i) = cpython_nonnegative_compact_index(idx, vm) { let elements = tuple.as_slice(); if i < elements.len() { @@ -4161,7 +4161,7 @@ impl ExecutingFrame<'_> { if let (Some(a_str), Some(b_int)) = ( a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), - ) && let Some(i) = specialization_nonnegative_compact_index(b_int, vm) + ) && let Some(i) = cpython_nonnegative_compact_index(b_int, vm) && let Ok(ch) = a_str.getitem_by_index(vm, i as isize) && ch.is_ascii() { @@ -5191,8 +5191,8 @@ impl ExecutingFrame<'_> { a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), ) && let (Some(a_val), Some(b_val)) = ( - specialization_compact_int_value(a_int, vm), - specialization_compact_int_value(b_int, vm), + cpython_compact_int_value(a_int, vm), + cpython_compact_int_value(b_int, vm), ) { let op = self.compare_op_from_arg(arg); let result = op.eval_ord(a_val.cmp(&b_val)); @@ -7268,7 +7268,7 @@ impl ExecutingFrame<'_> { if ptr == 0 { return None; } - // SAFETY: We only store pointers to entries in `BINARY_OP_EXTEND_DESCRIPTORS`. + // SAFETY: We only store pointers to entries in `BINARY_OP_EXTEND_DESCRS`. Some(unsafe { &*(ptr as *const BinaryOpExtendSpecializationDescr) }) } @@ -7280,7 +7280,7 @@ impl ExecutingFrame<'_> { rhs: &PyObject, vm: &VirtualMachine, ) -> Option<&'static BinaryOpExtendSpecializationDescr> { - BINARY_OP_EXTEND_DESCRIPTORS + BINARY_OP_EXTEND_DESCRS .iter() .find(|d| d.oparg == op && (d.guard)(lhs, rhs, vm)) } @@ -7797,7 +7797,7 @@ impl ExecutingFrame<'_> { bytecode::BinaryOperator::Subscr => { let b_is_nonnegative_int = b .downcast_ref_if_exact::(vm) - .is_some_and(|i| specialization_nonnegative_compact_index(i, vm).is_some()); + .is_some_and(|i| cpython_nonnegative_compact_index(i, vm).is_some()); if a.downcast_ref_if_exact::(vm).is_some() && b_is_nonnegative_int { Some(Instruction::BinaryOpSubscrListInt) } else if a.downcast_ref_if_exact::(vm).is_some() && b_is_nonnegative_int { @@ -8620,8 +8620,8 @@ impl ExecutingFrame<'_> { a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), ) { - if specialization_compact_int_value(a_int, vm).is_some() - && specialization_compact_int_value(b_int, vm).is_some() + if cpython_compact_int_value(a_int, vm).is_some() + && cpython_compact_int_value(b_int, vm).is_some() { Some(Instruction::CompareOpInt) } else { @@ -8919,7 +8919,7 @@ impl ExecutingFrame<'_> { idx.downcast_ref_if_exact::(vm), ) { let list_len = list.borrow_vec().len(); - if specialization_nonnegative_compact_index(int_idx, vm).is_some_and(|i| i < list_len) { + if cpython_nonnegative_compact_index(int_idx, vm).is_some_and(|i| i < list_len) { Some(Instruction::StoreSubscrListInt) } else { None From b3daabf169d19440671298de8d9abb4b54fb255f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 8 Mar 2026 22:18:47 +0900 Subject: [PATCH 02/10] Align type _spec_cache and latin1 singleton string paths --- crates/vm/src/builtins/type.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 0f63e7106df..8c3d1366231 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -276,7 +276,6 @@ pub struct TypeSpecializationCache { pub init: PyAtomicRef>, pub getitem: PyAtomicRef>, pub getitem_version: AtomicU32, - // Serialize cache writes/invalidation similar to CPython's BEGIN_TYPE_LOCK. write_lock: PyMutex<()>, retired: PyRwLock>, } @@ -302,9 +301,6 @@ impl TypeSpecializationCache { #[inline] fn swap_init(&self, new_init: Option>, vm: Option<&VirtualMachine>) { if let Some(vm) = vm { - // Keep replaced refs alive for the currently executing frame, matching - // CPython-style "old pointer remains valid during ongoing execution" - // without accumulating global retired refs. self.init.swap_to_temporary_refs(new_init, vm); return; } @@ -329,8 +325,6 @@ impl TypeSpecializationCache { #[inline] fn invalidate_for_type_modified(&self) { let _guard = self.write_lock.lock(); - // _spec_cache contract: type modification invalidates all cached - // specialization functions. self.swap_init(None, None); self.swap_getitem(None, None); } @@ -962,18 +956,17 @@ impl PyType { if func_version == 0 { return false; } - ext.specialization_cache - .swap_getitem(Some(getitem), Some(vm)); ext.specialization_cache .getitem_version - .store(func_version, Ordering::Relaxed); + .store(func_version, Ordering::Release); + ext.specialization_cache + .swap_getitem(Some(getitem), Some(vm)); true } /// Read cached __getitem__ for BINARY_OP_SUBSCR_GETITEM specialization. pub(crate) fn get_cached_getitem_for_specialization(&self) -> Option<(PyRef, u32)> { let ext = self.heaptype_ext.as_ref()?; - // Match CPython check order: pointer (Acquire) then function version. let getitem = ext .specialization_cache .getitem @@ -981,7 +974,7 @@ impl PyType { let cached_version = ext .specialization_cache .getitem_version - .load(Ordering::Relaxed); + .load(Ordering::Acquire); if cached_version == 0 { return None; } From e19335e8f26070a5fa841b7d3b1b46fca41e161f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 8 Mar 2026 22:26:24 +0900 Subject: [PATCH 03/10] Tighten CALL_ALLOC_AND_ENTER_INIT stack-space guard --- crates/vm/src/frame.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index d503c64f1de..25b07eaa0cc 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -4774,9 +4774,6 @@ impl ExecutingFrame<'_> { && let Some(init_func) = cls.get_cached_init_for_specialization(cached_version) && let Some(cls_alloc) = cls.slots.alloc.load() { - // Match CPython's `code->co_framesize + _Py_InitCleanup.co_framesize` - // shape, using RustPython's datastack-backed frame size - // equivalent for the extra shim frame. let init_cleanup_stack_bytes = datastack_frame_size_bytes_for_code(&vm.ctx.init_cleanup_code) .expect("_Py_InitCleanup shim is not a generator/coroutine"); From 9df4787aedabfa80a085bcc0c39051ff556f8795 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 8 Mar 2026 23:09:00 +0900 Subject: [PATCH 04/10] Align call-init frame flow and spec cache atomic ordering --- crates/vm/src/frame.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 25b07eaa0cc..af224aa6e0d 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -4784,9 +4784,8 @@ impl ExecutingFrame<'_> { ) { return self.execute_call_vectorcall(nargs, vm); } - // CPython creates `_Py_InitCleanup` + `__init__` frames here. - // Keep the guard conservative and deopt when the effective - // recursion budget for those two frames is not available. + // Two frames are created: `_Py_InitCleanup` + `__init__`. + // Guard recursion limit accordingly and fall back. if self.specialization_call_recursion_guard_with_extra_frames(vm, 1) { return self.execute_call_vectorcall(nargs, vm); } From fb0dfa102cee20642baa37cd1f636cc59053830f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 9 Mar 2026 00:02:12 +0900 Subject: [PATCH 05/10] address review: invalidate init cache on type modification, add cspell words --- .cspell.json | 6 ++++ crates/vm/src/builtins/function.rs | 56 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/.cspell.json b/.cspell.json index f6887a7bb35..f82920c6cff 100644 --- a/.cspell.json +++ b/.cspell.json @@ -60,7 +60,13 @@ "dedentations", "dedents", "deduped", + "compactlong", + "compactlongs", "deoptimize", + "descrs", + "downcastable", + "downcasted", + "dumpable", "emscripten", "excs", "interps", diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 0003720c669..8a2b5c4173a 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -736,6 +736,62 @@ impl Py { frame } + pub(crate) fn prepare_exact_args_frame( + &self, + mut args: Vec, + vm: &VirtualMachine, + ) -> FrameRef { + let code: PyRef = (*self.code).to_owned(); + + debug_assert_eq!(args.len(), code.arg_count as usize); + debug_assert!(code.flags.contains(bytecode::CodeFlags::OPTIMIZED)); + debug_assert!( + !code + .flags + .intersects(bytecode::CodeFlags::VARARGS | bytecode::CodeFlags::VARKEYWORDS) + ); + debug_assert_eq!(code.kwonlyarg_count, 0); + debug_assert!( + !code + .flags + .intersects(bytecode::CodeFlags::GENERATOR | bytecode::CodeFlags::COROUTINE) + ); + + let locals = if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) { + None + } else { + Some(ArgMapping::from_dict_exact(self.globals.clone())) + }; + + let frame = Frame::new( + code.clone(), + Scope::new(locals, self.globals.clone()), + self.builtins.clone(), + self.closure.as_ref().map_or(&[], |c| c.as_slice()), + Some(self.to_owned().into()), + true, // Exact-args fast path is only used for non-gen/coro functions. + vm, + ) + .into_ref(&vm.ctx); + + { + let fastlocals = unsafe { frame.fastlocals_mut() }; + for (slot, arg) in fastlocals.iter_mut().zip(args.drain(..)) { + *slot = Some(arg); + } + } + + if let Some(cell2arg) = code.cell2arg.as_deref() { + let fastlocals = unsafe { frame.fastlocals_mut() }; + for (cell_idx, arg_idx) in cell2arg.iter().enumerate().filter(|(_, i)| **i != -1) { + let x = fastlocals[*arg_idx as usize].take(); + frame.set_cell_contents(cell_idx, x); + } + } + + frame + } + /// Fast path for calling a simple function with exact positional args. /// Skips FuncArgs allocation, prepend_arg, and fill_locals_from_args. /// Only valid when: CO_OPTIMIZED, no VARARGS, no VARKEYWORDS, no kwonlyargs, From 76e6ece9412aa52542fed6d589e54a8f20fe40e6 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 9 Mar 2026 02:10:19 +0900 Subject: [PATCH 06/10] address review: check datastack space for extra_bytes, require CO_OPTIMIZED in vectorcall fast path --- .cspell.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/.cspell.json b/.cspell.json index f82920c6cff..dd9ffb34850 100644 --- a/.cspell.json +++ b/.cspell.json @@ -60,8 +60,6 @@ "dedentations", "dedents", "deduped", - "compactlong", - "compactlongs", "deoptimize", "descrs", "downcastable", From cb2db074633faa83e1e52e6461e53d30b981a89a Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 9 Mar 2026 12:32:38 +0900 Subject: [PATCH 07/10] Extract datastack_frame_size_bytes_for_code, skip monitoring for init_cleanup frames, guard trace dispatch - Extract datastack_frame_size_bytes_for_code as free function, use it to compute init_cleanup stack bytes instead of hardcoded constant - Add monitoring_disabled_for_code to skip instrumentation for synthetic init_cleanup code object in RESUME and execute_instrumented - Add is_trace_event guard so profile-only events skip trace_func dispatch - Reformat core.rs (rustfmt) --- crates/vm/src/builtins/function.rs | 56 ------------------------------ 1 file changed, 56 deletions(-) diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 8a2b5c4173a..0003720c669 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -736,62 +736,6 @@ impl Py { frame } - pub(crate) fn prepare_exact_args_frame( - &self, - mut args: Vec, - vm: &VirtualMachine, - ) -> FrameRef { - let code: PyRef = (*self.code).to_owned(); - - debug_assert_eq!(args.len(), code.arg_count as usize); - debug_assert!(code.flags.contains(bytecode::CodeFlags::OPTIMIZED)); - debug_assert!( - !code - .flags - .intersects(bytecode::CodeFlags::VARARGS | bytecode::CodeFlags::VARKEYWORDS) - ); - debug_assert_eq!(code.kwonlyarg_count, 0); - debug_assert!( - !code - .flags - .intersects(bytecode::CodeFlags::GENERATOR | bytecode::CodeFlags::COROUTINE) - ); - - let locals = if code.flags.contains(bytecode::CodeFlags::NEWLOCALS) { - None - } else { - Some(ArgMapping::from_dict_exact(self.globals.clone())) - }; - - let frame = Frame::new( - code.clone(), - Scope::new(locals, self.globals.clone()), - self.builtins.clone(), - self.closure.as_ref().map_or(&[], |c| c.as_slice()), - Some(self.to_owned().into()), - true, // Exact-args fast path is only used for non-gen/coro functions. - vm, - ) - .into_ref(&vm.ctx); - - { - let fastlocals = unsafe { frame.fastlocals_mut() }; - for (slot, arg) in fastlocals.iter_mut().zip(args.drain(..)) { - *slot = Some(arg); - } - } - - if let Some(cell2arg) = code.cell2arg.as_deref() { - let fastlocals = unsafe { frame.fastlocals_mut() }; - for (cell_idx, arg_idx) in cell2arg.iter().enumerate().filter(|(_, i)| **i != -1) { - let x = fastlocals[*arg_idx as usize].take(); - frame.set_cell_contents(cell_idx, x); - } - } - - frame - } - /// Fast path for calling a simple function with exact positional args. /// Skips FuncArgs allocation, prepend_arg, and fill_locals_from_args. /// Only valid when: CO_OPTIMIZED, no VARARGS, no VARKEYWORDS, no kwonlyargs, From 38de7462c0ca6392abb400a29f4af8abab232b5c Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 9 Mar 2026 12:41:30 +0900 Subject: [PATCH 08/10] Fix Constants newtype usage in init_cleanup_code --- .cspell.json | 1 - crates/vm/src/frame.rs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.cspell.json b/.cspell.json index dd9ffb34850..227ab6e4fdf 100644 --- a/.cspell.json +++ b/.cspell.json @@ -61,7 +61,6 @@ "dedents", "deduped", "deoptimize", - "descrs", "downcastable", "downcasted", "dumpable", diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index af224aa6e0d..59f496e2423 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -1176,7 +1176,7 @@ long_float_action!(compactlong_float_subtract, -); long_float_action!(compactlong_float_multiply, *); long_float_action!(compactlong_float_true_div, /); -static BINARY_OP_EXTEND_DESCRS: &[BinaryOpExtendSpecializationDescr] = &[ +static BINARY_OP_EXTEND_DESCRIPTORS: &[BinaryOpExtendSpecializationDescr] = &[ // long-long arithmetic BinaryOpExtendSpecializationDescr { oparg: bytecode::BinaryOperator::Or, @@ -7264,7 +7264,7 @@ impl ExecutingFrame<'_> { if ptr == 0 { return None; } - // SAFETY: We only store pointers to entries in `BINARY_OP_EXTEND_DESCRS`. + // SAFETY: We only store pointers to entries in `BINARY_OP_EXTEND_DESCRIPTORS`. Some(unsafe { &*(ptr as *const BinaryOpExtendSpecializationDescr) }) } @@ -7276,7 +7276,7 @@ impl ExecutingFrame<'_> { rhs: &PyObject, vm: &VirtualMachine, ) -> Option<&'static BinaryOpExtendSpecializationDescr> { - BINARY_OP_EXTEND_DESCRS + BINARY_OP_EXTEND_DESCRIPTORS .iter() .find(|d| d.oparg == op && (d.guard)(lhs, rhs, vm)) } From ea2d66e799bb4a32f42bab7c35faebe29e97b29e Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 9 Mar 2026 22:33:31 +0900 Subject: [PATCH 09/10] type lock --- crates/vm/src/builtins/type.rs | 418 ++++++++++++++-------- crates/vm/src/frame.rs | 45 +-- crates/vm/src/stdlib/_ctypes/base.rs | 10 +- crates/vm/src/stdlib/_ctypes/structure.rs | 6 +- crates/vm/src/stdlib/_ctypes/union.rs | 3 +- crates/vm/src/stdlib/posix.rs | 1 + crates/vm/src/vm/interpreter.rs | 1 + crates/vm/src/vm/mod.rs | 2 + 8 files changed, 304 insertions(+), 182 deletions(-) diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 8c3d1366231..3148ff420f9 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -451,9 +451,15 @@ fn is_subtype_with_mro(a_mro: &[PyTypeRef], a: &Py, b: &Py) -> b } impl PyType { + #[inline] + fn with_type_lock(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { + let _guard = vm.state.type_mutex.lock(); + f() + } + /// Assign a fresh version tag. Returns 0 if the version counter has been /// exhausted, in which case no new cache entries can be created. - pub fn assign_version_tag(&self) -> u32 { + fn assign_version_tag_inner(&self) -> u32 { let v = self.tp_version_tag.load(Ordering::Acquire); if v != 0 { return v; @@ -461,7 +467,7 @@ impl PyType { // Assign versions to all direct bases first (MRO invariant). for base in self.bases.read().iter() { - if base.assign_version_tag() == 0 { + if base.assign_version_tag_inner() == 0 { return 0; } } @@ -481,8 +487,23 @@ impl PyType { } } + pub fn assign_version_tag(&self) -> u32 { + self.assign_version_tag_inner() + } + + pub(crate) fn version_for_specialization(&self, vm: &VirtualMachine) -> u32 { + Self::with_type_lock(vm, || { + let version = self.tp_version_tag.load(Ordering::Acquire); + if version == 0 { + self.assign_version_tag_inner() + } else { + version + } + }) + } + /// Invalidate this type's version tag and cascade to all subclasses. - pub fn modified(&self) { + fn modified_inner(&self) { if let Some(ext) = self.heaptype_ext.as_ref() { ext.specialization_cache.invalidate_for_type_modified(); } @@ -499,11 +520,15 @@ impl PyType { let subclasses = self.subclasses.read(); for weak_ref in subclasses.iter() { if let Some(sub) = weak_ref.upgrade() { - sub.downcast_ref::().unwrap().modified(); + sub.downcast_ref::().unwrap().modified_inner(); } } } + pub fn modified(&self) { + self.modified_inner(); + } + pub fn new_simple_heap( name: &str, base: &Py, @@ -892,6 +917,74 @@ impl PyType { self.find_name_in_mro(attr_name) } + /// CPython-style `_PyType_LookupRefAndVersion` equivalent for interned names. + /// Returns the observed lookup result and the type version used for the lookup. + pub(crate) fn lookup_ref_and_version_interned( + &self, + name: &'static PyStrInterned, + vm: &VirtualMachine, + ) -> (Option, u32) { + let version = self.tp_version_tag.load(Ordering::Acquire); + if version != 0 { + let idx = type_cache_hash(version, name); + let entry = &TYPE_CACHE[idx]; + let name_ptr = name as *const _ as *mut _; + loop { + let seq1 = entry.begin_read(); + let entry_version = entry.version.load(Ordering::Acquire); + let type_version = self.tp_version_tag.load(Ordering::Acquire); + if entry_version != type_version + || !core::ptr::eq(entry.name.load(Ordering::Relaxed), name_ptr) + { + break; + } + let ptr = entry.value.load(Ordering::Acquire); + if ptr.is_null() { + if entry.end_read(seq1) { + return (None, entry_version); + } + continue; + } + if let Some(cloned) = unsafe { PyObject::try_to_owned_from_ptr(ptr) } { + let same_ptr = core::ptr::eq(entry.value.load(Ordering::Relaxed), ptr); + if same_ptr && entry.end_read(seq1) { + return (Some(cloned), entry_version); + } + drop(cloned); + continue; + } + break; + } + } + + Self::with_type_lock(vm, || { + let assigned = if self.tp_version_tag.load(Ordering::Acquire) == 0 { + self.assign_version_tag_inner() + } else { + self.tp_version_tag.load(Ordering::Acquire) + }; + let result = self.find_name_in_mro_uncached(name); + if assigned != 0 + && !TYPE_CACHE_CLEARING.load(Ordering::Acquire) + && self.tp_version_tag.load(Ordering::Acquire) == assigned + { + let idx = type_cache_hash(assigned, name); + let entry = &TYPE_CACHE[idx]; + let name_ptr = name as *const _ as *mut _; + entry.begin_write(); + entry.version.store(0, Ordering::Release); + let new_ptr = result.as_ref().map_or(core::ptr::null_mut(), |found| { + &**found as *const PyObject as *mut _ + }); + entry.value.store(new_ptr, Ordering::Relaxed); + entry.name.store(name_ptr, Ordering::Relaxed); + entry.version.store(assigned, Ordering::Release); + entry.end_write(); + } + (result, assigned) + }) + } + /// Cache __init__ for CALL_ALLOC_AND_ENTER_INIT specialization. /// The cache is valid only when guarded by the type version check. pub(crate) fn cache_init_for_specialization( @@ -906,15 +999,17 @@ impl PyType { if tp_version == 0 { return false; } - if self.tp_version_tag.load(Ordering::Acquire) != tp_version { - return false; - } - let _guard = ext.specialization_cache.write_lock.lock(); - if self.tp_version_tag.load(Ordering::Acquire) != tp_version { - return false; - } - ext.specialization_cache.swap_init(Some(init), Some(vm)); - true + Self::with_type_lock(vm, || { + if self.tp_version_tag.load(Ordering::Acquire) != tp_version { + return false; + } + let _guard = ext.specialization_cache.write_lock.lock(); + if self.tp_version_tag.load(Ordering::Acquire) != tp_version { + return false; + } + ext.specialization_cache.swap_init(Some(init), Some(vm)); + true + }) } /// Read cached __init__ for CALL_ALLOC_AND_ENTER_INIT specialization. @@ -948,20 +1043,22 @@ impl PyType { if tp_version == 0 { return false; } - let _guard = ext.specialization_cache.write_lock.lock(); - if self.tp_version_tag.load(Ordering::Acquire) != tp_version { - return false; - } - let func_version = getitem.get_version_for_current_state(); - if func_version == 0 { - return false; - } - ext.specialization_cache - .getitem_version - .store(func_version, Ordering::Release); - ext.specialization_cache - .swap_getitem(Some(getitem), Some(vm)); - true + Self::with_type_lock(vm, || { + let _guard = ext.specialization_cache.write_lock.lock(); + if self.tp_version_tag.load(Ordering::Acquire) != tp_version { + return false; + } + let func_version = getitem.get_version_for_current_state(); + if func_version == 0 { + return false; + } + ext.specialization_cache + .getitem_version + .store(func_version, Ordering::Release); + ext.specialization_cache + .swap_getitem(Some(getitem), Some(vm)); + true + }) } /// Read cached __getitem__ for BINARY_OP_SUBSCR_GETITEM specialization. @@ -1327,38 +1424,41 @@ impl PyType { // // TODO: how to uniquely identify the subclasses to remove? // } - *zelf.bases.write() = bases; - // Recursively update the mros of this class and all subclasses - fn update_mro_recursively(cls: &PyType, vm: &VirtualMachine) -> PyResult<()> { - let mut mro = - PyType::resolve_mro(&cls.bases.read()).map_err(|msg| vm.new_type_error(msg))?; - // Preserve self (mro[0]) when updating MRO - mro.insert(0, cls.mro.read()[0].to_owned()); - *cls.mro.write() = mro; - for subclass in cls.subclasses.write().iter() { - let subclass = subclass.upgrade().unwrap(); - let subclass: &Py = subclass.downcast_ref().unwrap(); - update_mro_recursively(subclass, vm)?; + Self::with_type_lock(vm, || { + *zelf.bases.write() = bases; + // Recursively update the mros of this class and all subclasses + fn update_mro_recursively(cls: &PyType, vm: &VirtualMachine) -> PyResult<()> { + let mut mro = + PyType::resolve_mro(&cls.bases.read()).map_err(|msg| vm.new_type_error(msg))?; + // Preserve self (mro[0]) when updating MRO + mro.insert(0, cls.mro.read()[0].to_owned()); + *cls.mro.write() = mro; + for subclass in cls.subclasses.write().iter() { + let subclass = subclass.upgrade().unwrap(); + let subclass: &Py = subclass.downcast_ref().unwrap(); + update_mro_recursively(subclass, vm)?; + } + Ok(()) } - Ok(()) - } - update_mro_recursively(zelf, vm)?; + update_mro_recursively(zelf, vm)?; - // Invalidate inline caches - zelf.modified(); + // Invalidate inline caches + zelf.modified_inner(); - // TODO: do any old slots need to be cleaned up first? - zelf.init_slots(&vm.ctx); + // TODO: do any old slots need to be cleaned up first? + zelf.init_slots(&vm.ctx); - // Register this type as a subclass of its new bases - let weakref_type = super::PyWeak::static_type(); - for base in zelf.bases.read().iter() { - base.subclasses.write().push( - zelf.as_object() - .downgrade_with_weakref_typ_opt(None, weakref_type.to_owned()) - .unwrap(), - ); - } + // Register this type as a subclass of its new bases + let weakref_type = super::PyWeak::static_type(); + for base in zelf.bases.read().iter() { + base.subclasses.write().push( + zelf.as_object() + .downgrade_with_weakref_typ_opt(None, weakref_type.to_owned()) + .unwrap(), + ); + } + Ok(()) + })?; Ok(()) } @@ -1450,20 +1550,30 @@ impl PyType { ))); } - let mut attrs = self.attributes.write(); - // First try __annotate__, in case that's been set explicitly - if let Some(annotate) = attrs.get(identifier!(vm, __annotate__)).cloned() { + let annotate_key = identifier!(vm, __annotate__); + let annotate_func_key = identifier!(vm, __annotate_func__); + let attrs = self.attributes.read(); + if let Some(annotate) = attrs.get(annotate_key).cloned() { return Ok(annotate); } - // Then try __annotate_func__ - if let Some(annotate) = attrs.get(identifier!(vm, __annotate_func__)).cloned() { - // TODO: Apply descriptor tp_descr_get if needed + if let Some(annotate) = attrs.get(annotate_func_key).cloned() { return Ok(annotate); } - // Set __annotate_func__ = None and return None + drop(attrs); + let none = vm.ctx.none(); - attrs.insert(identifier!(vm, __annotate_func__), none.clone()); - Ok(none) + Ok(Self::with_type_lock(vm, || { + let mut attrs = self.attributes.write(); + if let Some(annotate) = attrs.get(annotate_key).cloned() { + return annotate; + } + if let Some(annotate) = attrs.get(annotate_func_key).cloned() { + return annotate; + } + self.modified_inner(); + attrs.insert(annotate_func_key, none.clone()); + none + })) } #[pygetset(setter)] @@ -1486,20 +1596,24 @@ impl PyType { return Err(vm.new_type_error("__annotate__ must be callable or None")); } - let mut attrs = self.attributes.write(); - // Clear cached annotations only when setting to a new callable - if !vm.is_none(&value) { - attrs.swap_remove(identifier!(vm, __annotations_cache__)); - } - attrs.insert(identifier!(vm, __annotate_func__), value.clone()); + 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); + }); Ok(()) } #[pygetset] fn __annotations__(&self, vm: &VirtualMachine) -> PyResult { + let annotations_key = identifier!(vm, __annotations__); + let annotations_cache_key = identifier!(vm, __annotations_cache__); let attrs = self.attributes.read(); - if let Some(annotations) = attrs.get(identifier!(vm, __annotations__)).cloned() { + if let Some(annotations) = attrs.get(annotations_key).cloned() { // Ignore the __annotations__ descriptor stored on type itself. if !annotations.class().is(vm.ctx.types.getset_type) { if vm.is_none(&annotations) @@ -1514,8 +1628,7 @@ impl PyType { ))); } } - // Then try __annotations_cache__ - if let Some(annotations) = attrs.get(identifier!(vm, __annotations_cache__)).cloned() { + if let Some(annotations) = attrs.get(annotations_cache_key).cloned() { if vm.is_none(&annotations) || annotations.class().is(vm.ctx.types.dict_type) || self.slots.flags.has_feature(PyTypeFlags::HEAPTYPE) @@ -1552,11 +1665,20 @@ impl PyType { vm.ctx.new_dict().into() }; - // Cache the result in __annotations_cache__ - self.attributes - .write() - .insert(identifier!(vm, __annotations_cache__), annotations.clone()); - Ok(annotations) + Ok(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; + } + if let Some(existing) = attrs.get(annotations_cache_key).cloned() { + return existing; + } + self.modified_inner(); + attrs.insert(annotations_cache_key, annotations.clone()); + annotations + })) } #[pygetset(setter)] @@ -1572,43 +1694,45 @@ impl PyType { ))); } - let mut attrs = self.attributes.write(); - let has_annotations = attrs.contains_key(identifier!(vm, __annotations__)); - - match value { - crate::function::PySetterValue::Assign(value) => { - // SET path: store the value (including None) - let key = if has_annotations { - identifier!(vm, __annotations__) - } else { - identifier!(vm, __annotations_cache__) - }; - attrs.insert(key, value); - if has_annotations { - attrs.swap_remove(identifier!(vm, __annotations_cache__)); - } - } - crate::function::PySetterValue::Delete => { - // DELETE path: remove the key - let removed = if has_annotations { - attrs - .swap_remove(identifier!(vm, __annotations__)) - .is_some() - } else { - attrs - .swap_remove(identifier!(vm, __annotations_cache__)) - .is_some() - }; - if !removed { - return Err(vm.new_attribute_error("__annotations__")); + Self::with_type_lock(vm, || { + self.modified_inner(); + let mut attrs = self.attributes.write(); + let has_annotations = attrs.contains_key(identifier!(vm, __annotations__)); + + match value { + crate::function::PySetterValue::Assign(value) => { + let key = if has_annotations { + identifier!(vm, __annotations__) + } else { + identifier!(vm, __annotations_cache__) + }; + attrs.insert(key, value); + if has_annotations { + attrs.swap_remove(identifier!(vm, __annotations_cache__)); + } } - if has_annotations { - attrs.swap_remove(identifier!(vm, __annotations_cache__)); + crate::function::PySetterValue::Delete => { + let removed = if has_annotations { + attrs + .swap_remove(identifier!(vm, __annotations__)) + .is_some() + } else { + attrs + .swap_remove(identifier!(vm, __annotations_cache__)) + .is_some() + }; + if !removed { + return Err(vm.new_attribute_error("__annotations__")); + } + if has_annotations { + attrs.swap_remove(identifier!(vm, __annotations_cache__)); + } } } - } - attrs.swap_remove(identifier!(vm, __annotate_func__)); - attrs.swap_remove(identifier!(vm, __annotate__)); + attrs.swap_remove(identifier!(vm, __annotate_func__)); + attrs.swap_remove(identifier!(vm, __annotate__)); + Ok(()) + })?; Ok(()) } @@ -1641,9 +1765,12 @@ impl PyType { #[pygetset(setter)] fn set___module__(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { self.check_set_special_type_attr(identifier!(vm, __module__), vm)?; - let mut attributes = self.attributes.write(); - attributes.swap_remove(identifier!(vm, __firstlineno__)); - attributes.insert(identifier!(vm, __module__), value); + Self::with_type_lock(vm, || { + self.modified_inner(); + self.attributes + .write() + .insert(identifier!(vm, __module__), value); + }); Ok(()) } @@ -1765,24 +1892,26 @@ impl PyType { value: PySetterValue, vm: &VirtualMachine, ) -> PyResult<()> { + let key = identifier!(vm, __type_params__); match value { - PySetterValue::Assign(ref val) => { - let key = identifier!(vm, __type_params__); + PySetterValue::Assign(val) => { self.check_set_special_type_attr(key, vm)?; - self.modified(); - self.attributes.write().insert(key, val.clone().into()); + Self::with_type_lock(vm, || { + self.modified_inner(); + self.attributes.write().insert(key, val.into()); + }); } PySetterValue::Delete => { - // For delete, we still need to check if the type is immutable if self.slots.flags.has_feature(PyTypeFlags::IMMUTABLETYPE) { return Err(vm.new_type_error(format!( "cannot delete '__type_params__' attribute of immutable type '{}'", self.slot_name() ))); } - let key = identifier!(vm, __type_params__); - self.modified(); - self.attributes.write().shift_remove(&key); + Self::with_type_lock(vm, || { + self.modified_inner(); + self.attributes.write().shift_remove(&key); + }); } } Ok(()) @@ -2390,10 +2519,12 @@ impl Py { // Check if we can set this special type attribute self.check_set_special_type_attr(identifier!(vm, __doc__), vm)?; - // Set the __doc__ in the type's dict - self.attributes - .write() - .insert(identifier!(vm, __doc__), value); + PyType::with_type_lock(vm, || { + self.modified_inner(); + self.attributes + .write() + .insert(identifier!(vm, __doc__), value); + }); Ok(()) } @@ -2455,23 +2586,26 @@ impl SetAttr for PyType { } let assign = value.is_assign(); - // 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(); + 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); - } else { - let prev_value = zelf.attributes.write().shift_remove(attr_name); // TODO: swap_remove applicable? - if prev_value.is_none() { - return Err(vm.new_attribute_error(format!( - "type object '{}' has no attribute '{}'", - zelf.name(), - attr_name, - ))); + if let PySetterValue::Assign(value) = value { + 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() { + return Err(vm.new_attribute_error(format!( + "type object '{}' has no attribute '{}'", + zelf.name(), + attr_name, + ))); + } } - } + Ok(()) + })?; if attr_name.as_wtf8().starts_with("__") && attr_name.as_wtf8().ends_with("__") { if assign { diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 59f496e2423..08dc3fe35f6 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -7316,10 +7316,7 @@ impl ExecutingFrame<'_> { .load() .is_some_and(|f| f as usize == PyBaseObject::getattro as *const () as usize); if !is_default_getattro { - let mut type_version = cls.tp_version_tag.load(Acquire); - if type_version == 0 { - type_version = cls.assign_version_tag(); - } + let type_version = cls.version_for_specialization(_vm); if type_version != 0 && !oparg.is_method() && !self.specialization_eval_frame_active(_vm) @@ -7357,10 +7354,7 @@ impl ExecutingFrame<'_> { } // Get or assign type version - let mut type_version = cls.tp_version_tag.load(Acquire); - if type_version == 0 { - type_version = cls.assign_version_tag(); - } + let type_version = cls.version_for_specialization(_vm); if type_version == 0 { // Version counter overflow — backoff to avoid re-attempting every execution unsafe { @@ -7580,10 +7574,7 @@ impl ExecutingFrame<'_> { let owner_type = obj.downcast_ref::().unwrap(); // Get or assign type version for the type object itself - let mut type_version = owner_type.tp_version_tag.load(Acquire); - if type_version == 0 { - type_version = owner_type.assign_version_tag(); - } + let type_version = owner_type.version_for_specialization(_vm); if type_version == 0 { unsafe { self.code.instructions.write_adaptive_counter( @@ -7618,10 +7609,7 @@ impl ExecutingFrame<'_> { } let mut metaclass_version = 0; if !mcl.slots.flags.has_feature(PyTypeFlags::IMMUTABLETYPE) { - metaclass_version = mcl.tp_version_tag.load(Acquire); - if metaclass_version == 0 { - metaclass_version = mcl.assign_version_tag(); - } + metaclass_version = mcl.version_for_specialization(_vm); if metaclass_version == 0 { unsafe { self.code.instructions.write_adaptive_counter( @@ -7808,16 +7796,14 @@ impl ExecutingFrame<'_> { Some(Instruction::BinaryOpSubscrListSlice) } else { let cls = a.class(); + let (getitem, type_version) = + cls.lookup_ref_and_version_interned(identifier!(vm, __getitem__), vm); if cls.slots.flags.has_feature(PyTypeFlags::HEAPTYPE) && !self.specialization_eval_frame_active(vm) - && let Some(_getitem) = cls.get_attr(identifier!(vm, __getitem__)) + && let Some(_getitem) = getitem && let Some(func) = _getitem.downcast_ref_if_exact::(vm) && func.can_specialize_call(2) { - let mut type_version = cls.tp_version_tag.load(Acquire); - if type_version == 0 { - type_version = cls.assign_version_tag(); - } if type_version != 0 { if cls.cache_getitem_for_specialization( func.to_owned(), @@ -8356,11 +8342,8 @@ impl ExecutingFrame<'_> { && cls_new_fn as usize == obj_new_fn as usize && cls_alloc_fn as usize == obj_alloc_fn as usize { - let init = cls.get_attr(identifier!(vm, __init__)); - let mut version = cls.tp_version_tag.load(Acquire); - if version == 0 { - version = cls.assign_version_tag(); - } + let (init, version) = + cls.lookup_ref_and_version_interned(identifier!(vm, __init__), vm); if version == 0 { unsafe { self.code.instructions.write_adaptive_counter( @@ -8680,10 +8663,7 @@ impl ExecutingFrame<'_> { && cls.slots.as_sequence.length.load().is_none() { // Cache type version for ToBoolAlwaysTrue guard - let mut type_version = cls.tp_version_tag.load(Acquire); - if type_version == 0 { - type_version = cls.assign_version_tag(); - } + let type_version = cls.version_for_specialization(vm); if type_version != 0 { unsafe { self.code @@ -9021,10 +9001,7 @@ impl ExecutingFrame<'_> { } // Get or assign type version - let mut type_version = cls.tp_version_tag.load(Acquire); - if type_version == 0 { - type_version = cls.assign_version_tag(); - } + let type_version = cls.version_for_specialization(vm); if type_version == 0 { unsafe { self.code.instructions.write_adaptive_counter( diff --git a/crates/vm/src/stdlib/_ctypes/base.rs b/crates/vm/src/stdlib/_ctypes/base.rs index 0bfbe57bb04..c5714e7cc2f 100644 --- a/crates/vm/src/stdlib/_ctypes/base.rs +++ b/crates/vm/src/stdlib/_ctypes/base.rs @@ -2554,10 +2554,11 @@ fn make_fields( } let new_descr = super::PyCField::new_from_field(fdescr, index, offset); - cls.set_attr( + cls.as_object().set_attr( vm.ctx.intern_str(fname.as_wtf8()), new_descr.to_pyobject(vm), - ); + vm, + )?; } Ok(()) @@ -2596,10 +2597,11 @@ pub(super) fn make_anon_fields(cls: &Py, vm: &VirtualMachine) -> PyResul let mut new_descr = super::PyCField::new_from_field(descr, 0, 0); new_descr.set_anonymous(true); - cls.set_attr( + cls.as_object().set_attr( vm.ctx.intern_str(fname.as_wtf8()), new_descr.to_pyobject(vm), - ); + vm, + )?; make_fields(cls, descr, descr.index, descr.offset, vm)?; } diff --git a/crates/vm/src/stdlib/_ctypes/structure.rs b/crates/vm/src/stdlib/_ctypes/structure.rs index c9ab9205601..0b310db0745 100644 --- a/crates/vm/src/stdlib/_ctypes/structure.rs +++ b/crates/vm/src/stdlib/_ctypes/structure.rs @@ -498,7 +498,11 @@ impl PyCStructType { }; // Set the CField as a class attribute - cls.set_attr(vm.ctx.intern_str(name.clone()), c_field.to_pyobject(vm)); + cls.as_object().set_attr( + vm.ctx.intern_str(name.clone()), + c_field.to_pyobject(vm), + vm, + )?; // Update tracking - don't advance offset for packed bitfields if field_advances_offset { diff --git a/crates/vm/src/stdlib/_ctypes/union.rs b/crates/vm/src/stdlib/_ctypes/union.rs index c1882141e98..10ef3ba578a 100644 --- a/crates/vm/src/stdlib/_ctypes/union.rs +++ b/crates/vm/src/stdlib/_ctypes/union.rs @@ -312,7 +312,8 @@ impl PyCUnionType { PyCField::new(name.clone(), field_type_ref, 0, size as isize, index) }; - cls.set_attr(vm.ctx.intern_str(name), c_field.to_pyobject(vm)); + cls.as_object() + .set_attr(vm.ctx.intern_str(name), c_field.to_pyobject(vm), vm)?; } // Calculate total_align and aligned_size diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index 8cde18a47ba..cc5b9d4760f 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -840,6 +840,7 @@ pub mod module { reinit_mutex_after_fork(&vm.state.atexit_funcs); reinit_mutex_after_fork(&vm.state.global_trace_func); reinit_mutex_after_fork(&vm.state.global_profile_func); + reinit_mutex_after_fork(&vm.state.type_mutex); reinit_mutex_after_fork(&vm.state.monitoring); // PyGlobalState parking_lot::Mutex locks diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 5bf7436e958..a6754d40204 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -115,6 +115,7 @@ where switch_interval: AtomicCell::new(0.005), global_trace_func: PyMutex::default(), global_profile_func: PyMutex::default(), + type_mutex: PyMutex::default(), #[cfg(feature = "threading")] main_thread_ident: AtomicCell::new(0), #[cfg(feature = "threading")] diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 05210eb09d7..4da4587dd16 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -599,6 +599,8 @@ pub struct PyGlobalState { pub global_trace_func: PyMutex>, /// Global profile function for all threads (set by sys._setprofileallthreads) pub global_profile_func: PyMutex>, + /// Global type mutation/versioning mutex for CPython-style FT type operations. + pub type_mutex: PyMutex<()>, /// Main thread identifier (pthread_self on Unix) #[cfg(feature = "threading")] pub main_thread_ident: AtomicCell, From 18657330c9dbe7b4738378d85e2b06993be262f9 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Tue, 17 Mar 2026 21:00:02 +0900 Subject: [PATCH 10/10] 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. --- .github/workflows/cron-ci.yaml | 18 ++--- Lib/test/test_class.py | 2 + crates/vm/src/builtins/type.rs | 95 ++++++++++++----------- crates/vm/src/stdlib/_ctypes/base.rs | 10 +-- crates/vm/src/stdlib/_ctypes/structure.rs | 6 +- crates/vm/src/stdlib/_ctypes/union.rs | 3 +- extra_tests/custom_text_test_runner.py | 54 ++++++------- 7 files changed, 95 insertions(+), 93 deletions(-) diff --git a/.github/workflows/cron-ci.yaml b/.github/workflows/cron-ci.yaml index 68ef4dea47a..a481e71de06 100644 --- a/.github/workflows/cron-ci.yaml +++ b/.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 diff --git a/Lib/test/test_class.py b/Lib/test/test_class.py index 7420f289b16..ea8653cd0d9 100644 --- a/Lib/test/test_class.py +++ b/Lib/test/test_class.py @@ -1,5 +1,6 @@ "Test the functionality of Python classes implementing operators." +import sys import unittest from test import support from test.support import cpython_only, import_helper, script_helper @@ -614,6 +615,7 @@ def assertNotOrderable(self, a, b): with self.assertRaises(TypeError): a >= b + @unittest.skipIf(sys.platform == "win32", "TODO: RUSTPYTHON; flaky on Windows") def testHashComparisonOfMethods(self): # Test comparison and hash of methods class A: diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 3148ff420f9..dd937f98fc0 100644 --- a/crates/vm/src/builtins/type.rs +++ b/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,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 { // 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("__") { diff --git a/crates/vm/src/stdlib/_ctypes/base.rs b/crates/vm/src/stdlib/_ctypes/base.rs index c5714e7cc2f..0bfbe57bb04 100644 --- a/crates/vm/src/stdlib/_ctypes/base.rs +++ b/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, 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)?; } diff --git a/crates/vm/src/stdlib/_ctypes/structure.rs b/crates/vm/src/stdlib/_ctypes/structure.rs index 0b310db0745..c9ab9205601 100644 --- a/crates/vm/src/stdlib/_ctypes/structure.rs +++ b/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 { diff --git a/crates/vm/src/stdlib/_ctypes/union.rs b/crates/vm/src/stdlib/_ctypes/union.rs index 10ef3ba578a..c1882141e98 100644 --- a/crates/vm/src/stdlib/_ctypes/union.rs +++ b/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 diff --git a/extra_tests/custom_text_test_runner.py b/extra_tests/custom_text_test_runner.py index afec493a66c..3457bdfd0e4 100644 --- a/extra_tests/custom_text_test_runner.py +++ b/extra_tests/custom_text_test_runner.py @@ -38,6 +38,16 @@ from unittest.runner import registerResult, result +def _get_method_dict(test): + """Get the __dict__ of the underlying function for a test method. + + Works for both bound methods (__func__.__dict__) and plain functions. + """ + method = getattr(test, test._testMethodName) + func = getattr(method, "__func__", method) + return func.__dict__ + + class TablePrinter(object): # Modified from https://github.com/agramian/table-printer, same license as above "Print a list of dicts as a table" @@ -325,9 +335,7 @@ def startTest(self, test): self.stream.writeln("TEST SUITE: %s" % self.suite) self.stream.writeln("Description: %s" % self.getSuiteDescription(test)) try: - name_override = getattr(test, test._testMethodName).__func__.__dict__[ - "test_case_name" - ] + name_override = _get_method_dict(test)["test_case_name"] except: name_override = None self.case = name_override if name_override else self.case @@ -345,7 +353,11 @@ def startTest(self, test): self.results["suites"][self.suite_number] = { "name": self.suite, "class": test.__class__.__name__, - "module": re.compile(".* \((.*)\)").match(str(test)).group(1), + "module": ( + m.group(1) + if (m := re.compile(r".* \((.*)\)").match(str(test))) + else str(test) + ), "description": self.getSuiteDescription(test), "cases": {}, "used_case_names": {}, @@ -380,34 +392,22 @@ def startTest(self, test): if "test_type" in getattr( test, test._testMethodName ).__func__.__dict__ and set([s.lower() for s in self.test_types]) == set( - [ - s.lower() - for s in getattr(test, test._testMethodName).__func__.__dict__[ - "test_type" - ] - ] + [s.lower() for s in _get_method_dict(test)["test_type"]] ): pass else: - getattr(test, test._testMethodName).__func__.__dict__[ - "__unittest_skip_why__" - ] = 'Test run specified to only run tests of type "%s"' % ",".join( - self.test_types + _get_method_dict(test)["__unittest_skip_why__"] = ( + 'Test run specified to only run tests of type "%s"' + % ",".join(self.test_types) ) - getattr(test, test._testMethodName).__func__.__dict__[ - "__unittest_skip__" - ] = True - if "skip_device" in getattr(test, test._testMethodName).__func__.__dict__: - for device in getattr(test, test._testMethodName).__func__.__dict__[ - "skip_device" - ]: + _get_method_dict(test)["__unittest_skip__"] = True + if "skip_device" in _get_method_dict(test): + for device in _get_method_dict(test)["skip_device"]: if self.config and device.lower() in self.config["device_name"].lower(): - getattr(test, test._testMethodName).__func__.__dict__[ - "__unittest_skip_why__" - ] = "Test is marked to be skipped on %s" % device - getattr(test, test._testMethodName).__func__.__dict__[ - "__unittest_skip__" - ] = True + _get_method_dict(test)["__unittest_skip_why__"] = ( + "Test is marked to be skipped on %s" % device + ) + _get_method_dict(test)["__unittest_skip__"] = True break def stopTest(self, test):