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.