vm: finalize specialization GC safety, tests, and cleanup

RustPython · youknowone · Mar 8, 2026 · Mar 8, 2026 · Mar 8, 2026 · Mar 8, 2026
commit e65cbc0d58146156651440658cf5b1205ac361c3
diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs
@@ -640,7 +640,7 @@ impl Py<PyFunction> {
         new_v
     }
 
-    /// CPython function_kind(SIMPLE_FUNCTION) equivalent for CALL specialization.
+    /// function_kind(SIMPLE_FUNCTION) equivalent for CALL specialization.
     /// Returns true if: CO_OPTIMIZED, no VARARGS, no VARKEYWORDS, no kwonly args.
     pub(crate) fn is_simple_for_call_specialization(&self) -> bool {
         let code: &Py<PyCode> = &self.code;
@@ -705,8 +705,8 @@ impl Py<PyFunction> {
         );
         debug_assert_eq!(code.kwonlyarg_count, 0);
 
-        // Generator/coroutine code objects are SIMPLE_FUNCTION in CPython's
-        // call specialization classification, but their call path must still
+        // Generator/coroutine code objects are SIMPLE_FUNCTION in call
+        // specialization classification, but their call path must still
         // go through invoke() to produce generator/coroutine objects.
         if code
             .flags

diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs
@@ -235,6 +235,7 @@ unsafe impl crate::object::Traverse for PyType {
             .count();
         if let Some(ext) = self.heaptype_ext.as_ref() {
             ext.specialization_init.read().traverse(tracer_fn);
+            ext.specialization_getitem.read().traverse(tracer_fn);
         }
     }
 
@@ -263,11 +264,19 @@ unsafe impl crate::object::Traverse for PyType {
                 out.push(val);
             }
         }
-        if let Some(ext) = self.heaptype_ext.as_ref()
-            && let Some(mut guard) = ext.specialization_init.try_write()
-            && let Some(init) = guard.take()
-        {
-            out.push(init.into());
+        if let Some(ext) = self.heaptype_ext.as_ref() {
+            if let Some(mut guard) = ext.specialization_init.try_write()
+                && let Some(init) = guard.take()
+            {
+                out.push(init.into());
+            }
+            if let Some(mut guard) = ext.specialization_getitem.try_write()
+                && let Some(getitem) = guard.take()
+            {
+                out.push(getitem.into());
+                ext.specialization_getitem_version
+                    .store(0, Ordering::Release);
+            }
         }
     }
 }
