diff --git a/.cspell.json b/.cspell.json index f6887a7bb35..227ab6e4fdf 100644 --- a/.cspell.json +++ b/.cspell.json @@ -61,6 +61,9 @@ "dedents", "deduped", "deoptimize", + "downcastable", + "downcasted", + "dumpable", "emscripten", "excs", "interps", 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 0f63e7106df..dd937f98fc0 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); } @@ -457,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; @@ -467,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; } } @@ -487,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(); } @@ -505,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, @@ -898,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( @@ -912,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. @@ -954,26 +1043,27 @@ 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 - .swap_getitem(Some(getitem), Some(vm)); - ext.specialization_cache - .getitem_version - .store(func_version, Ordering::Relaxed); - 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. 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 +1071,7 @@ impl PyType { let cached_version = ext .specialization_cache .getitem_version - .load(Ordering::Relaxed); + .load(Ordering::Acquire); if cached_version == 0 { return None; } @@ -1334,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(()) } @@ -1457,20 +1550,31 @@ 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) + 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, None); + } + if let Some(annotate) = attrs.get(annotate_func_key).cloned() { + return (annotate, None); + } + self.modified_inner(); + let prev = attrs.insert(annotate_func_key, none.clone()); + (none, prev) + }); + Ok(result) } #[pygetset(setter)] @@ -1493,20 +1597,27 @@ 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()); + let _prev_values = Self::with_type_lock(vm, || { + self.modified_inner(); + let mut attrs = self.attributes.write(); + 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(()) } #[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) @@ -1521,8 +1632,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) @@ -1559,11 +1669,21 @@ 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) + 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, None); + } + if let Some(existing) = attrs.get(annotations_cache_key).cloned() { + return (existing, None); + } + self.modified_inner(); + let prev = attrs.insert(annotations_cache_key, annotations.clone()); + (annotations, prev) + }); + Ok(result) } #[pygetset(setter)] @@ -1579,43 +1699,43 @@ 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__")); + 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__) + }; + prev.extend(attrs.insert(key, value)); + if has_annotations { + prev.extend(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__)) + } else { + attrs.swap_remove(identifier!(vm, __annotations_cache__)) + }; + if removed.is_none() { + return Err(vm.new_attribute_error("__annotations__")); + } + prev.extend(removed); + if has_annotations { + prev.extend(attrs.swap_remove(identifier!(vm, __annotations_cache__))); + } } } - } - attrs.swap_remove(identifier!(vm, __annotate_func__)); - attrs.swap_remove(identifier!(vm, __annotate__)); + prev.extend(attrs.swap_remove(identifier!(vm, __annotate_func__))); + prev.extend(attrs.swap_remove(identifier!(vm, __annotate__))); + Ok(prev) + })?; Ok(()) } @@ -1648,9 +1768,13 @@ 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); + let _prev_values = Self::with_type_lock(vm, || { + self.modified_inner(); + 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(()) } @@ -1772,24 +1896,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()); + let _prev_value = 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); + let _prev_value = Self::with_type_lock(vm, || { + self.modified_inner(); + self.attributes.write().shift_remove(&key) + }); } } Ok(()) @@ -2397,10 +2523,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); + let _prev_value = PyType::with_type_lock(vm, || { + self.modified_inner(); + self.attributes + .write() + .insert(identifier!(vm, __doc__), value) + }); Ok(()) } @@ -2462,23 +2590,29 @@ 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(); - - 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, - ))); + // 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 { + 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() { + return Err(vm.new_attribute_error(format!( + "type object '{}' has no attribute '{}'", + zelf.name(), + attr_name, + ))); + } + Ok(prev_value) } - } + })?; 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 d8e3d35f9e3..08dc3fe35f6 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()?; @@ -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() { @@ -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"); @@ -4787,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); } @@ -5191,8 +5187,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)); @@ -7320,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) @@ -7361,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 { @@ -7584,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( @@ -7622,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( @@ -7797,7 +7781,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 { @@ -7812,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(), @@ -8360,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( @@ -8620,8 +8599,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 { @@ -8684,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 @@ -8919,7 +8895,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 @@ -9025,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/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, 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):