Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/common/src/refcount.rs`:
- Around line 4-6: The layout comment mentions a "[1 bit: weaked]" flag that has
no corresponding implementation; either remove that token from the comment to
match the current code or add the missing bit definition and accessors. To fix,
decide whether to actually reserve the bit: if unused, update the comment to
drop "weaked" and adjust bit position explanations (remove any reference to
WEAKED/State::weaked); if intended, add a WEAKED constant (e.g., WEAKED_MASK)
and implement State::weaked(&self) -> bool plus setter/clearer logic where State
bits are manipulated (matching how destructed/leaked are handled), and ensure
weak_count/strong_count masks/shift constants are unchanged. Use existing
symbols like State, destructed, leaked, weak_count, strong_count, and follow the
pattern for other flag constants so the comment and code remain consistent.
- Around line 144-153: The dec() method currently does an unconditional
self.state.fetch_sub(COUNT, Ordering::SeqCst) then checks
State::from_raw(...).leaked(), which allows underflow to clear the LEAKED bit;
fix by detecting leaked() after the fetch and immediately restoring the
subtraction (call self.state.fetch_add(COUNT, Ordering::SeqCst)) before
returning false so the LEAKED flag and count aren't corrupted; update dec() to
perform the undo restore when old.leaked() is true and then return false
(optionally note you could replace with a CAS loop to avoid the transient
window).
- Around line 179-193: The deferred-drop thread-local and APIs must be compiled
only when the standard library is available: wrap the thread_local! block
(IN_DEFERRED_CONTEXT and DEFERRED_QUEUE), the DeferredDropGuard struct, and the
functions with_deferred_drops, try_defer_drop, and flush_deferred_drops with
#[cfg(feature = "std")] so they are excluded from no_std builds; ensure the
attribute is applied to each item (or to a surrounding mod) referencing those
symbols so the crate compiles without std just like refcount_overflow() does.
- Around line 99-110: Update the doc comment and safety semantics for pub fn
inc(&self) in refcount.rs: clarify that inc() assumes the caller already holds a
valid strong reference (e.g., used in to_owned()) and is not a resurrection path
— recommend safe_inc() for resurrecting from weak refs; replace the vague
comment "// The previous fetch_add created a permission to run decrement again"
with a brief protocol description (double fetch_add is an optimization to avoid
CAS when caller already had a strong ref and may race with a destructor that is
already in-flight), and make overflow handling consistent with inc_by() by
adding the same strong-count overflow check (use State::strong()/STRONG logic
and call refcount_overflow() on overflow) or, if you intentionally omit overflow
detection, document why it is safe to omit it; reference the symbols inc,
safe_inc, inc_by, State, COUNT, STRONG, destructed, and refcount_overflow in the
change.

In `@crates/vm/src/gc_state.rs`:
- Around line 439-444: The unconditional reset of generations[0].count loses
increments from concurrent track_object calls after gen_locks are dropped;
change the logic in the collection exit paths (where
generations[0].count.store(0, ...) is called) to either (A) switch maybe_collect
to use alloc_count as the authoritative allocation counter and reset alloc_count
there instead of touching generations[0].count, ensuring track_object increments
alloc_count, or (B) recompute gen0's true count after promotions by reading
generation_objects[0] (via its read lock) and store its length into
generations[0].count before releasing locks; update all early-return branches
(the places with generations[0].count.store(0, ...)) to use the chosen approach
so concurrent increments are not lost.
- Around line 734-758: promote_survivors currently removes an object from its
source generation and then silently skips adding it to next_gen if acquiring the
write lock for self.generation_objects[next_gen] fails, orphaning the object;
change the target-generation lock acquisition to propagate a poisoned-lock panic
instead of silently failing by replacing the conditional try-lock for next_gen
with an unconditional write() followed by .expect(...) (e.g. let mut next_set =
self.generation_objects[next_gen].write().expect("poisoned
generation_objects[next_gen] lock");) and then perform next_set.insert(ptr) and
the fetch_add on self.generations[next_gen].count, so that a poisoned lock
triggers a panic and the invariant that every tracked object is in exactly one
generation is preserved.

In `@crates/vm/src/vm/mod.rs`:
- Line 880: The doc comment above the block that clears the `sys` and `builtins`
dicts is mislabeled as "Phase 6" but `finalize_modules` actually treats that
block as Phase 7; update the comment to read "Phase 7" (or match the existing
phase numbering used in `finalize_modules`) so the phase labels are consistent
with the earlier Phase 6 GC collection and the surrounding phase comments.

---

Outside diff comments:
In `@crates/vm/src/vm/interpreter.rs`:
- Around line 386-395: Update the "Finalization steps" doc comment so it matches
the actual shutdown sequence in this file: enumerate the real order (flush
stdout/stderr via flush_std, handle exit exception -> exit code, call
threading._shutdown to wait for threads, set the finalizing flag (suppresses
unraisable exceptions), run atexit exit functions, run GC collect_force (where
the doc previously claimed atexit ran elsewhere), call finalize_modules for
module cleanup, and note any final flush/GC behavior; remove the duplicate
threading._shutdown step and correct the step labels to reference the actual
calls (threading._shutdown, atexit, collect_force, finalize_modules, flush_std)
so the numbered list matches the code flow.

In `@crates/vm/src/vm/mod.rs`:
- Around line 1122-1149: The defer cleanup (scopeguard::defer!) must be created
before running the trace/execution so panics still trigger cleanup; move the
scopeguard::defer! block to just after setting up the
frame/owner/exception/recursion state and before calling
self.trace_event(TraceEvent::Call, None) and f(frame). Ensure TraceEvent::Return
is still emitted on successful result (keep the existing trace_event call after
f(frame)), and preserve the same cleanup actions inside the guard
(frame.owner.store(...), self.exceptions.borrow_mut().stack.pop(),
crate::vm::thread::set_current_frame(old_frame), self.frames.borrow_mut().pop(),
cfg thread pop_thread_frame(), and self.recursion_depth.update(|d| d - 1)) so
any panic during trace_event or f(frame) will run the guard.

---

Nitpick comments:
In `@crates/common/src/refcount.rs`:
- Around line 43-46: The code does unnecessary arithmetic because COUNT is 1;
simplify by removing the redundant / COUNT in strong() and * COUNT in
add_strong(): in the strong() method use ((self.inner & STRONG) as u32) (or
equivalent masking) to extract the strong count directly, and in add_strong()
add the increment directly to the strong bits instead of multiplying by COUNT;
update any related bit-manipulation comments to reflect the simplification and
keep the STRONG and COUNT constants as needed (or remove COUNT if unused).
- Around line 250-252: The #[inline] attribute on flush_deferred_drops is
misleading because the function always allocates a new Vec when draining,
preventing meaningful inlining; remove the #[inline] on the flush_deferred_drops
function or restructure it so the cheap fast-path (e.g., an early is_empty check
on the deferred queue) is performed in a small #[inline] helper that returns
early while the heavier Vec-allocating work remains in a non-inlined function;
locate flush_deferred_drops and either drop the attribute or split out the
empty-queue check into an inline helper to preserve the intended optimization.

In `@crates/vm/src/gc_state.rs`:
- Around line 464-480: The code currently calls unsafe
obj.gc_get_referent_ptrs() twice per object; change to compute and store each
object's adjacency once and reuse it: during the initial loop over collecting
(the block that iterates over collecting, reads obj.gc_get_referent_ptrs(), and
updates gc_refs) build a HashMap<GcObjectPtr, Vec<GcObjectPtr>> (or similar)
keyed by gc_ptr and populate it from obj.gc_get_referent_ptrs(); then in the
later reachability traversal (the code that walks reachable objects) read
referents from that cache instead of calling gc_get_referent_ptrs() again.
Update lookups to use the cache where you currently call gc_get_referent_ptrs(),
and ensure you still respect the same alive/strong_count checks and membership
tests against collecting and gc_refs.
- Around line 598-614: Consolidate the separate read and write lock around
finalized_objects into a single write-lock per object: in the loop over
unreachable_refs (using GcObjectPtr), acquire self.finalized_objects.write()
once, check if the pointer is already present, and if not insert it; then drop
the write lock and call obj_ref.try_call_finalizer() only when you performed the
insert. This removes the TOCTOU window with untrack_object while preserving the
behavior that the object is marked finalized before __del__ runs
(is_finalized()/mark_finalized semantics).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/common/src/refcount.rs`:
- Around line 120-138: safe_inc currently increments without checking
strong-count overflow and can wrap into weak bits; before attempting the
compare_exchange in safe_inc, detect the overflow the same way inc_by does: read
old via State::from_raw(self.state.load(...)), and if adding 1 would exceed the
maximum strong count (e.g., compare old.strong() against State's max or check
that old.add_strong(1) would overflow/wrap), return false; only proceed to
compute new_state = old.add_strong(1) and call
self.state.compare_exchange(old.as_raw(), new_state.as_raw(), ...) when the
increment is safe and old.destructed() is false. Ensure you reference
State::from_raw, destructed, add_strong, as_raw and the existing
compare_exchange call when making the change.