@@ -860,14 +869,18 @@ impl PyType {
         let Some(ext) = self.heaptype_ext.as_ref() else {
             return false;
         };
-        if tp_version == 0 || self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+        if tp_version == 0 {
             return false;
         }
         let func_version = getitem.get_version_for_current_state();
         if func_version == 0 {
             return false;
         }
-        *ext.specialization_getitem.write() = Some(getitem);
+        let mut guard = ext.specialization_getitem.write();
+        if self.tp_version_tag.load(Ordering::Acquire) != tp_version {
+            return false;
+        }
+        *guard = Some(getitem);
         ext.specialization_getitem_version
             .store(func_version, Ordering::Release);
         true
@@ -876,15 +889,15 @@ impl PyType {
     /// Read cached __getitem__ for BINARY_OP_SUBSCR_GETITEM specialization.
     pub(crate) fn get_cached_getitem_for_specialization(&self) -> Option<(PyRef<PyFunction>, u32)> {
         let ext = self.heaptype_ext.as_ref()?;
-        if self.tp_version_tag.load(Ordering::Acquire) == 0 {
-            return None;
-        }
         let cached_version = ext.specialization_getitem_version.load(Ordering::Acquire);
         if cached_version == 0 {
             return None;
         }
-        ext.specialization_getitem
-            .read()
+        let guard = ext.specialization_getitem.read();
+        if self.tp_version_tag.load(Ordering::Acquire) == 0 {
+            return None;
+        }
+        guard
             .as_ref()
             .map(|getitem| (getitem.to_owned(), cached_version))
     }
@@ -2326,7 +2339,7 @@ impl Py<PyType> {
 
     #[pymethod]
     fn __instancecheck__(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
-        // Use real_is_instance to avoid infinite recursion, matching CPython's behavior
+        // Use real_is_instance to avoid infinite recursion
         obj.real_is_instance(self.as_object(), vm)
     }
 

diff --git a/crates/vm/src/builtins/union.rs b/crates/vm/src/builtins/union.rs
@@ -51,7 +51,7 @@ impl PyUnion {
         })
     }
 
-    /// Direct access to args field, matching CPython's _Py_union_args
+    /// Direct access to args field (_Py_union_args)
     #[inline]
     pub fn args(&self) -> &Py<PyTuple> {
         &self.args
@@ -292,8 +292,8 @@ fn dedup_and_flatten_args(args: &Py<PyTuple>, vm: &VirtualMachine) -> PyResult<U
     // - For hashable elements: use Python's set semantics (hash + equality)
     // - For unhashable elements: use equality comparison
     //
-    // This avoids calling __eq__ when hashes differ, matching CPython behavior
-    // where `int | BadType` doesn't raise even if BadType.__eq__ raises.
+    // This avoids calling __eq__ when hashes differ, so `int | BadType`
+    // doesn't raise even if BadType.__eq__ raises.
 
     let mut new_args: Vec<PyObjectRef> = Vec::with_capacity(args.len());
 

diff --git a/crates/vm/src/coroutine.rs b/crates/vm/src/coroutine.rs
@@ -189,7 +189,7 @@ impl Coro {
         exc_tb: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<PyIterReturn> {
-        // Validate throw arguments (matching CPython _gen_throw)
+        // Validate throw arguments (_gen_throw)
         if exc_type.fast_isinstance(vm.ctx.exceptions.base_exception_type) && !vm.is_none(&exc_val)
         {
             return Err(vm.new_type_error("instance exception may not have a separate value"));

diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs
@@ -1670,7 +1670,7 @@ impl ExecutingFrame<'_> {
                 self.execute_bin_op(vm, op_val)
             }
             // Super-instruction for BINARY_OP_ADD_UNICODE + STORE_FAST targeting
-            // the left local, mirroring CPython's BINARY_OP_INPLACE_ADD_UNICODE shape.
+            // the left local, matching BINARY_OP_INPLACE_ADD_UNICODE shape.
             Instruction::BinaryOpInplaceAddUnicode => {
                 let b = self.top_value();
                 let a = self.nth_value(1);
@@ -4631,8 +4631,8 @@ impl ExecutingFrame<'_> {
                     && let Some(init_func) = cls.get_cached_init_for_specialization(cached_version)
                     && let Some(cls_alloc) = cls.slots.alloc.load()
                 {
-                    // CPython guards with code->co_framesize + _Py_InitCleanup.co_framesize.
-                    // RustPython does not materialize frame-specials on datastack, so use
+                    // co_framesize + _Py_InitCleanup.co_framesize guard.
+                    // We do not materialize frame-specials on datastack, so use
                     // only the cleanup shim's eval-stack payload (2 stack slots).
                     const INIT_CLEANUP_STACK_BYTES: usize = 2 * core::mem::size_of::<usize>();
                     if !self.specialization_has_datastack_space_for_func_with_extra(
@@ -5392,8 +5392,7 @@ impl ExecutingFrame<'_> {
             Instruction::LoadGlobalModule => {
                 let oparg = u32::from(arg);
                 let cache_base = self.lasti() as usize;
-                // Keep specialized opcode on guard miss, matching CPython's
-                // JUMP_TO_PREDICTED(LOAD_GLOBAL) behavior.
+                // Keep specialized opcode on guard miss (JUMP_TO_PREDICTED behavior).
                 let cached_version = self.code.instructions.read_cache_u16(cache_base + 1);
                 let cached_index = self.code.instructions.read_cache_u16(cache_base + 3);
                 if let Ok(current_version) = u16::try_from(self.globals.version())
@@ -7271,7 +7270,7 @@ impl ExecutingFrame<'_> {
                     && func.can_specialize_call(1)
                     && !self.specialization_eval_frame_active(_vm)
                 {
-                    // Property specialization caches fget directly, matching CPython.
+                    // Property specialization caches fget directly.
                     let fget_ptr = &*fget as *const PyObject as usize;
                     unsafe {
                         self.write_cached_descriptor(cache_base, type_version, fget_ptr);
@@ -8630,7 +8629,7 @@ impl ExecutingFrame<'_> {
 
     #[inline]
     fn specialization_compact_int_value(i: &PyInt, vm: &VirtualMachine) -> Option<isize> {
-        // CPython's _PyLong_IsCompact() means a one-digit PyLong (base 2^30),
+        // _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;
         let v = i.try_to_primitive::<i64>(vm).ok()?;
@@ -8643,7 +8642,7 @@ impl ExecutingFrame<'_> {
 
     #[inline]
     fn specialization_nonnegative_compact_index(i: &PyInt, vm: &VirtualMachine) -> Option<usize> {
-        // CPython's _PyLong_IsNonNegativeCompact() uses a single base-2^30 digit.
+        // _PyLong_IsNonNegativeCompact(): a single base-2^30 digit.
         const CPYTHON_COMPACT_LONG_MAX: u64 = (1u64 << 30) - 1;
         let v = i.try_to_primitive::<u64>(vm).ok()?;
         if v <= CPYTHON_COMPACT_LONG_MAX {

diff --git a/crates/vm/src/function/method.rs b/crates/vm/src/function/method.rs
@@ -214,7 +214,7 @@ impl PyMethodDef {
         class: &'static Py<PyType>,
     ) -> PyRef<PyNativeMethod> {
         debug_assert!(self.flags.contains(PyMethodFlags::STATIC));
-        // Set zelf to the class, matching CPython's m_self = type for static methods.
+        // Set zelf to the class (m_self = type for static methods).
         // Callable::call skips prepending when STATIC flag is set.
         let func = PyNativeFunction {
             zelf: Some(class.to_owned().into()),

diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs
@@ -1674,10 +1674,9 @@ impl PyObject {
         }
 
         // 2. Clear dict and member slots (subtype_clear)
-        // Use mutable access to actually detach the dict, matching CPython's
-        // Py_CLEAR(*_PyObject_GetDictPtr(self)) which NULLs the dict pointer
-        // without clearing dict contents. This is critical because the dict
-        // may still be referenced by other live objects (e.g. function.__globals__).
+        // Detach the dict via Py_CLEAR(*_PyObject_GetDictPtr(self)) — NULL
+        // the pointer without clearing dict contents. The dict may still be
+        // referenced by other live objects (e.g. function.__globals__).
         if obj.0.has_ext() {
             let self_addr = (ptr as *const u8).addr();
             let ext_ptr = core::ptr::with_exposed_provenance_mut::<ObjExt>(

diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs
@@ -181,7 +181,7 @@ mod builtins {
 
     /// Decode source bytes to a string, handling PEP 263 encoding declarations
     /// and BOM. Raises SyntaxError for invalid UTF-8 without an encoding
-    /// declaration (matching CPython behavior).
+    /// declaration.
     /// Check if an encoding name is a UTF-8 variant after normalization.
     /// Matches: utf-8, utf_8, utf8, UTF-8, etc.
     #[cfg(feature = "parser")]

diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs
@@ -873,7 +873,7 @@ pub mod module {
     }
 
     /// Best-effort number of OS threads in this process.
-    /// Returns <= 0 when unavailable, mirroring CPython fallback behavior.
+    /// Returns <= 0 when unavailable.
     fn get_number_of_os_threads() -> isize {
         #[cfg(target_os = "macos")]
         {

diff --git a/crates/vm/src/stdlib/sys/monitoring.rs b/crates/vm/src/stdlib/sys/monitoring.rs
@@ -777,7 +777,7 @@ fn fire(
                 // Non-local events (RAISE, EXCEPTION_HANDLED, PY_UNWIND, etc.)
                 // cannot be disabled per code object.
                 if event_id >= LOCAL_EVENTS_COUNT {
-                    // Remove the callback, matching CPython behavior.
+                    // Remove the callback.
                     let mut state = vm.state.monitoring.lock();
                     state.callbacks.remove(&(tool, event_id));
                     return Err(vm.new_value_error(format!(

diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs
@@ -253,8 +253,7 @@ impl StopTheWorldState {
     }
 
     /// Try to CAS detached threads directly to SUSPENDED and check whether
-    /// stop countdown reached zero after parking detached threads
-    /// (`park_detached_threads`), matching CPython behavior class.
+    /// stop countdown reached zero after parking detached threads.
     fn park_detached_threads(&self, vm: &VirtualMachine) -> bool {
         use thread::{THREAD_ATTACHED, THREAD_DETACHED, THREAD_SUSPENDED};
         let requester = self.requester.load(Ordering::Relaxed);

diff --git a/extra_tests/snippets/specialization_binary_op_inplace_add_unicode.py b/extra_tests/snippets/specialization_binary_op_inplace_add_unicode.py
diff --git a/extra_tests/snippets/specialization_binary_op_subscr_str_int_ascii_cache.py b/extra_tests/snippets/specialization_binary_op_subscr_str_int_ascii_cache.py
diff --git a/extra_tests/snippets/specialization_binary_op_subscr_str_int_latin1_singleton.py b/extra_tests/snippets/specialization_binary_op_subscr_str_int_latin1_singleton.py
diff --git a/extra_tests/snippets/vm_specialization.py b/extra_tests/snippets/vm_specialization.py
@@ -0,0 +1,68 @@
+## BinaryOp inplace-add unicode: deopt falls back to __add__/__iadd__
+
+class S(str):
+    def __add__(self, other):
+        return "ADD"
+
+    def __iadd__(self, other):
+        return "IADD"
+
+
+def add_path_fallback_uses_add():
+    x = "a"
+    y = "b"
+    for i in range(1200):
+        if i == 600:
+            x = S("s")
+            y = "t"
+        x = x + y
+    return x
+
+
+def iadd_path_fallback_uses_iadd():
+    x = "a"
+    y = "b"
+    for i in range(1200):
+        if i == 600:
+            x = S("s")
+            y = "t"
+        x += y
+    return x
+
+
+assert add_path_fallback_uses_add().startswith("ADD")
+assert iadd_path_fallback_uses_iadd().startswith("IADD")
+
+
+## BINARY_SUBSCR_STR_INT: ASCII singleton identity
+
+def check_ascii_subscr_singleton_after_warmup():
+    s = "abc"
+    first = None
+    for i in range(4000):
+        c = s[0]
+        if i >= 3500:
+            if first is None:
+                first = c
+            else:
+                assert c is first
+
+
+check_ascii_subscr_singleton_after_warmup()
+
+
+## BINARY_SUBSCR_STR_INT: Latin-1 singleton identity
+
+def check_latin1_subscr_singleton_after_warmup():
+    for s in ("abc", "éx"):
+        first = None
+        for i in range(5000):
+            c = s[0]
+            if i >= 4500:
+                if first is None:
+                    first = c
+                else:
+                    assert c is first
+
+
+check_latin1_subscr_singleton_after_warmup()