Thread-safe type operations: type lock, QSBR type-cache reclamation, GC stop-the-world, and interpreter optimizations - #7416
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes type-mutation locking under a new global Changes
Sequence Diagram(s)sequenceDiagram
participant Frame as Frame (specializer)
participant Type as PyType
participant VM as VM (type_mutex)
participant Cache as TYPE_CACHE
Frame->>Type: lookup_ref_and_version_interned(name, vm)
Type->>VM: with_type_lock (acquire type_mutex)
alt TYPE_CACHE hit with non-zero version
Type->>Cache: read cached entry
Cache-->>Type: (value, version)
else cache miss or version==0
Type->>Type: perform MRO lookup for name
Type->>Cache: insert/update entry (under lock)
end
Type->>VM: release type_mutex
Type-->>Frame: return (attr_ref_opt, type_version)
Frame->>Frame: use returned attr_ref_opt and type_version in guard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
d0cfe84 to
1daba38
Compare
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_generators.py (TODO: 10) dependencies: dependent tests: (no tests depend on generator) [x] test: cpython/Lib/test/test_frame.py (TODO: 4) dependencies: dependent tests: (no tests depend on frame) [x] test: cpython/Lib/test/test_descr.py (TODO: 32) dependencies: dependent tests: (no tests depend on descr) [x] lib: cpython/Lib/ssl.py dependencies:
dependent tests: (53 tests)
[x] lib: cpython/Lib/asyncio dependencies:
dependent tests: (7 tests)
[ ] test: cpython/Lib/test/test_monitoring.py (TODO: 5) dependencies: dependent tests: (no tests depend on monitoring) [x] lib: cpython/Lib/inspect.py dependencies:
dependent tests: (96 tests)
[x] lib: cpython/Lib/pdb.py dependencies:
dependent tests: (1 tests)
[x] lib: cpython/Lib/io.py dependencies:
dependent tests: (108 tests)
[x] lib: cpython/Lib/traceback.py dependencies:
dependent tests: (161 tests)
Legend:
|
1865733 to
da1d518
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
extra_tests/custom_text_test_runner.py (1)
391-396:⚠️ Potential issue | 🟡 MinorInconsistent access pattern may cause AttributeError for plain functions.
Line 392-394 still uses the old
__func__.__dict__access pattern while line 395 uses the new helper. For plain functions (without__func__), this will raiseAttributeErrorbefore the helper is even called.Proposed fix
if self.test_types: - if "test_type" in getattr( - test, test._testMethodName - ).__func__.__dict__ and set([s.lower() for s in self.test_types]) == set( + if "test_type" in _get_method_dict(test) and set([s.lower() for s in self.test_types]) == set( [s.lower() for s in _get_method_dict(test)["test_type"]] ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extra_tests/custom_text_test_runner.py` around lines 391 - 396, The conditional uses getattr(test, test._testMethodName).__func__.__dict__ which will raise AttributeError for plain functions; replace that access with the safe helper used on the next line by calling _get_method_dict(test) consistently: check for "test_type" in _get_method_dict(test) and compare set([s.lower() for s in self.test_types]) against set([s.lower() for s in _get_method_dict(test)["test_type"]]) so you never access __func__ directly (symbols: self.test_types, test._testMethodName, _get_method_dict, "test_type").
🧹 Nitpick comments (1)
crates/vm/src/frame.rs (1)
7436-7436: Keep the remaining attr specializers on one type snapshot.These sites now start from
version_for_specialization(), but the surrounding specializer still does later MRO/dict/descriptor inspection after that call. That leavesLOAD_ATTR/STORE_ATTRon a looser snapshot than the new__getitem__/__init__paths. I’d either switch these to an atomic lookup helper as well or revalidate the version right before publishing the specialized opcode/cache.Also applies to: 7474-7474, 7694-7694, 7729-7729, 9121-9121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/frame.rs` at line 7436, The attr specializers call cls.version_for_specialization() early but then perform additional MRO/dict/descriptor inspection, leaving LOAD_ATTR/STORE_ATTR snapshots looser than the new __getitem__/__init__ paths; update the attr-specializer flows (the sites calling version_for_specialization(), e.g. the code around version_for_specialization at the lines flagged and the logic that publishes the specialized opcode/cache for LOAD_ATTR/STORE_ATTR) to either perform the whole lookup atomically via a helper (similar to the __getitem__/__init__ path) or revalidate/refresh the class version immediately before publishing the specialized opcode/cache so the snapshot used for specialization is exact; locate references to version_for_specialization, the LOAD_ATTR/STORE_ATTR specialization code path, and the publish/emit-cache logic and ensure the version is checked/locked at the final publish point.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/builtins/type.rs`:
- Around line 2607-2629: The attribute mutation currently invalidates versions
and mutates the attributes map inside with_type_lock but applies slot table
updates (update_slot) outside the same lock; move the call to update_slot() into
the same with_type_lock closure (alongside modified_inner() and the
attributes.write() insert/shift_remove) so the tp_version_tag, inline-cache
invalidation, dict mutation, and slot rewrite occur atomically under the type
lock, then return the previous value and drop it outside the lock as before;
ensure both assign and remove paths in the closure call update_slot on the
appropriate slot and preserve the existing error handling (references:
with_type_lock, modified_inner, attributes.write(), update_slot).
- Around line 1427-1443: Compute and validate the new bases/MROs for zelf and
all descendants before mutating the live structures: inside with_type_lock, do
not immediately assign to zelf.bases; instead build the candidate bases and run
PyType::resolve_mro for zelf and every subclass (the same logic as
update_mro_recursively) into a temporary mapping of PyType -> new_mro
(preserving mro[0] where needed) and return any error if resolution fails; only
after all resolve_mro calls succeed, assign *zelf.bases.write() = bases and then
write each cls.mro from the prepared mapping and proceed (this ensures
update_mro_recursively-like validation happens before any live mutation).
- Around line 1436-1439: The loop over cls.subclasses currently unwraps weak
refs and downcasts (calling upgrade().unwrap() and downcast_ref().unwrap()),
which can panic on stale weakrefs; instead, change the loop to skip entries that
fail to upgrade or fail the downcast by using conditional checks (e.g., if let
Some(strong) = subclass.upgrade() { if let Some(subclass_pytype) =
strong.downcast_ref::<Py<PyType>>() { update_mro_recursively(subclass_pytype,
vm)? } } ) so stale weakrefs are ignored rather than unwrapping and crashing;
apply the same non-unwrapping pattern to the other occurrences around
update_mro_recursively (the similar block at lines ~1451-1459).
- Around line 1427-1449: The MRO update only rewrites bases/mro for zelf and
leaves derived metadata (per-class slot tables and __base__) stale on
subclasses; modify update_mro_recursively (inside with_type_lock) so that after
computing and writing each class's mro you also recompute and write its
base/__base__ (the same logic used for the original class), call that class's
init_slots(&vm.ctx) to rebuild its slots, and call modified_inner() to
invalidate caches for that class before recursing into its subclasses; ensure
you perform these steps for zelf as well (not just at the top level) so every
descendant gets updated slot tables and base-chain info.
- Around line 454-458: with_type_lock currently takes a zero-arg closure so
callers (like assign_version_tag via version_for_specialization /
find_name_in_mro) can end up executing code outside the held lock; change
with_type_lock signature to fn with_type_lock<R>(vm: &VirtualMachine, f: impl
FnOnce(&VirtualMachine) -> R) -> R, acquire the lock into _guard as before and
call f(vm) while the guard is in scope, then update all call sites (notably
version_for_specialization, assign_version_tag, find_name_in_mro and the block
around 490-503) to accept the &VirtualMachine parameter so the work runs while
type_mutex is held, preserving the invariant used by modified_inner.
---
Outside diff comments:
In `@extra_tests/custom_text_test_runner.py`:
- Around line 391-396: The conditional uses getattr(test,
test._testMethodName).__func__.__dict__ which will raise AttributeError for
plain functions; replace that access with the safe helper used on the next line
by calling _get_method_dict(test) consistently: check for "test_type" in
_get_method_dict(test) and compare set([s.lower() for s in self.test_types])
against set([s.lower() for s in _get_method_dict(test)["test_type"]]) so you
never access __func__ directly (symbols: self.test_types, test._testMethodName,
_get_method_dict, "test_type").
---
Nitpick comments:
In `@crates/vm/src/frame.rs`:
- Line 7436: The attr specializers call cls.version_for_specialization() early
but then perform additional MRO/dict/descriptor inspection, leaving
LOAD_ATTR/STORE_ATTR snapshots looser than the new __getitem__/__init__ paths;
update the attr-specializer flows (the sites calling
version_for_specialization(), e.g. the code around version_for_specialization at
the lines flagged and the logic that publishes the specialized opcode/cache for
LOAD_ATTR/STORE_ATTR) to either perform the whole lookup atomically via a helper
(similar to the __getitem__/__init__ path) or revalidate/refresh the class
version immediately before publishing the specialized opcode/cache so the
snapshot used for specialization is exact; locate references to
version_for_specialization, the LOAD_ATTR/STORE_ATTR specialization code path,
and the publish/emit-cache logic and ensure the version is checked/locked at the
final publish point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a8a615eb-9d3f-4cd2-a1b8-841c4089a979
⛔ Files ignored due to path filters (1)
Lib/test/test_class.pyis excluded by!Lib/**
📒 Files selected for processing (8)
.cspell.json.github/workflows/cron-ci.yamlcrates/vm/src/builtins/type.rscrates/vm/src/frame.rscrates/vm/src/stdlib/posix.rscrates/vm/src/vm/interpreter.rscrates/vm/src/vm/mod.rsextra_tests/custom_text_test_runner.py
| 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<PyType> = subclass.downcast_ref().unwrap(); | ||
| update_mro_recursively(subclass, vm)?; | ||
| } | ||
| Ok(()) | ||
| } | ||
| Ok(()) | ||
| } | ||
| update_mro_recursively(zelf, vm)?; | ||
| update_mro_recursively(zelf, vm)?; |
There was a problem hiding this comment.
Validate the full __bases__ rewrite before mutating the live hierarchy.
zelf.bases is overwritten before resolve_mro() has succeeded for zelf and every descendant. If any recursive update_mro_recursively() call fails, this setter raises after leaving a partially applied base/MRO graph behind.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/builtins/type.rs` around lines 1427 - 1443, Compute and
validate the new bases/MROs for zelf and all descendants before mutating the
live structures: inside with_type_lock, do not immediately assign to zelf.bases;
instead build the candidate bases and run PyType::resolve_mro for zelf and every
subclass (the same logic as update_mro_recursively) into a temporary mapping of
PyType -> new_mro (preserving mro[0] where needed) and return any error if
resolution fails; only after all resolve_mro calls succeed, assign
*zelf.bases.write() = bases and then write each cls.mro from the prepared
mapping and proceed (this ensures update_mro_recursively-like validation happens
before any live mutation).
| for subclass in cls.subclasses.write().iter() { | ||
| let subclass = subclass.upgrade().unwrap(); | ||
| let subclass: &Py<PyType> = subclass.downcast_ref().unwrap(); | ||
| update_mro_recursively(subclass, vm)?; |
There was a problem hiding this comment.
Don't unwrap subclass weakrefs during the recursive walk.
subclasses is lazily cleaned, and this branch only appends new weakrefs, so upgrade() can legitimately return None here. Turning that into unwrap() makes a stale weakref crash __bases__ assignment instead of just being skipped.
🔧 Suggested fix
- for subclass in cls.subclasses.write().iter() {
- let subclass = subclass.upgrade().unwrap();
- let subclass: &Py<PyType> = subclass.downcast_ref().unwrap();
- update_mro_recursively(subclass, vm)?;
- }
+ let subclasses: Vec<_> = {
+ let mut subclasses = cls.subclasses.write();
+ subclasses.retain(|weak| weak.upgrade().is_some());
+ subclasses.iter().filter_map(|weak| weak.upgrade()).collect()
+ };
+ for subclass in subclasses {
+ let subclass: &Py<PyType> = subclass.downcast_ref().unwrap();
+ update_mro_recursively(subclass, vm)?;
+ }Also applies to: 1451-1459
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/builtins/type.rs` around lines 1436 - 1439, The loop over
cls.subclasses currently unwraps weak refs and downcasts (calling
upgrade().unwrap() and downcast_ref().unwrap()), which can panic on stale
weakrefs; instead, change the loop to skip entries that fail to upgrade or fail
the downcast by using conditional checks (e.g., if let Some(strong) =
subclass.upgrade() { if let Some(subclass_pytype) =
strong.downcast_ref::<Py<PyType>>() { update_mro_recursively(subclass_pytype,
vm)? } } ) so stale weakrefs are ignored rather than unwrapping and crashing;
apply the same non-unwrapping pattern to the other occurrences around
update_mro_recursively (the similar block at lines ~1451-1459).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/vm/src/frame.rs (2)
7713-7744:⚠️ Potential issue | 🔴 CriticalThe metaclass descriptor check still has a version-skew window.
mcl_attris inspected beforemetaclass_versionis captured here. If another thread installs a metaclass data descriptor between those two steps,LoadAttrClass*can still be cached against the post-mutation metaclass version while bypassing the new descriptor precedence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/frame.rs` around lines 7713 - 7744, The metaclass descriptor check reads mcl_attr before capturing metaclass_version, leaving a race where a data descriptor could be installed between those steps; to fix, call mcl.version_for_specialization(_vm) and store metaclass_version (and handle the zero-version backoff early) before calling mcl.get_attr(attr_name), then only inspect the attribute's descr_set if metaclass_version is nonzero; reference mcl.version_for_specialization, metaclass_version, mcl.get_attr, mcl_attr, and the LoadAttrClass* specialization path when moving the attribute-inspection after the version capture and preserving the existing adaptive_counter_backoff behavior when version_for_specialization returns 0.
7433-7439:⚠️ Potential issue | 🔴 CriticalCapture the type version from the same snapshot as these slot checks.
These blocks still read mutable type behavior (
getattro/setattro,__bool__/__len__,tp_new/tp_alloc) before they capture the version they cache against. Under free-threading, another thread can mutate the type in between and make us publish a specialization under the new version while still relying on the old behavior, which can skip a newly-installed__getattribute__,__setattr__,__bool__/__len__, or__new__.Also applies to: 7477-7490, 8455-8488, 8781-8794, 9106-9112, 9124-9136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/frame.rs` around lines 7433 - 7439, The slot-check code reads mutable slot pointers (e.g., cls.slots.getattro.load() / cls.slots.setattro.load(), tp_new/tp_alloc, __bool__/__len__) before capturing the type version, which can race; fix by first taking a single snapshot of the type version (read and store the version tag field from cls once into a local, e.g., version_snapshot) and then read the slot pointers and compute is_default_* flags, finally publish the specialization tagged with that same version_snapshot; apply the same change to the equivalent blocks that check cls.slots.setattro, tp_new/tp_alloc, and __bool__/__len__ (the other occurrences noted at the listed ranges) so all slot-checks and the cached version come from the same snapshot.
♻️ Duplicate comments (5)
crates/vm/src/builtins/type.rs (5)
2614-2636:⚠️ Potential issue | 🔴 CriticalKeep the slot rewrite in the same type-lock transaction.
Lines 2617-2636 publish the dict mutation and version invalidation, but Lines 2638-2643 update the slot table after
type_mutexis released. Another thread can observe the new attribute state, assign a fresh version tag, or dispatch through the old slot table in that window.🔒 Suggested fix
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)) + let prev_value = 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 '{}'", @@ attr_name, ))); } - Ok(prev_value) - } + prev_value + }; + + if attr_name.as_wtf8().starts_with("__") && attr_name.as_wtf8().ends_with("__") { + if assign { + zelf.update_slot::<true>(attr_name, &vm.ctx); + } else { + zelf.update_slot::<false>(attr_name, &vm.ctx); + } + } + + Ok(prev_value) })?; - - if attr_name.as_wtf8().starts_with("__") && attr_name.as_wtf8().ends_with("__") { - if assign { - zelf.update_slot::<true>(attr_name, &vm.ctx); - } else { - zelf.update_slot::<false>(attr_name, &vm.ctx); - } - } Ok(())Also applies to: 2638-2643
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/builtins/type.rs` around lines 2614 - 2636, The dict mutation, version invalidation, and the slot-table rewrite must occur inside the same type lock to avoid a race; move the slot-table update logic into the closure passed to Self::with_type_lock so modified_inner(), the attributes.write().insert/shift_remove mutation (handling PySetterValue::Assign), and the slot table rewrite/update happen before the lock is released, but keep dropping the previous value outside the lock by returning it from the closure (as _prev_value) so its destructor runs after with_type_lock returns.
1434-1450:⚠️ Potential issue | 🔴 CriticalValidate the full
__bases__rewrite before touching live state.Line 1435 overwrites
zelf.basesbefore anyresolve_mro()call succeeds. Ifupdate_mro_recursively()then fails forzelfor any descendant, this setter unwinds after partially mutating the hierarchy.
1437-1456:⚠️ Potential issue | 🟠 MajorRecompute
baseand slot tables for every rebased class.
update_mro_recursively()only rewritesmro, and onlyzelfrunsinit_slots()on Line 1456. Descendants keep slot tables derived from the old MRO, andself.base/__base__is never refreshed for any of the rewritten types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/builtins/type.rs` around lines 1437 - 1456, update_mro_recursively currently only rewrites each type's mro, but doesn't refresh per-class base/slot state; update the function so that after computing and assigning mro it also recomputes and writes the class base/__base__ (e.g. set cls.base to the appropriate MRO entry) and calls cls.init_slots(&vm.ctx) and cls.modified_inner() for that class (instead of only calling init_slots/modified on zelf after recursion). Refer to update_mro_recursively, PyType::resolve_mro, cls.mro, cls.base, cls.subclasses, modified_inner, and init_slots to locate where to update base and reinitialize slots for each rewritten subclass.
1443-1446:⚠️ Potential issue | 🟠 MajorSkip dead subclass weakrefs instead of unwrapping.
subclassesis lazily cleaned. Line 1444'supgrade().unwrap()can legitimately fail and turn__bases__assignment into a panic instead of just ignoring a stale weakref.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/builtins/type.rs` around lines 1443 - 1446, The loop currently calls upgrade().unwrap() (and then downcast_ref().unwrap()) on entries in cls.subclasses, which can panic on dead weakrefs; change it to safely skip stale weakrefs by checking the Option/Result: replace the unwrap chain with a conditional that first does if let Some(strong) = subclass.upgrade() { if let Some(sub_pytype) = strong.downcast_ref::<Py<PyType>>() { update_mro_recursively(sub_pytype, vm)?; } } so dead weakrefs or unexpected types are ignored instead of causing a panic.
496-508:⚠️ Potential issue | 🔴 CriticalSerialize the public version-tag path too.
assign_version_tag()still just forwards to_inner, and Line 1138 still reaches it fromfind_name_in_mro()withouttype_mutex. That leaves the base-before-subclass tagging invariant racing withmodified_inner(), so a subclass can still end up with a nonzerotp_version_tagafter its base was reset to0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/builtins/type.rs`:
- Around line 1607-1617: The annotations cache cleanup should run
unconditionally: inside the closure passed to Self::with_type_lock (around
modified_inner, attributes.write(), and the attrs.insert(identifier!(vm,
__annotate_func__), value) call), always remove any existing identifier!(vm,
__annotations_cache__) instead of only calling attrs.swap_remove(...) when
!vm.is_none(&value); i.e., drop the vm.is_none conditional and unconditionally
call attrs.swap_remove(identifier!(vm, __annotations_cache__)) before inserting
the new __annotate_func__ value so assigning None will also clear the cached
__annotations__.
---
Outside diff comments:
In `@crates/vm/src/frame.rs`:
- Around line 7713-7744: The metaclass descriptor check reads mcl_attr before
capturing metaclass_version, leaving a race where a data descriptor could be
installed between those steps; to fix, call mcl.version_for_specialization(_vm)
and store metaclass_version (and handle the zero-version backoff early) before
calling mcl.get_attr(attr_name), then only inspect the attribute's descr_set if
metaclass_version is nonzero; reference mcl.version_for_specialization,
metaclass_version, mcl.get_attr, mcl_attr, and the LoadAttrClass* specialization
path when moving the attribute-inspection after the version capture and
preserving the existing adaptive_counter_backoff behavior when
version_for_specialization returns 0.
- Around line 7433-7439: The slot-check code reads mutable slot pointers (e.g.,
cls.slots.getattro.load() / cls.slots.setattro.load(), tp_new/tp_alloc,
__bool__/__len__) before capturing the type version, which can race; fix by
first taking a single snapshot of the type version (read and store the version
tag field from cls once into a local, e.g., version_snapshot) and then read the
slot pointers and compute is_default_* flags, finally publish the specialization
tagged with that same version_snapshot; apply the same change to the equivalent
blocks that check cls.slots.setattro, tp_new/tp_alloc, and __bool__/__len__ (the
other occurrences noted at the listed ranges) so all slot-checks and the cached
version come from the same snapshot.
---
Duplicate comments:
In `@crates/vm/src/builtins/type.rs`:
- Around line 2614-2636: The dict mutation, version invalidation, and the
slot-table rewrite must occur inside the same type lock to avoid a race; move
the slot-table update logic into the closure passed to Self::with_type_lock so
modified_inner(), the attributes.write().insert/shift_remove mutation (handling
PySetterValue::Assign), and the slot table rewrite/update happen before the lock
is released, but keep dropping the previous value outside the lock by returning
it from the closure (as _prev_value) so its destructor runs after with_type_lock
returns.
- Around line 1437-1456: update_mro_recursively currently only rewrites each
type's mro, but doesn't refresh per-class base/slot state; update the function
so that after computing and assigning mro it also recomputes and writes the
class base/__base__ (e.g. set cls.base to the appropriate MRO entry) and calls
cls.init_slots(&vm.ctx) and cls.modified_inner() for that class (instead of only
calling init_slots/modified on zelf after recursion). Refer to
update_mro_recursively, PyType::resolve_mro, cls.mro, cls.base, cls.subclasses,
modified_inner, and init_slots to locate where to update base and reinitialize
slots for each rewritten subclass.
- Around line 1443-1446: The loop currently calls upgrade().unwrap() (and then
downcast_ref().unwrap()) on entries in cls.subclasses, which can panic on dead
weakrefs; change it to safely skip stale weakrefs by checking the Option/Result:
replace the unwrap chain with a conditional that first does if let Some(strong)
= subclass.upgrade() { if let Some(sub_pytype) =
strong.downcast_ref::<Py<PyType>>() { update_mro_recursively(sub_pytype, vm)?; }
} so dead weakrefs or unexpected types are ignored instead of causing a panic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c8c994ce-d646-4364-b6ea-ea561ef96c2b
📒 Files selected for processing (5)
crates/vm/src/builtins/type.rscrates/vm/src/frame.rscrates/vm/src/stdlib/posix.rscrates/vm/src/vm/interpreter.rscrates/vm/src/vm/mod.rs
✅ Files skipped from review due to trivial changes (1)
- crates/vm/src/stdlib/posix.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/vm/src/vm/interpreter.rs
- crates/vm/src/vm/mod.rs
| 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) | ||
| }); |
There was a problem hiding this comment.
Clear __annotations_cache__ even when __annotate__ becomes None.
Right now the cache is only cleared when the new value is callable. If the class already materialized __annotations__, assigning None here leaves __annotations_cache__ behind, so later reads keep returning the stale dict.
💡 Minimal fix
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 removed = attrs.swap_remove(identifier!(vm, __annotations_cache__));
let prev = attrs.insert(identifier!(vm, __annotate_func__), value);
(removed, prev)
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/builtins/type.rs` around lines 1607 - 1617, The annotations
cache cleanup should run unconditionally: inside the closure passed to
Self::with_type_lock (around modified_inner, attributes.write(), and the
attrs.insert(identifier!(vm, __annotate_func__), value) call), always remove any
existing identifier!(vm, __annotations_cache__) instead of only calling
attrs.swap_remove(...) when !vm.is_none(&value); i.e., drop the vm.is_none
conditional and unconditionally call attrs.swap_remove(identifier!(vm,
__annotations_cache__)) before inserting the new __annotate_func__ value so
assigning None will also clear the cached __annotations__.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/vm/src/frame.rs (1)
7936-7949:⚠️ Potential issue | 🔴 CriticalFix the
__getitem__cache synchronization before expanding this fast path.Lines 7936-7949 expand
BINARY_OP_SUBSCRcoverage throughcache_getitem_for_specialization(), but the reader side has a memory ordering bug. Incrates/vm/src/builtins/type.rs, line 1070 readsgetitem_versionwithOrdering::Relaxed, while the writer at line 1054 stores withOrdering::Release. This breaks the Release-Acquire synchronization protocol—the reader must useOrdering::Acquireto safely pair with the Release store. Without this fix, concurrent type invalidation can surface stale(func, version)pairs, allowing dispatch to obsolete__getitem__implementations. Change line 1070 toload(Ordering::Acquire)before relying on this cache in the specialization path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/frame.rs` around lines 7936 - 7949, The reader side of the getitem cache uses a relaxed load and must use acquire semantics to pair with the release store to avoid seeing stale (func, version) pairs; in the reader code that consults getitem_version (the load currently using Ordering::Relaxed in the type lookup path referenced by cache_getitem_for_specialization and the BINARY_OP_SUBSCR specialization), change the atomic load to use Ordering::Acquire before relying on the cached (func, version) tuple so it correctly synchronizes with the writer's store(Ordering::Release).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/builtins/type.rs`:
- Around line 1001-1010: The code in the closure passed to Self::with_type_lock
repeats the same tp_version_tag check twice
(self.tp_version_tag.load(Ordering::Acquire) != tp_version); remove the
redundant second if so the version is checked once before calling
ext.specialization_cache.swap_init(Some(init), Some(vm)), keeping the early
return behavior intact and preserving the overall control flow.
---
Outside diff comments:
In `@crates/vm/src/frame.rs`:
- Around line 7936-7949: The reader side of the getitem cache uses a relaxed
load and must use acquire semantics to pair with the release store to avoid
seeing stale (func, version) pairs; in the reader code that consults
getitem_version (the load currently using Ordering::Relaxed in the type lookup
path referenced by cache_getitem_for_specialization and the BINARY_OP_SUBSCR
specialization), change the atomic load to use Ordering::Acquire before relying
on the cached (func, version) tuple so it correctly synchronizes with the
writer's store(Ordering::Release).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8a43a5f6-0d1f-49d6-861e-c135fbadbb96
📒 Files selected for processing (5)
crates/vm/src/builtins/type.rscrates/vm/src/frame.rscrates/vm/src/stdlib/posix.rscrates/vm/src/vm/interpreter.rscrates/vm/src/vm/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/vm/src/vm/interpreter.rs
| Self::with_type_lock(vm, || { | ||
| if self.tp_version_tag.load(Ordering::Acquire) != tp_version { | ||
| return false; | ||
| } | ||
| if self.tp_version_tag.load(Ordering::Acquire) != tp_version { | ||
| return false; | ||
| } | ||
| ext.specialization_cache.swap_init(Some(init), Some(vm)); | ||
| true | ||
| }) |
There was a problem hiding this comment.
Remove duplicate version check.
Lines 1002-1003 and 1005-1006 perform the identical check twice consecutively. This appears to be a copy-paste error.
🔧 Proposed fix
Self::with_type_lock(vm, || {
if self.tp_version_tag.load(Ordering::Acquire) != tp_version {
return false;
}
- if self.tp_version_tag.load(Ordering::Acquire) != tp_version {
- return false;
- }
ext.specialization_cache.swap_init(Some(init), Some(vm));
true
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Self::with_type_lock(vm, || { | |
| if self.tp_version_tag.load(Ordering::Acquire) != tp_version { | |
| return false; | |
| } | |
| 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; | |
| } | |
| ext.specialization_cache.swap_init(Some(init), Some(vm)); | |
| true | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/builtins/type.rs` around lines 1001 - 1010, The code in the
closure passed to Self::with_type_lock repeats the same tp_version_tag check
twice (self.tp_version_tag.load(Ordering::Acquire) != tp_version); remove the
redundant second if so the version is checked once before calling
ext.specialization_cache.swap_init(Some(init), Some(vm)), keeping the early
return behavior intact and preserving the overall control flow.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/vm/src/frame.rs (1)
7919-7928: Hoist the cheap gates beforelookup_ref_and_version_interned().This helper now takes the global
type_mutexand may allocate a fresh version tag. Since this path can only specialize heap types when eval-frame specialization is enabled, checking those conditions first would avoid paying the lock/version cost on immediate deopts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/frame.rs` around lines 7919 - 7928, Hoist the cheap guards before calling cls.lookup_ref_and_version_interned by first checking cls.slots.flags.has_feature(PyTypeFlags::HEAPTYPE), !self.specialization_eval_frame_active(vm), and whether the identifier lookup would be a PyFunction that can_specialize_call(2) (using the same downcast_ref_if_exact::<PyFunction> and can_specialize_call checks) — only then call lookup_ref_and_version_interned(identifier!(vm, __getitem__), vm) to obtain (getitem, type_version); this avoids taking the global lock / allocating a version tag on fast-failing paths and keep the subsequent logic that uses type_version and cls.cache_getitem_for_specialization unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/vm/src/frame.rs`:
- Around line 7919-7928: Hoist the cheap guards before calling
cls.lookup_ref_and_version_interned by first checking
cls.slots.flags.has_feature(PyTypeFlags::HEAPTYPE),
!self.specialization_eval_frame_active(vm), and whether the identifier lookup
would be a PyFunction that can_specialize_call(2) (using the same
downcast_ref_if_exact::<PyFunction> and can_specialize_call checks) — only then
call lookup_ref_and_version_interned(identifier!(vm, __getitem__), vm) to obtain
(getitem, type_version); this avoids taking the global lock / allocating a
version tag on fast-failing paths and keep the subsequent logic that uses
type_version and cls.cache_getitem_for_specialization unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c9408811-8c8e-494f-9382-cff7f814ef3e
📒 Files selected for processing (5)
crates/vm/src/builtins/type.rscrates/vm/src/frame.rscrates/vm/src/stdlib/posix.rscrates/vm/src/vm/interpreter.rscrates/vm/src/vm/mod.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/vm/src/vm/interpreter.rs
- crates/vm/src/vm/mod.rs
| let subclass: &Py<PyType> = subclass.downcast_ref().unwrap(); | ||
| update_mro_recursively(subclass, vm)?; | ||
| Self::with_type_lock(vm, || { | ||
| *zelf.bases.write() = bases; |
There was a problem hiding this comment.
do we still need bases lock?
| 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; |
37ff5f7 to
8326db9
Compare
e637be3 to
6ed555e
Compare
On unix threading builds, publish each thread's top Python frame in a single relaxed AtomicPtr store from set_current_frame instead of pushing onto a parking_lot::Mutex<Vec<FramePtr>> per call. Cross-thread readers (sys._current_frames, cross-thread f_back, faulthandler.dump_traceback, the GC unreachable debug-assert) run under stop-the-world and walk the published top frame down the Frame::previous chain; the owning thread is then parked at a safepoint, so the pointer and the frames it reaches are quiescent and alive. The faulthandler watchdog is a plain OS thread that cannot stop-the-world, so it walks the chain lock-free and best-effort. Non-unix threading builds have no stop-the-world and keep the existing mutex-guarded frame stack unchanged. Assisted-by: Claude
with_frame saves and restores the shared exc_info slot around every Python call to contain frames that leave it unbalanced. Cache a has_exc_handling bit on PyCode at creation, set when the bytecode contains any opcode that calls vm.set_exception (PushExcInfo, PopExcept, CheckEgMatch, EndAsyncFor, InstrumentedEndAsyncFor). A callee whose code has none of these cannot mutate the slot, so the save and restore are skipped for it. Generators go through resume_gen_frame and are unaffected. Assisted-by: Claude
Optimized (function) frames now expose `f_locals` as a `FrameLocalsProxy` implementing PEP 667 semantics instead of a cached snapshot dict: - reads go live through the fast-local slots; each access mints a fresh proxy; keys that do not name a fast local are stored in a per-frame `f_extra_locals` side dict and folded into `locals()`. - writes to a fast-local key store into the slot (or its cell) in place; deleting a fast local raises ValueError; extra keys delete normally. - full mapping protocol: keys/values/items (lists), get/pop/setdefault, update (dict or FrameLocalsProxy only), __or__/__ior__/__ror__ (dict result), copy (plain dict), __reduce__ blocks pickling/copy, repr with recursion guard, mapping-pattern and Mapping ABC support. Class/module/exec frames keep returning their namespace mapping directly. Cross-thread access to a frame running on another thread still raises RuntimeError. A closed generator now keeps its frame locals when a durable frame reference escaped (f_locals proxy, sys._getframe, f_back), matching take_ownership; the escape is tracked with a per-frame flag. The snapshot-then-fold locals_to_fast/locals_dirty write-back is retired since proxy writes reach the slots directly. Assisted-by: Claude
When a frame escapes its execution (referenced through a traceback, `sys._getframe`, `f_locals`, ...), capture a strong reference to its caller at release time. `f_back` consults it once the caller has left the live frame chain, so the Python-visible frame chain survives return. The retained reference is a GC-traversed edge and is cleared by `frame.clear()`, so ancestor chains stay collectable. Assisted-by: Claude
Make check_c_stack_overflow one-sided (trip whenever the stack pointer is below the soft limit) so a single native frame larger than the margin cannot step past the danger band undetected. Raise the debug STACK_MARGIN_BYTES from 4096 to 16384 words so the margin exceeds a single debug interpreter frame, leaving headroom to raise RecursionError. Release margin unchanged. Clamp the soft-limit margin to half the stack so small explicit thread stacks do not get a soft limit above their stack top. Assisted-by: Claude
Exception construction went through into_ref_with_type, which eagerly allocated an empty instance dict for every HAS_DICT type. Add into_ref_with_type_lazy_dict, which builds the instance with an unallocated dict slot, and route the four exception construction sites (PyBaseException, PyOSError, OSErrorBuilder, PyBaseExceptionGroup) through it. The dict now materializes on first attribute write or __dict__ access via the existing get_or_insert path. add_note and PyImportError::slot_init now obtain the dict through object_get_dict so they materialize it instead of assuming it exists. A freshly constructed exception no longer reports an empty dict in gc.get_referents, matching the reference interpreter. Assisted-by: Claude
Route vm.new_exception() through into_ref_with_type_lazy_dict so internally raised exceptions (new_type_error, new_value_error, etc.) start without an instance dict, matching the slot_new path. The dict is materialized on the first attribute write or __dict__ access. Assisted-by: Claude
Reject keyword arguments and require exactly one positional argument, raising TypeError with the "takes no keyword arguments" and "takes exactly one argument (N given)" messages. Assisted-by: Claude
- Drop stale vm.frames reference from the release_datastack_frame uniqueness argument. - Assert has_exc_handling when unwinding an Except-typed stack slot, documenting the invariant that guards the shared exc_info write. - Truncate the specialized __init__ return-type name to 200 chars, matching the unspecialized wrapper. - Reword two comments to describe behavior without prose references to CPython. Assisted-by: Claude
Wrap the unix stop-the-world registry walk in a scope with scopeguard::defer! so start_the_world runs on panic, matching the f_back and get_all_current_frames sites. Add scopeguard to the stdlib dependencies. Also correct the restore_exception doc comment to name with_frame after the rename. Assisted-by: Claude
Assisted-by: Claude
Match the builtin_* naming convention of extra_tests/snippets. Assisted-by: Claude
Reformat with rustfmt, fix import spacing in builtin_type_bases.py with ruff, and add "pointee" to the cspell word list. Assisted-by: Claude
`online`, `drain_all`, and `reset_after_fork` are called only from unix code (thread attach/detach, post-fork reset), and `offline` from unix code plus a unit test. Gate them with matching cfg so `-D dead_code` does not fire on non-unix targets. Assisted-by: Claude
PyFunction::traverse visited the cells inside the closure tuple instead of the tuple object itself, so the tuple's reference from the function was never subtracted during cycle collection. A closure tuple that reached back to its function (or, through a frame retained by f_back, to a Thread) was stranded as a false GC root and never collected, leaking the whole cycle. Visit the tuple itself, matching clear(). Assisted-by: Claude
Assisted-by: Claude
test_asyncgen_finalization_by_gc and test_asyncgen_finalization_by_gc_in_other_thread now pass; GC finalizes the async generators. Assisted-by: Claude
The servername-callback reference cycle is now collected by GC. Assisted-by: Claude
Change `cfg(all(unix, feature = "threading"))` to `cfg(feature = "threading")` on the stop-the-world machinery so it also compiles and runs on non-unix threading builds: - StopTheWorldState, its stats, stw_trace, and the stop_the_world field - ThreadSlot state/stop_requested/thread fields and their initializers - wait_while_suspended/attach_thread/detach_thread/suspend_if_needed/do_suspend, allow_threads, stop_requested_for_current_thread, and the enter_vm / VmBootstrapGuard / attach_current_thread / release_current_thread / cleanup attach-state wiring - eval_breaker_tripped, check_signals, run_scheduled_gc, signal GC_BIT / schedule_gc / take_gc_scheduled, and the frame.rs safepoint call - CollectStopTheWorld and its use in collect_inner - QSBR::online/offline, now called from attach/detach on all threading builds - debug_assert_current_thread_attached and its type-cache call sites maybe_collect defers auto-collection to the bytecode safepoint on every threading build instead of only unix; non-threading builds keep the inline collect. stw_trace writes to std stderr on non-unix. top_frame publishing (CURRENT_TOP_FRAME_SLOT, set_current_frame), the frame-walk debug assert in collect_inner, and the fork reinit helpers remain unix-only; non-unix keeps ThreadSlot::frames for introspection. Assisted-by: Claude
Wrap the blocking Windows wait calls in `vm.allow_threads` so the calling thread transitions ATTACHED -> DETACHED for the duration of the wait: - _winapi: WaitForSingleObject, WaitForMultipleObjects, BatchedWaitForMultipleObjects, ConnectNamedPipe, ReadFile, Overlapped.GetOverlappedResult - _overlapped: Overlapped.getresult These previously blocked while ATTACHED, so a stop-the-world requester could never suspend the thread and spun in its wait loop indefinitely.
…bject The refcount test asserted exact incref/decref deltas on PyInt's shared type object. That object's reference count is perturbed by the other capi tests running in parallel, and is immortal under some interpreter configurations, so the deltas were not reliably +1/-1. Assert them on a freshly created, uniquely owned list whose reference count is private to the test and mortal. Assisted-by: Claude
The `#[pyexception]` struct macro now forwards a `traverse` option to the generated `#[pyclass]`, and `ExceptionItemMeta` accepts the `traverse` key. `PyOSError` is marked `traverse = "manual"`, so `HAS_TRAVERSE` is true and OSError-family instances are tracked at creation and traversed by the collector. `PyOSError::traverse` now visits the underlying `PyBaseException` (traceback, cause, context, args) instead of `PyException::try_traverse`, which was a no-op because `PyException` has `HAS_TRAVERSE = false`.
The BlockingIOError reference cycle is now collected by GC. Assisted-by: Claude
…GC stop-the-world, and interpreter optimizations (#7416) * type lock * 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. * Align type lock behavior with CPython * Add PUBLISHED flag bit to RefCount state word Assisted-by: Claude * Add QSBR module for deferred memory reclamation Assisted-by: Claude * Defer memory free of cache-published objects via QSBR Assisted-by: Claude * Wire QSBR checkpoints into thread lifecycle and eval breaker Assisted-by: Claude * Publish type-cache values to QSBR and bypass cache without a VM Mark cached type-method values as QSBR-published before storing their pointer in TYPE_CACHE, so racing readers never try-incref freed memory. Debug-assert that the current thread is ATTACHED whenever a lock-free cache read happens. Delete find_name_in_mro_without_vm and its unsound direct SeqLock read outside any VM/thread registration; find_name_in_mro now falls back to the locked, uncached MRO walk when no VM is current. This also removes assign_version_tag()'s last caller, so its unlocked fallback is deleted along with it (version_for_specialization keeps the locked path). VirtualMachine::initialize() runs Python bytecode (e.g. importing codecs/encodings) before any enter_vm scope exists, which left the bootstrap thread not ATTACHED for cache reads during startup. Add thread::VmBootstrapGuard, an RAII counterpart to enter_vm usable across statements that need &mut VirtualMachine, and wrap initialize() with it. Assisted-by: Claude * Use try-incref reads and QSBR-backed swaps in specialization cache Assisted-by: Claude * Reset QSBR thread registry after fork in the child Add Qsbr::reset_after_fork, which clears the registered thread slots before draining the retire queue. Call it in py_os_after_fork_child, right after type_cache_after_fork() and before _thread::after_fork_child() re-registers the surviving thread. Dead parent threads' Arc<QsbrSlot> handles live on in the child's copied memory with no destructor ever running, so without this the registry keeps them 'online' forever and future grace periods never complete. Assisted-by: Claude * Add threaded stress test for type cache mutation races Assisted-by: Claude * Restrict qsbr internals to pub(crate) and fix clippy lints Downgrade pub items in the private qsbr::threading module (QsbrSlot, Qsbr, QSBR, and its methods) to pub(crate), since they were never reachable outside the crate. Downgrade ThreadSlot::qsbr accordingly to avoid a private-interfaces warning. Import Arc/Weak from alloc instead of std to match clippy::std_instead_of_alloc. Rewrite the process() comment: goals are not strictly ordered by push order since advance() and the queue push aren't atomic together, so concurrent free_delayed calls can interleave; the drained prefix stays sound because each item individually passed poll, with reordering only affecting reclamation latency. Assisted-by: Claude * Gate per-instruction QSBR check behind a global pending flag eval_breaker_tripped() and check_signals() called thread::qsbr_break_requested() on every bytecode instruction, which does a thread-local lookup, RefCell borrow, and atomic load even when no QSBR retirement is pending. This caused a measured 10-18% slowdown on tight interpreter loops. Add Qsbr::pending, a global AtomicBool set under the queue lock while free_delayed() holds a retired allocation, and cleared under the same lock by process()/drain_all() once the queue empties. The hot path now checks Qsbr::break_pending() (a single relaxed static load) before touching thread-local state, and only pays the TLS cost while a retirement is actually in flight. Assisted-by: Claude * Merge signal and QSBR eval-breaker flags into one atomic word Replace the separate ANY_TRIGGERED AtomicBool and QSBR pending AtomicBool checks in the per-instruction eval-breaker path with a single AtomicU8 (EVAL_BREAKER) holding a bit per source. The global QSBR instance mirrors its pending state into the QSBR bit under the same queue-lock critical sections that already toggle pending, while local QSBR instances used by unit tests are left untouched. eval_breaker_tripped() and check_signals() now read/act on the merged word (eval_breaker_pending(), qsbr_bit_set()) instead of issuing two separate atomic loads per instruction. set_triggered() switches from store to fetch_or so a signal handler never clobbers the QSBR bit. Removed the now-unreferenced is_triggered() helper; break_pending() is kept for unit tests only and marked allow(dead_code) outside test builds. Assisted-by: Claude * Skip freelist reuse for published objects in default_dealloc Freelist-eligible payloads (tuple, int, list, dict, float, ...) were routed to T::freelist_push before ever reaching PyInner::dealloc, so a published object (e.g. a tuple cached as a type attribute) bypassed the is_published -> free_delayed QSBR hook entirely. That let PyRef::new_ref reuse the slot and rewrite the refcount word with a non-atomic core::ptr::write racing a reader's atomic try-incref, and let FreeList::drop call alloc::dealloc directly while a reader could still be mid-read. Gate the freelist branch in default_dealloc on RefCount::is_published(), checked before any teardown. A published object now always falls through to PyInner::dealloc, whose existing hook defers the memory free via QSBR. Document the non-unix (e.g. Windows) reclamation-latency limitation in the QSBR design doc's Lifecycle integration section (previously only noted in a code comment in vm/thread.rs). Extend the type-cache stress test so the mutator also churns a freelist-eligible published tuple class attribute (C.shape), read by readers each inner loop and deleted on the same cadence as C.m, to exercise the fixed bypass. Assisted-by: Claude * Use itoa for PyInt::to_str_radix_10 i64 fast path Format small integers with itoa::Buffer instead of i64::to_string() in PyInt::to_str_radix_10(), which backs both str(int)/repr(int) for exact int and PyObject::str()'s exact-int fast path. Adds the itoa crate as a workspace dependency. Assisted-by: Claude * Make object attribute dunders wrapper descriptors Remove the __getattribute__, __setattr__ and __delattr__ #[pymethod]s from PyBaseObject so add_operators installs PyWrapper slot wrappers for them from the #[pyslot] functions instead of method descriptors. lookup_slot_in_mro now classifies these entries as NativeSlot, so heap types without Python-level overrides keep the native getattro/setattro slot functions, which lets LOAD_ATTR/STORE_ATTR specialization apply to instances of Python-defined classes. Since __setattr__ and __delattr__ share the setattro slot, resolve both names together in update_one_slot: overriding only one of them no longer lets the other name's native resolution overwrite the dispatching wrapper. Assisted-by: Claude * Resolve TpSetattro pair on attribute deletion, not just addition update_one_slot's TpSetattro branch only ran the combined __setattr__/__delattr__ resolution (added in the previous commit) when ADD was true. On deletion (e.g. del C.__setattr__), it fell back to inherit_from_mro, which copies the base class's slot and discards the class's own remaining Python-level override of the other name. lookup_slot_in_mro reads the current attribute dicts, so the same resolution is correct for both addition and deletion; the ADD-only guard is removed and the logic now always runs. Assisted-by: Claude * Generate Opcode::cache_entries/deopt as table lookups generate_rs_opcode_metadata.py now emits a 256-entry const array for each function instead of a chained match over opcode names. Opcode::deopt() indexes an [Option<Opcode>; 256] table built from the existing specialization-to-base mapping, and Opcode::cache_entries() indexes a [u8; 256] table that bakes in the same deopt/to_base composition deoptimize() previously performed before its own match. Regenerated opcode_metadata.rs from the updated script; PseudoOpcode's bodies are unchanged since it has no cache/deopt entries to table. Added an exhaustive test in instruction.rs that checks every valid u8 opcode against a frozen copy of the old chained-match bodies. Assisted-by: Claude * Make Opcode/Instruction numeric conversions O(1) Opcode::as_numeric, Opcode::as_instruction and Instruction::as_opcode were each a per-variant match over ~230 arms in the define_opcodes! macro. Opcode now carries the same explicit $op_id discriminants and #[repr($typ)] as Instruction, which makes the conversions sound as: - Opcode::as_numeric: a plain `self as $typ` identity cast. - Opcode::as_instruction / Instruction::as_opcode: a mem::transmute, relying on both enums sharing one-$typ-wide layout (Instruction's payload fields are all the zero-sized Arg<T> marker, checked by a new per-instantiation size_of assertion). try_from_numeric is left as a match: $op_id values have gaps (specialized/instrumented opcode ranges), so a range check can't replace it. Adds #[inline] to the rewritten conversions plus deoptimize, and to the small generated wrapper/table-lookup functions from the previous opcode-metadata change (as_u8/as_u16, cache_entries, deopt, to_base) via generate_rs_opcode_metadata.py, then regenerates opcode_metadata.rs. Extends the instruction.rs equivalence-test pattern with two exhaustive tests (u8 and u16 instantiations) checking the new identity-cast/transmute conversions against the untouched try_from_numeric/TryFrom paths. Assisted-by: Claude * Pop CallIsinstance arguments directly instead of collecting into Vecs The specialized handler built two heap-allocated Vecs per call (pop_multiple().collect() plus a with_capacity(2) buffer) before invoking is_instance. Pop the class and instance straight off the stack for the effective two-argument case, removing the per-call allocations. Assisted-by: Claude * Inherit tp_new instead of always installing new_wrapper update_one_slot's TpNew branch stored new_wrapper for every ADD, so all heap types had slots.new == new_wrapper and the CallAllocAndEnterInit specialization never fired. Now __new__ is resolved through the MRO dicts: an own or Python-level definition installs new_wrapper, while a builtin __new__ entry (or none) inherits slots.new from the solid base. The now-reachable CallAllocAndEnterInit path gains the missing guards: the specializer requires co_argcount == oparg + 1 (via can_specialize_call) and rejects generator-like __init__, and the handler re-checks the argcount against the class-level init cache. Without the argcount check, __init__ defaults broke re._parser (UnboundLocalError in SubPattern.__init__). Remove the now-unused is_simple_for_call_specialization. Assisted-by: Claude * Skip locals dict allocation for the init-cleanup shim frame Mark the shim code object NEWLOCALS and create the shim frame with no locals mapping so FrameLocals::lazy() is used. The shim only executes ExitInitCheck/ReturnValue and never touches locals, so this removes one dict allocation per specialized instantiation. Assisted-by: Claude * Guard CALL_ALLOC_AND_ENTER_INIT with cached function version The init specialization cache stored only the type version, so swapping __init__.__code__ (which zeroes func_version) did not deopt the warmed call site and stale code assumptions (argcount shape, no varargs/kwonly, not a generator) were applied to the new code object. Store get_version_for_current_state() in the specialization cache when caching __init__ and re-check func_version() in the handler before using the cached function, deopting to the generic call path on mismatch, the same way the getitem specialization cache does. Assisted-by: Claude * Apply rustfmt and ruff-format fixes to files from earlier commits Assisted-by: Claude * Add frame object freelist Frame.iframe becomes FrameUnsafeCell<Option<InterpreterFrame>> so a dead frame can be left as an empty husk. Traverse::clear extracts all owned child references (including localsplus values via LocalsPlus::clear_into) and empties the cell. Dead frame allocations are cached in a thread-local FreeList<Frame> (up to 200 entries) and reused by PyRef::new_ref. clear_generator returns early when the frame was already cleared by cycle collection; generator drops can run after tp_clear of their frame. gc: skip tp_clear for objects saved to gc.garbage by DEBUG_SAVEALL, since they remain reachable from Python. Assisted-by: Claude * Drop frame children directly in tp_clear instead of extracting Pushing the frame's ~10 child references into the clear buffer grew a Vec on every frame deallocation. Take the interpreter frame out of the cell and drop it in place; LocalsPlus::clear_into is removed. Assisted-by: Claude * Untrack objects before tp_clear and guard cross-thread f_locals The cycle collector called tp_clear on unreachable objects while they were still linked into the GC generation lists, so another thread could obtain a strong reference to an already-cleared object (e.g. a frame husk with iframe == None) via gc.get_objects() and access the cleared payload. Untrack the dead set before the clear phase, mirroring the untrack-then-clear ordering of the refcount dealloc path. Objects that gained an external reference before untracking are found by comparing each object's strong count against the references coming from within the dead set; such late-resurrected objects and everything reachable from them are re-tracked and skipped, letting a later collection retry once the external references are released. Also reject f_locals access for a frame currently executing on another thread. Reading fastlocals of such a frame races with the executing thread overwriting the slots (torn reads of dropped values). Access from the executing thread itself (locals(), trace callbacks) is still allowed via the current-frame chain check. Assisted-by: Claude * Skip localsplus heap copy for uniquely referenced dying frames release_datastack_frame now checks the frame's strong count after untracking it from the GC generation lists. When the caller holds the only reference, localsplus values are dropped in place and the data stack storage is released without the heap copy; escaped frames (traceback, sys._getframe, trace callbacks) keep the copy path. The three inline materialize+pop call sites in function.rs are routed through release_datastack_frame. Assisted-by: Claude * Stage exact-args call arguments in stack slot buffers CallPyExactArgs, CallBoundMethodExactArgs and CallAllocAndEnterInit no longer collect popped arguments into a per-call Vec (plus a second Vec for self-prepending). Arguments are popped into a fixed-size Option<PyObjectRef> slot buffer (CallArgBuffer, 8 inline slots, heap fallback for larger arities) and moved into fastlocals via the new invoke_exact_args_slots / a take() iterator. - prepare_exact_args_frame now accepts an ExactSizeIterator of args - specialization_run_init_cleanup_shim takes the slot buffer and fills slot 0 with new_obj, dropping one redundant clone - LoadAttrGetattributeOverridden, LoadAttrProperty and BinaryOpSubscrGetitem pass inline slot arrays instead of vec![] Assisted-by: Claude * Reduce allocations in generic call paths - FuncArgs::prepend_arg: use reserve instead of reserve_exact so exact-capacity vectors do not realloc on every prepend. - IntoFuncArgs::into_method_args: build the final args vec once with capacity len + 1 instead of prepending into a full vector. - PyType::call: skip cloning FuncArgs when no init slot can run after slot_new (no init slot, not `type`, and slot_new is not new_wrapper). Assisted-by: Claude * Skip redundant exc_info restore on frame exit with_frame_impl saves the current exc_info slot value and restores it when the frame exits. When the slot still holds the same value, the restore rewrites the slot and recomputes the thread-exception mirror for no effect. Add restore_exception, which compares the slot against the saved value by pointer identity and skips the store when they match. The saved value is a strong reference held for the whole frame scope, so the pointed-to object cannot be freed and its address reused while the frame runs, making the pointer comparison free of ABA. Assisted-by: Claude * Apply rustfmt to gc_state.rs Assisted-by: Claude * Avoid empty FuncArgs clone in PyType::call Cloning args for the init call clones the kwargs map even when args is empty, which shows up in no-argument instantiation profiles. Prepare the init-call args before slot_new: a default FuncArgs when args is empty (indistinguishable from a clone of empty args), the existing clone otherwise. The empty check also keeps the init-slot load of the clone-elision path off the no-argument call path. Assisted-by: Claude * Push freelist husks only after tp_clear and child drops default_dealloc pushed the object onto the thread-local freelist before running clear_fn. Payloads that drop children inside clear (Frame) can run __del__ there, and a reentrant allocation could pop the husk and overwrite its payload while clear_fn still held a &mut borrow of it. Tuples had the same window through the extracted-edges drop, which ran after the push. Reorder default_dealloc to clear, drop the extracted children, and only then attempt the freelist push, so the husk becomes reusable only when no borrows into the payload remain. The tuple freelist bucketed husks by element count, which required reading the payload during push; after this reordering the elements are already cleared. PyInner<PyTuple> is a fixed-size allocation (the elements box is dropped and replaced on reuse), so replace the per-size buckets with the shared single-list FreeList. Remove the now-unused pyinner_layout helper. Assisted-by: Claude * Resolve __set__ and __delete__ together for the descr_set slot The descr_set slot serves both __set__ and __delete__, but the TpDescrSet accessor resolved only the single modified name and fell back to MRO inheritance on delete. Deleting one of the pair could disable the surviving operation: after `del D.__set__`, an instance delete stopped calling the remaining __delete__. Resolve both names together like the TpSetattro accessor does: any Python-level definition selects the dispatching wrapper, matching native functions are stored directly, a single native function with the other name absent is stored directly, and the slot is inherited from the MRO only when neither name resolves. Assisted-by: Claude * Update type base on __bases__ assignment set_bases replaced bases and mro but never updated the stored base, so __base__ kept reporting the old value after reassignment and slot resolution (tp_new inheritance, set_new/set_alloc during init_slots) read the stale solid base. Change the base field to PyAtomicRef<Option<PyType>> so it can be swapped under the type lock while remaining lock-free for readers, and recompute it in set_bases with the same best_base validation type creation uses (BASETYPE flag, instance layout conflict among bases). The swapped-out base is parked in the frame's temporary refs; other references released inside the critical section are dropped after the lock is released. Also in set_bases: - remove this class from the old bases' subclass lists (pruning dead entries), so repeated assignment no longer accumulates duplicates - roll back bases, base, and all updated mros when the recursive mro update fails, instead of leaving the type half-reparented - fix the misformatted empty-tuple error message Remove the expectedFailure marker from test_unsubclassable_types, which now passes. Assisted-by: Claude * Fix rollback order, dead weakrefs, and lock-held drops in set_bases - Restore recorded mros in reverse on rollback so a class visited multiple times through diamond inheritance ends with its original mro instead of an intermediate one. - Skip dead weakrefs in the subclass list during the recursive mro update instead of panicking on upgrade. - Retire the replaced mros on the success path instead of dropping them while the type lock is held. Assisted-by: Claude * Reify number sub-slot wrapper once in update_one_slot The update_sub_slot! macro expanded the Python-method wrapper closure at both the own and inherited store sites. Each expansion is a distinct fn item, so a base and a non-overriding subclass stored wrappers with distinct addresses in unmerged debug builds. binary_op1 compares slot fn addresses to detect whether a subclass overrides the operator, so the inherited slot was misread as an override and C() // E() dispatched to __rfloordiv__ instead of __floordiv__. Bind the wrapper store in a single closure and call it from both branches so the fn item is reified once and the slot value stays identical across a base and its subclasses. Document the conservative-on-mismatch nature of the new_wrapper address guard in call_wrapped. Assisted-by: Claude * Restrict type_cache_after_fork visibility and guard type-cache reads type_cache_after_fork is only called from within the crate, so mark it pub(crate) to resolve the unreachable-pub warning. Add debug_assert_current_thread_attached at the has_name_in_mro lock-free type-cache read site, matching lookup_ref_and_version_interned, and gate the function on debug_assertions so it is not compiled unused in release builds where the call sites are elided. Assisted-by: Claude * Guard BinaryOpSubscrGetitem with the cached type version The specialized BINARY_OP_SUBSCR_GETITEM handler used the cached __getitem__ after checking only the function version, unlike the sibling specialized handlers which revalidate the type version tag first. Store the type version in the inline cache at specialization time and revalidate owner.class().tp_version_tag against it before using the cached function, deopting to the generic subscript path on mismatch. Assisted-by: Claude * Call __abstractmethods__ __len__ once in object.__new__ The abstract-method check invoked the user-visible __len__ twice: once via length_opt for the count and again while materializing the method names. Derive the count from the materialized list instead, so __len__ runs once. Assisted-by: Claude * Use i64 fast paths for specialized int add/sub/mul Rewrite execute_binary_op_int to box results through new_int via i64 checked arithmetic instead of raw BigInt ops with new_bigint. Add an int_mul helper and a shared int_fast_op that computes the i64 result and falls back to the BigInt operation on to_i64 or checked-op failure. Wire int_mul into both the specialized BinaryOpMultiplyInt handler and the generic execute_bin_op Multiply/InplaceMultiply path, gated on exact int operands so subclasses keep dispatching through _mul/_imul. Assisted-by: Claude * Use i64 fast paths for exact int floordiv and remainder Add floordiv_i64/mod_i64 computing i64 floor-division and divisor-signed remainder, guarding zero divisor and i64::MIN overflow by returning None. Wire them through int_floordiv/int_mod (shared int_div_fast_op boxes via new_int) into the generic execute_bin_op FloorDivide/Remainder and their Inplace variants, gated on exact int operands so subclasses and zero divisors fall through to the existing _floordiv/_mod/_ifloordiv/_imod slow path. Assisted-by: Claude * Skip trashcan and untrack for non-GC-tracked objects in dealloc default_dealloc read is_gc_tracked() only to decide untracking, and entered the trashcan recursion guard unconditionally. Non-GC objects (int, float, str, ...) own no child references that recurse during deallocation, so they need neither the trashcan nor untracking. Read is_gc_tracked() once and gate both trashcan begin/end and untrack on it, removing three thread-local accesses per non-GC object deallocation. Assisted-by: Claude * Merge trashcan depth and defer queue into one thread-local struct trashcan::end accessed two separate thread-locals (DEALLOC_DEPTH and DEALLOC_QUEUE) at the outermost deallocation. Combine both into a single `Trashcan` thread-local holding Cell-based depth and queue fields, so begin and end each reach their state through one thread-local access. The queue is set back before each deferred dealloc call so reentrant begin/end during draining never holds an outstanding borrow. Assisted-by: Claude * Check object layout compatibility on __bases__ and __class__ assignment Add a shared compatible_for_assignment helper that walks each type to the base that fixes its instance layout and rejects the assignment when the old and new layouts differ (basicsize, itemsize, member count, __dict__, __weakref__, and __slots__ names). Wire it into set_bases, which previously performed no layout check, and replace the inline check in __class__ assignment, which compared the two types directly and over-rejected a subclass that adds no layout. Enable test_descr.test_builtin_bases, which the new set_bases check passes. Assisted-by: Claude * Stop the world around GC pointer-reading phases The cycle collector reads each tracked object's interpreter state during reference subtraction, the reachability walk and the strong-reference snapshot, including the localsplus of frames other threads are executing. Those slots are written without synchronization, so the reads are a data race under threading. collect_inner now stops the world before taking the generation read locks and restarts it after the strong-reference snapshot, before the finalizer/weakref/tp_clear phases. A debug assertion checks that no frame on any thread's call stack was classified unreachable. Automatic collections from maybe_collect are deferred to the next bytecode safepoint via a new eval-breaker GC bit instead of running synchronously: a synchronous collection can hold an internal lock (e.g. the lazy frame locals cell) that another thread is blocked on with no way to reach a safepoint, deadlocking the stop. Hardens the stop-the-world machinery for these frequent, concurrent GC stops: attach_thread now honors a pending stop after re-attaching so a thread doing rapid allow_threads calls cannot run past the requester; stop_the_world completion is level-triggered on all-threads-suspended rather than an edge-triggered countdown; and the thread-start started/ready handshakes detach while waiting so the waiter is parkable. Updates the Frame and PyRwLock/PyMutex traversal SAFETY comments to state the actual invariant. Assisted-by: Claude * Add threaded GC vs executing-frame stress snippet Workers churn frame state (recursion, generators, frame cycles) while a collector loops gc.collect() and an introspector walks live frame objects. Exercises the stop-the-world barrier around GC traversal. Assisted-by: Claude * Serialize fork and GC stop-the-world requesters fork() (posix before/after-fork) and the cycle collector both drive the single StopTheWorldState. With no mutual exclusion their requester word and suspension countdown could interleave and be clobbered, so the completion check never converged and a requester waited on itself forever (reproducible parent-side hang when forking with GC enabled). Add one exclusion held for the whole stop->start span of either requester: stop_the_world acquires it before any stop bookkeeping and start_the_world/reset_after_fork release it. The acquire is park-friendly (poll try-lock and honor a pending suspend between tries) so a requester blocked behind an active stop can still be force-parked instead of deadlocking the active requester. The acquirer holds no other lock while spinning. Also correct the PyRwLock traverse SAFETY note: a failed try_read may mean a force-parked thread holds the write lock; skipping is safe because under-traversal only over-approximates liveness. Document the residual gc.collect()-under-a-non-generation-lock exposure at the collect_inner barrier. Assisted-by: Claude * Add fork under concurrent GC stop-the-world snippet Worker threads allocate cyclic garbage with GC enabled while the main thread forks repeatedly and each child collects. Exercises the fork/GC stop-the-world exclusion; the allocation rate is throttled so a collection stays cheap in unoptimized builds. Assisted-by: Claude * Detach while acquiring the import lock The global import lock is held across bytecode by the importlib bootstrap, so its holder can be parked at a safepoint mid-hold. Acquiring it while attached let another thread block attached on the lock, so a stop-the-world requester could wait forever for that attached thread to suspend while the holder stayed parked. Wrap IMP_LOCK acquisition in allow_threads in both _imp.acquire_lock and the pre-fork acquire_imp_lock_for_fork so the wait honors stop-the-world requests. Correct the acquire_exclusion comment: a spinning requester may hold IMP_LOCK (fork) or the collecting mutex (GC); safety relies on those never being acquired attached-blocking by another thread. Assisted-by: Claude * Add concurrent-import vs GC deadlock snippet Two threads re-import modules (contending the import lock) while a third storms the cycle collector and a fourth allocates cyclic garbage. Regressed as a hang before the import lock acquisition was made park-friendly. Assisted-by: Claude * Create call frames untracked and track generators explicitly Add PyPayload::NEW_REF_UNTRACKED (default false, true for Frame) so PyRef::new_ref skips auto-tracking ordinary call frames in the GC. Generator/coroutine/async-generator frames are tracked explicitly in invoke_with_locals before their generator back-reference is installed. run_frame debug-asserts that a datastack frame is untracked on entry. Assisted-by: Claude * Track escaped call frames lazily at datastack release Rewrite release_datastack_frame around the invariant that a datastack frame is never GC-tracked while it runs: strong_count() == 1 means the frame never escaped, so drop localsplus in place without touching the GC; strong_count() > 1 means it escaped, so materialize localsplus onto the heap and then track the frame. This removes the untrack and the untrack-recheck dance from the common non-escaping path. Assisted-by: Claude * Guard the tracked-frame localsplus invariant Document at Frame::traverse that references to a frame are always recorded as graph edges, so the collector reads a frame's localsplus only when the frame is itself a tracked candidate. Add debug assertions at both frame track sites (escaped datastack frames and generator frames) that a frame is heap-backed before it is tracked, so a collector never reads data-stack-resident, still-mutating storage. Assisted-by: Claude * Reword frame/dealloc invariant comments and drop needless borrow - frame.rs: replace the task codename in the tracked-frame localsplus invariant comment with a description of the collector-vs-executing-frame behavior. - object.rs: remove needless borrow on current_cls in the __class__ assignment compatibility check. - core.rs: rewrite the default_dealloc trashcan/untrack-skip comment to state the actual invariant, which now covers untracked non-escaped frames releasing at interpreter depth with bounded recursion. Assisted-by: Claude * Move type SetAttr slot rewrite inside the type lock and tighten cache reads Run update_slot inside the same with_type_lock transaction as modified_inner and the attributes dict mutation, so the version invalidation, dict change, and slot-table rewrite are published together. Load init_version and getitem_version with Acquire in the specialization cache readers to pair with the Release stores in the writers. Assisted-by: Claude * Capture the type version before reading mutable slots in specializers specialize_load_attr, specialize_store_attr, specialize_to_bool, and the CallAllocAndEnterInit path of specialize_call read the version tag with version_for_specialization before inspecting getattro, setattro, the bool/len slots, and tp_new/tp_alloc, so a concurrent install of the corresponding dunder invalidates the version the specialization is cached against. In the BinaryOpSubscr __getitem__ path, check the HEAPTYPE and eval-frame gates before lookup_ref_and_version_interned, which takes the global type lock and may allocate a version tag. Assisted-by: Claude * Use _get_method_dict for the test_type check in the test runner Replace the direct __func__.__dict__ access with the _get_method_dict helper so plain functions without __func__ do not raise AttributeError. Assisted-by: Claude * Fix lint hooks: cspell dictionary entries, spelling fix, formatting Add qsbr to the rustpython dictionary and reborrows/reparenting to the top-level word list. Rename oldto/newto locals to old_to/new_to. Fix Stabilise -> Stabilize typo in a frame.rs comment. Apply cargo fmt to object.rs, type.rs, and frame.rs, and ruff format/check fixes to two extra_tests snippet files. Assisted-by: Claude * Rebuild all slots for type and descendants on __bases__ reassignment Add PyType::update_all_slots, which invalidates version tags and iterates the full SLOT_DEFS name table calling update_slot::<true> for each distinct name. Unlike init_slots, which is additive and driven only by dunder names present in the current MRO, this resets a slot whose method left the MRO instead of leaving a stale dispatcher pointer. Call it from set_bases in place of the previous modified_inner + init_slots pair, so reassigning __bases__ rebuilds slots for the type and every descendant. Add extra_tests/snippets/type_bases_slot_rebuild.py covering removed-method resets on zelf and deep descendants, the wrong-target switch, __getattr__, the added-method mirror, and a swap-away-and-back round trip. Assisted-by: Claude * Derive the thread-local frame stack from the frame chain Remove the per-VM `frames` Vec. current_frame, sys._getframe, sys._getframemodulename, frame.f_back, sys.monitoring re-instrumentation, gc.get_referrers, faulthandler stack dumps and the post-fork slot rebuild now walk the signal-safe CURRENT_FRAME/`previous` chain instead. Add `Py::from_payload_ptr` to recover a frame object from a chain pointer. The cross-thread `ThreadSlot::frames` registry (sys._current_frames, cross-thread f_back, GC stop-the-world assertion) is unchanged. Assisted-by: Claude * Inherit sub-slot fallback into the field being resolved update_one_slot's number/sequence/mapping fallback inherited via the accessor's default field. Left and right binary ops (add/right_add) share one accessor but occupy distinct fields, so resolving an absent right op or deleting it reset the left op's field, dropping a still-defined __add__ dispatcher. Inherit the exact field under resolution instead. Assisted-by: Claude * Gate type_cache_after_fork to its fork caller and use Self in downcast type_cache_after_fork is only called from the unix fork path, so gate it with all(feature = "host_env", unix) to match the caller and avoid a dead-code error on non-unix targets. Replace the explicit PyType with Self in the modified_inner downcast. Assisted-by: Claude * Unmark test_attr and test_method_call_error in test_monitoring Both TestLoadSuperAttr tests now pass; remove their expectedFailure markers. Assisted-by: Claude * Run tp_new specialization __init__ without a trampoline frame CallAllocAndEnterInit ran __init__ inside a synthetic init-cleanup shim frame whose code carried the __init__ name. Since the thread frame stack is derived from the frame chain, that shim frame was visible to sys._getframe, f_back walks, traceback construction and inspect.stack / inspect.trace. Its code object has empty co_positions, so a stack walk that reached it raised StopIteration inside inspect._get_code_position, cascading into unrelated failures (test_inspect trace/stack/frame, asyncio source traceback). Call __init__ directly via run_frame, enforce the __init__() should return None contract inline, and drop the shim: the init-cleanup code object, its builder, with_frame_untraced, monitoring_disabled_for_code, and the extra-frame datastack/recursion budget. Removing the second frame per construction also cuts the specialization's per-call cost. Assisted-by: Claude * Replace per-call thread-frame mutex with an atomic top-of-stack (unix) On unix threading builds, publish each thread's top Python frame in a single relaxed AtomicPtr store from set_current_frame instead of pushing onto a parking_lot::Mutex<Vec<FramePtr>> per call. Cross-thread readers (sys._current_frames, cross-thread f_back, faulthandler.dump_traceback, the GC unreachable debug-assert) run under stop-the-world and walk the published top frame down the Frame::previous chain; the owning thread is then parked at a safepoint, so the pointer and the frames it reaches are quiescent and alive. The faulthandler watchdog is a plain OS thread that cannot stop-the-world, so it walks the chain lock-free and best-effort. Non-unix threading builds have no stop-the-world and keep the existing mutex-guarded frame stack unchanged. Assisted-by: Claude * Skip exc_info save/restore for callees that never touch the slot with_frame saves and restores the shared exc_info slot around every Python call to contain frames that leave it unbalanced. Cache a has_exc_handling bit on PyCode at creation, set when the bytecode contains any opcode that calls vm.set_exception (PushExcInfo, PopExcept, CheckEgMatch, EndAsyncFor, InstrumentedEndAsyncFor). A callee whose code has none of these cannot mutate the slot, so the save and restore are skipped for it. Generators go through resume_gen_frame and are unaffected. Assisted-by: Claude * Return a write-through FrameLocalsProxy from frame.f_locals Optimized (function) frames now expose `f_locals` as a `FrameLocalsProxy` implementing PEP 667 semantics instead of a cached snapshot dict: - reads go live through the fast-local slots; each access mints a fresh proxy; keys that do not name a fast local are stored in a per-frame `f_extra_locals` side dict and folded into `locals()`. - writes to a fast-local key store into the slot (or its cell) in place; deleting a fast local raises ValueError; extra keys delete normally. - full mapping protocol: keys/values/items (lists), get/pop/setdefault, update (dict or FrameLocalsProxy only), __or__/__ior__/__ror__ (dict result), copy (plain dict), __reduce__ blocks pickling/copy, repr with recursion guard, mapping-pattern and Mapping ABC support. Class/module/exec frames keep returning their namespace mapping directly. Cross-thread access to a frame running on another thread still raises RuntimeError. A closed generator now keeps its frame locals when a durable frame reference escaped (f_locals proxy, sys._getframe, f_back), matching take_ownership; the escape is tracked with a per-frame flag. The snapshot-then-fold locals_to_fast/locals_dirty write-back is retired since proxy writes reach the slots directly. Assisted-by: Claude * Retain the caller frame so f_back resolves after it returns When a frame escapes its execution (referenced through a traceback, `sys._getframe`, `f_locals`, ...), capture a strong reference to its caller at release time. `f_back` consults it once the caller has left the live frame chain, so the Python-visible frame chain survives return. The retained reference is a GC-traversed edge and is cleared by `frame.clear()`, so ancestor chains stay collectable. Assisted-by: Claude * Fix debug-build native stack overflow on deep recursion Make check_c_stack_overflow one-sided (trip whenever the stack pointer is below the soft limit) so a single native frame larger than the margin cannot step past the danger band undetected. Raise the debug STACK_MARGIN_BYTES from 4096 to 16384 words so the margin exceeds a single debug interpreter frame, leaving headroom to raise RecursionError. Release margin unchanged. Clamp the soft-limit margin to half the stack so small explicit thread stacks do not get a soft limit above their stack top. Assisted-by: Claude * Allocate exception instance __dict__ lazily Exception construction went through into_ref_with_type, which eagerly allocated an empty instance dict for every HAS_DICT type. Add into_ref_with_type_lazy_dict, which builds the instance with an unallocated dict slot, and route the four exception construction sites (PyBaseException, PyOSError, OSErrorBuilder, PyBaseExceptionGroup) through it. The dict now materializes on first attribute write or __dict__ access via the existing get_or_insert path. add_note and PyImportError::slot_init now obtain the dict through object_get_dict so they materialize it instead of assuming it exists. A freshly constructed exception no longer reports an empty dict in gc.get_referents, matching the reference interpreter. Assisted-by: Claude * Allocate exception __dict__ lazily in vm.new_exception Route vm.new_exception() through into_ref_with_type_lazy_dict so internally raised exceptions (new_type_error, new_value_error, etc.) start without an instance dict, matching the slot_new path. The dict is materialized on the first attribute write or __dict__ access. Assisted-by: Claude * Validate FrameLocalsProxy.update() arguments Reject keyword arguments and require exactly one positional argument, raising TypeError with the "takes no keyword arguments" and "takes exactly one argument (N given)" messages. Assisted-by: Claude * Address review nits in frame and faulthandler - Drop stale vm.frames reference from the release_datastack_frame uniqueness argument. - Assert has_exc_handling when unwinding an Except-typed stack slot, documenting the invariant that guards the shared exc_info write. - Truncate the specialized __init__ return-type name to 200 chars, matching the unspecialized wrapper. - Reword two comments to describe behavior without prose references to CPython. Assisted-by: Claude * Use scopeguard for start_the_world in faulthandler dump_all_threads Wrap the unix stop-the-world registry walk in a scope with scopeguard::defer! so start_the_world runs on panic, matching the f_back and get_all_current_frames sites. Add scopeguard to the stdlib dependencies. Also correct the restore_exception doc comment to name with_frame after the rename. Assisted-by: Claude * Gitignore docs/superpowers Assisted-by: Claude * Rename type_bases_slot_rebuild.py to builtin_type_bases.py Match the builtin_* naming convention of extra_tests/snippets. Assisted-by: Claude * Apply rustfmt, ruff, and cspell lint fixes Reformat with rustfmt, fix import spacing in builtin_type_bases.py with ruff, and add "pointee" to the cspell word list. Assisted-by: Claude * Gate unix-only QSBR methods to their call sites `online`, `drain_all`, and `reset_after_fork` are called only from unix code (thread attach/detach, post-fork reset), and `offline` from unix code plus a unit test. Gate them with matching cfg so `-D dead_code` does not fire on non-unix targets. Assisted-by: Claude * Visit the function closure tuple as a GC edge PyFunction::traverse visited the cells inside the closure tuple instead of the tuple object itself, so the tuple's reference from the function was never subtracted during cycle collection. A closure tuple that reached back to its function (or, through a frame retained by f_back, to a Thread) was stranded as a false GC root and never collected, leaking the whole cycle. Visit the tuple itself, matching clear(). Assisted-by: Claude * Apply formatting hook fix to builtin_type_bases.py Assisted-by: Claude * Unmark asyncgen finalization-by-gc tests in test_base_events test_asyncgen_finalization_by_gc and test_asyncgen_finalization_by_gc_in_other_thread now pass; GC finalizes the async generators. Assisted-by: Claude * Unmark test_sni_callback_refcycle in test_ssl The servername-callback reference cycle is now collected by GC. Assisted-by: Claude * Widen GC stop-the-world gates from unix-only to all threading builds Change `cfg(all(unix, feature = "threading"))` to `cfg(feature = "threading")` on the stop-the-world machinery so it also compiles and runs on non-unix threading builds: - StopTheWorldState, its stats, stw_trace, and the stop_the_world field - ThreadSlot state/stop_requested/thread fields and their initializers - wait_while_suspended/attach_thread/detach_thread/suspend_if_needed/do_suspend, allow_threads, stop_requested_for_current_thread, and the enter_vm / VmBootstrapGuard / attach_current_thread / release_current_thread / cleanup attach-state wiring - eval_breaker_tripped, check_signals, run_scheduled_gc, signal GC_BIT / schedule_gc / take_gc_scheduled, and the frame.rs safepoint call - CollectStopTheWorld and its use in collect_inner - QSBR::online/offline, now called from attach/detach on all threading builds - debug_assert_current_thread_attached and its type-cache call sites maybe_collect defers auto-collection to the bytecode safepoint on every threading build instead of only unix; non-threading builds keep the inline collect. stw_trace writes to std stderr on non-unix. top_frame publishing (CURRENT_TOP_FRAME_SLOT, set_current_frame), the frame-walk debug assert in collect_inner, and the fork reinit helpers remain unix-only; non-unix keeps ThreadSlot::frames for introspection. Assisted-by: Claude * Detach current thread around blocking _winapi/_overlapped waits Wrap the blocking Windows wait calls in `vm.allow_threads` so the calling thread transitions ATTACHED -> DETACHED for the duration of the wait: - _winapi: WaitForSingleObject, WaitForMultipleObjects, BatchedWaitForMultipleObjects, ConnectNamedPipe, ReadFile, Overlapped.GetOverlappedResult - _overlapped: Overlapped.getresult These previously blocked while ATTACHED, so a stop-the-world requester could never suspend the thread and spun in its wait loop indefinitely. * Format WaitForMultipleObjects allow_threads closure * Treat concurrent sni_callback removal as no-op in invoke_sni_callback * Assert capi refcount on a fresh mortal list instead of the int type object The refcount test asserted exact incref/decref deltas on PyInt's shared type object. That object's reference count is perturbed by the other capi tests running in parallel, and is immortal under some interpreter configurations, so the deltas were not reliably +1/-1. Assert them on a freshly created, uniquely owned list whose reference count is private to the test and mortal. Assisted-by: Claude * Traverse and GC-track PyOSError instances The `#[pyexception]` struct macro now forwards a `traverse` option to the generated `#[pyclass]`, and `ExceptionItemMeta` accepts the `traverse` key. `PyOSError` is marked `traverse = "manual"`, so `HAS_TRAVERSE` is true and OSError-family instances are tracked at creation and traversed by the collector. `PyOSError::traverse` now visits the underlying `PyBaseException` (traceback, cause, context, args) instead of `PyException::try_traverse`, which was a no-op because `PyException` has `HAS_TRAVERSE = false`. * Unmark test_blockingioerror in test_io The BlockingIOError reference cycle is now collected by GC. Assisted-by: Claude
Introduce vm.state.type_mutex to serialize type mutation and version-tag assignment, matching CPython's free-threading model.
Summary by CodeRabbit
Bug Fixes
Performance
Chores