---

Outside diff comments:
In `@crates/vm/src/vm/interpreter.rs`:
- Around line 385-395: Update the finalization doc comment in vm::interpreter.rs
(the block describing the steps for finalizing the VM—see the comment above the
finalize function) to match the current implementation: remove the duplicated
threading._shutdown() step and remove the “Final GC pass” entry after
finalize_modules, and reorder or renumber the listed steps so they accurately
reflect the actual sequence (flush stdout/stderr; handle exit exception; wait
for thread shutdown/set finalizing flag/call threading._shutdown to join
non-daemon threads; run atexit functions; GC pass and module cleanup via
finalize_modules). Ensure the comment references threading._shutdown and
finalize_modules exactly as named.

---

Duplicate comments:
In `@crates/common/src/refcount.rs`:
- Around line 98-108: inc() must explicitly prevent strong-count overflow and
clarify the safety assumption; replace the current blind fetch_add pattern with
a compare-and-swap loop (e.g., using fetch_update or compare_exchange) on
self.state to read current State::from_raw, verify !val.destructed() and that
val.strong() + COUNT does not exceed the strong-capacity (call
refcount_overflow() if it would), then atomically store the incremented raw
state; also add an explicit comment or debug_assert in inc() that the caller
already holds a strong reference (the function relies on that invariant) and
remove the ambiguous double-fetch_add; keep references to State::from_raw,
COUNT, self.state, destructed(), strong(), and refcount_overflow() so reviewers
can locate the change.

In `@crates/vm/src/gc_state.rs`:
- Around line 733-755: promote_survivors currently removes `ptr` from the source
generation (`generation_objects[gen_idx]`) before trying to acquire the target
generation write lock (`generation_objects[next_gen].write()`), so if the
next-gen lock acquisition fails (poisoned) the object is lost; change the logic
in `promote_survivors` to either acquire the `next_gen` write lock before
removing from the source, or if you must remove first then handle the `Err` from
`generation_objects[next_gen].write()` by reinserting `ptr` into the original
`gen_set` and restoring the corresponding `generations[gen_idx].count` (and
return/continue) so the object is not orphaned; reference the symbols
`promote_survivors`, `generation_objects`, `generations`, `from_gen`,
`next_gen`, `gen_set`, `next_set`, and `ptr` when making the change.
- Around line 439-444: The unconditional self.generations[0].count.store(0,
Ordering::SeqCst) can clobber increments from concurrent track_object() calls;
remove the unconditional reset in early-return paths (e.g., the branch where
collecting.is_empty()) and only adjust the atomic counter when you actually
remove objects — perform an atomic adjustment (use
AtomicUsize::fetch_sub(amount, Ordering::SeqCst) when you know how many were
collected, or AtomicUsize::swap(0, Ordering::SeqCst) to atomically
grab-and-reset) in the collection code paths (referencing generations[0].count,
track_object(), and the methods that call update_stats) so concurrent increments
aren’t lost.

---

Nitpick comments:
In `@crates/vm/src/signal.rs`:
- Around line 66-75: The current code holds an immutable RefCell borrow from
vm.signal_handlers.get().unwrap().borrow() across the Python call
(callable.invoke), which can panic if the handler calls signal.signal() (a
borrow_mut); fix by snapshotting the handlers to invoke and dropping the RefCell
borrow before any Python invocation: iterate TRIGGERS and for each triggered
entry, read and clone/cloneable-convert the handler reference (or store
indices/signum + handler.to_callable() results) into a local Vec (e.g.,
Vec<(usize, Callable)>) while the borrow is held, then drop the borrow and loop
over that Vec to call callable.invoke((signum, vm.ctx.none()), vm) so no RefCell
borrow is held during invocation; reference functions/fields:
vm.signal_handlers.get(), signal_handlers.borrow(), TRIGGERS, trigger.swap,
handler.to_callable(), and callable.invoke.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/vm/src/vm/mod.rs`:
- Around line 824-825: The doc comment above the module-dict clearing block is
mislabeled "Phase 4" while finalize_modules runs it as Phase 5; update the
comment to read "Phase 5: Clear module dicts in reverse import order. Skip
builtins and sys — those are cleared last in Phase 7." so the phase numbering
matches finalize_modules/GC sequencing.
- Around line 723-733: Update the doc comment above the method
finalize_clear_module_dicts to correct the phase label from "Phase 4" to "Phase
5" so it matches the sequencing in finalize_modules (where the first GC collect
is Phase 4 and the dict-clearing pass is Phase 5); locate the comment that
currently precedes the finalize_clear_module_dicts function and change the phase
number text only, leaving the rest of the explanation intact.

---

Duplicate comments:
In `@crates/common/src/refcount.rs`:
- Around line 120-139: safe_inc() can overflow the strong count and corrupt WEAK
bits because it computes new_state = old.add_strong(1) without checking limits;
before calling State::add_strong or performing compare_exchange, check that
old.strong() < STRONG (the max strong sentinel) and return false if not, so you
never attempt to increment past the strong limit; update the logic in safe_inc
(referencing safe_inc, State::strong(), State::add_strong(), STRONG and the
compare_exchange on self.state) to perform this guard and only compute/add the
strong count when it's safe.
- Around line 98-109: inc() currently increments the strong count
unconditionally and can overflow into the weak bits; mirror the overflow guard
used in inc_by(): read State::from_raw(self.state.fetch_add(COUNT,
Ordering::SeqCst)) into val, check if (val.strong() as usize) > STRONG - 1 and
call refcount_overflow() before performing the increment (or bail), and keep the
existing destructed() and permission re-grant logic (the second
self.state.fetch_add(COUNT, Ordering::SeqCst)) unchanged so the function
enforces the same strong-count ceiling as inc_by(); reference functions/values:
inc(), inc_by(), State, STRONG, COUNT, refcount_overflow, and
self.state.fetch_add.
- Around line 141-152: The current dec() unconditionally calls
self.state.fetch_sub(COUNT) which can underflow and clear the LEAKED bit;
replace that unconditional subtraction with an atomic CAS loop that first reads
state via self.state.load(Ordering::SeqCst), checks
State::from_raw(raw).leaked() and returns false if leaked, and otherwise
attempts a compare_exchange_weak from raw to raw - COUNT (using
Ordering::SeqCst) until it succeeds, then compute and return whether the
previous strong count was 1 (use State::from_raw(raw).strong() == 1); update
references to self.state.fetch_sub(COUNT) and keep using State::from_raw,
leaked(), and LEAKED in the logic.

In `@crates/vm/src/vm/mod.rs`:
- Line 880: The Phase 7 label comment in vm::mod.rs (the doc/comment "Phase 7:
Clear sys and builtins dicts last.") has been corrected; no code changes are
required—keep the existing comment as-is and approve the change around the Phase
7 section in the VM teardown logic (look for the Phase 7 label near the VM
cleanup functions in vm::mod.rs).

---

Nitpick comments:
In `@crates/common/src/refcount.rs`:
- Around line 93-96: Add the #[must_use] attribute to the reference-counting
methods that return a bool so callers cannot accidentally ignore their result:
annotate the dec() and safe_inc() methods (and any other overloads/variants of
these methods in the same module/class) with #[must_use] to force checking the
returned boolean; locate the implementations by name (dec and safe_inc in the
RefCount/State impl) and add the attribute above their signatures.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/vm/src/object/core.rs`:
- Around line 128-133: The end() function can underflow the per-thread
DEALLOC_DEPTH by doing d.get() - 1; change the decrement to a safe operation
(e.g., use saturating_sub or checked_sub with a debug_assert) so depth never
wraps to usize::MAX, then set the new depth and return it; reference
DEALLOC_DEPTH and end(), and keep begin() semantics intact while optionally
adding a debug_assert that ensures a matching begin() existed (and/or log an
error) instead of allowing silent release-mode wraparound that trips
TRASHCAN_LIMIT logic.

---

Duplicate comments:
In `@crates/common/src/refcount.rs`:
- Around line 148-157: The dec() method unconditionally calls
self.state.fetch_sub(COUNT, Ordering::SeqCst) before checking
State::from_raw(...).leaked(), which can underflow the packed counter for LEAKED
states; restore the prior value if leaked() is true by re-adding COUNT (e.g.
call self.state.fetch_add(COUNT, Ordering::SeqCst)) and return false, so update
dec() to fetch_sub, compute old = State::from_raw(...), check old.leaked() and
if true call fetch_add(COUNT) to undo the subtraction and return false; keep the
final return old.strong() == 1 as-is.
- Around line 98-109: The inc() implementation must add the same strong-count
overflow guard used by inc_by(): after computing val =
State::from_raw(self.state.fetch_add(COUNT, ...)) check whether (val.strong() as
usize) > STRONG - 1 (i.e. would overflow by adding 1) and call
refcount_overflow() if so, then preserve the existing destructed() check and the
conditional extra fetch_add for when val.strong() == 0; update function inc() to
mirror inc_by()’s overflow logic using the same COUNT, STRONG and State symbols.

In `@crates/vm/src/vm/mod.rs`:
- Around line 880-884: The phase label in the finalize_clear_sys_builtins_dict
function comment has been corrected to "Phase 7" already; no code changes are
required—leave the comment and the function
Self::module_clear_dict(&self.sys_module.dict(), self) and
Self::module_clear_dict(&self.builtins.dict(), self) as-is in
finalize_clear_sys_builtins_dict.

---

Nitpick comments:
In `@crates/common/src/refcount.rs`:
- Around line 258-270: flush_deferred_drops currently drains DEFERRED_QUEUE into
a Vec and calls each op, but if any op() panics the remaining closures are
dropped and never run (leading to GC stale pointers); fix by making the drain
panic-safe: wrap each Box<dyn FnOnce()> into an RAII wrapper (e.g., struct
CallOnDrop(Option<Box<dyn FnOnce()>>) with a Drop impl that calls the closure if
still present), change the drain to map each closure into CallOnDrop(Some(f))
and collect that Vec, then iterate the Vec and take().call each wrapper to run
closures normally—any closures not yet taken will be invoked from
CallOnDrop::drop during panic unwinding, ensuring no deferred drop is silently
skipped.
- Around line 43-45: The strong() method performs an unnecessary division by
COUNT; update the strong function (fn strong(self) -> u32) to compute
((self.inner & STRONG) as u32) (or equivalent masking and casting) without
dividing by COUNT since COUNT == 1, and ensure the result is still cast to u32;
reference the symbols strong, self.inner, STRONG and COUNT when making the
change.

In `@crates/vm/src/object/core.rs`:
- Around line 434-452: Between releasing and re-acquiring the weakref stripe
lock (weakref_lock::lock) the code unconditionally treats the new weakref as
generic and overwrites self.generic, which can allow two generic nodes; after
re-acquiring the lock (before setting self.generic and self.head) re-check
whether a generic already exists (inspect self.generic/head via NonNull::new or
equivalent) and if one is present avoid marking this node as the generic
head—insert it as a normal/basic weakref (update pointers accordingly) or update
the existing generic pointer only if still None; ensure the check uses the same
pointer types/ordering as the current insertion logic (node_ptr,
WeakLink::pointers, self.head, self.generic) to preserve atomic consistency.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/common/src/refcount.rs`:
- Around line 208-219: The Drop impl for DeferredDropGuard currently always
calls flush_deferred_drops() which can panic during unwinding; update
DeferredDropGuard::drop to check std::panic::panicking() (or equivalent) after
restoring IN_DEFERRED_CONTEXT.with(|in_ctx| in_ctx.set(self.was_in_context));
only call flush_deferred_drops() when !self.was_in_context &&
!std::panic::panicking() so flush is skipped during unwinding; keep restoring
IN_DEFERRED_CONTEXT unconditionally and do not change other logic.
- Around line 262-270: flush_deferred_drops currently drains DEFERRED_QUEUE and
calls each op, but if any op panics the rest are lost and callers can invoke it
while IN_DEFERRED_CONTEXT is true; update flush_deferred_drops to first assert
(or debug_assert) that IN_DEFERRED_CONTEXT is false to prevent reentrancy, then
take the ops vec and execute each op inside std::panic::catch_unwind so a panic
from one op does not skip others, and if a panic occurs re-push any remaining
ops back into DEFERRED_QUEUE (using DEFERRED_QUEUE.with) before resuming the
panic unwind; reference flush_deferred_drops, DEFERRED_QUEUE,
IN_DEFERRED_CONTEXT and ensure any untrack_object semantics are preserved by
requeuing unexecuted closures.

In `@crates/vm/src/object/core.rs`:
- Around line 441-449: In the re-check path where you return a resurrected
generic (the block using is_generic, generic_ptr, generic and calling
generic.0.ref_count.safe_inc()), clear the weak's wr_object before the early
return so drop of the local weak won't call unlink_weakref on an uninserted
node; i.e., set the weak/inner wr_object to null (use the same atomic store
semantics as other places, e.g., Ordering::Relaxed) on generic.0 (the PyWeak
inner) immediately prior to returning PyRef::from_raw(generic_ptr).

---

Duplicate comments:
In `@crates/common/src/refcount.rs`:
- Around line 148-157: The dec() implementation unconditionally does
self.state.fetch_sub(COUNT, Ordering::SeqCst), which can underflow and clear the
LEAKED bit; change dec() (and the fetch_sub usage) to use a CAS loop that loads
State::from_raw(self.state.load(...)), checks state.leaked() and if leaked
returns false without performing the subtraction, otherwise attempts a
compare_exchange to subtract COUNT (or retry on failure); ensure the code
references the COUNT constant and uses State::from_raw and leaked() to detect
and skip mutation when LEAKED is set so the LEAKED bit is never cleared by
underflow.

In `@crates/vm/src/object/core.rs`:
- Around line 128-135: The current debug_assert in end() (inside
DEALLOC_DEPTH.with in pub(super) unsafe fn end) prevents usize underflow only in
debug builds; change it to a runtime check so release builds can't underflow:
read d.get() into a local, if it's zero call panic! with "trashcan::end called
without matching begin" (or use checked_sub(1).unwrap_or_else(|| panic!(...)))
before calling d.set and returning the decremented depth. Ensure the check and
decrement happen inside the same DEALLOC_DEPTH.with closure so you update the
same thread-local value.

---

Nitpick comments:
In `@crates/common/src/refcount.rs`:
- Around line 94-96: The get() method currently uses Ordering::SeqCst for the
atomic load while is_leaked() uses Ordering::Acquire, creating inconsistent read
orderings; change the ordering in get() (the call to self.state.load and
subsequent State::from_raw(...).strong()) to use the same weaker ordering as
is_leaked() (e.g., Ordering::Acquire, or Ordering::Relaxed if this is purely a
diagnostic read) so both reads are consistent.
- Around line 43-45: The State::strong() method performs a redundant division by
COUNT (which equals 1) making "((self.inner & STRONG) / COUNT) as u32" a no-op;
simplify the expression by removing the "/ COUNT" so the method returns
"(self.inner & STRONG) as u32" (referencing State::strong, the inner field,
STRONG and COUNT) to make the intent clearer.

In `@crates/vm/src/object/core.rs`:
- Around line 157-166: Move the trashcan check before running finalizers: call
trashcan::begin(obj, default_dealloc::<T>) at the top of default_dealloc and if
it returns false, return immediately (so the object is queued) instead of
running drop_slow_inner; only when trashcan::begin returns true should you call
drop_slow_inner and later call trashcan::end (so objects deferred by
trashcan::begin are queued before any finalizer/weakref work and drop_slow_inner
is executed exactly once when processed).