impl gc finialized #6689

youknowone · 2026-01-10T08:28:26Z

helps #6410

Summary by CodeRabbit

Release Notes

Chores
- Improved garbage collection infrastructure with enhanced object finalization state tracking mechanisms to prevent duplicate cleanup operations and ensure proper resource lifecycle management across the system.
- Strengthened memory management safety and stability for concurrent execution environments by implementing additional safeguards, state validation, and improved control flow during object deallocation and cleanup processes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-10T08:28:31Z

Warning

Rate limit exceeded

@youknowone has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 28 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between daec095 and 9b3532d.

📒 Files selected for processing (1)

crates/vm/src/object/core.rs

📝 Walkthrough

Walkthrough

This PR introduces a new garbage collection bits mechanism for free-threading by adding a GcBits bitflags type, extending PyInner<T> with a gc_bits field, implementing finalized-state tracking methods, and modifying the destructor to guard __del__ invocation with a finalized check.

Changes

Cohort / File(s)	Summary
GC State Tracking System `crates/vm/src/object/core.rs`	Added `GcBits` bitflags type with constants (TRACKED, FINALIZED, UNREACHABLE, FROZEN, SHARED, SHARED_INLINE, DEFERRED); extended `PyInner<T>` with public `gc_bits: PyAtomic<u8>` field; implemented `gc_finalized()` and `set_gc_finalized()` methods on `PyObject`; modified `drop_slow_inner()` destructor to check finalized state before invoking `__del__` slot and set FINALIZED bit afterward; initialized `gc_bits` in all `PyInner<T>` and `PyType` payload constructors

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Bits and flags now track the way,
Objects know when they must decay,
No more __del__ runs twice or thrice,
Garbage collection, oh so nice! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'impl gc finialized' directly relates to the main change (implementing GC finalization mechanism), but contains a typo ('finialized' instead of 'finalized') and lacks clarity about the specific implementation details.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @crates/vm/src/object/core.rs:
- Around line 102-125: The set_gc_finalized() implementation does a non-atomic
read-modify-write using load + store on self.0.gc_bits which can lose concurrent
updates; replace that logic with an atomic fetch_or using
self.0.gc_bits.fetch_or(GcBits::FINALIZED.bits(), Relaxed) to set the finalized
bit safely. Also, if GcBits is not part of the VM public API, reduce its
visibility from pub to pub(crate) to limit its scope.
- Around line 811-829: The current set_gc_finalized implementation does a
non-atomic load then store and can clobber concurrent GC bit updates; replace
the load+store sequence in set_gc_finalized with an atomic read-modify-write
using gc_bits.fetch_or(..., Ordering::Relaxed) to set GcBits::FINALIZED without
losing other bits (use the same Ordering::Relaxed used in gc_finalized),
referencing the GcBits bitmask and the gc_bits field on the same struct so the
operation atomically ORs the FINALIZED bit in place.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c15799 and daec095.

📒 Files selected for processing (1)

crates/vm/src/object/core.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/object/core.rs

🔇 Additional comments (4)

crates/vm/src/object/core.rs (4)

472-488: gc_bits initialization to 0 is correct and keeps construction straightforward.
No concerns with this initialization site (assuming other PyInner constructors/initializers are updated similarly, which init_type_hierarchy is).

859-868: Finalized-guarded __del__ call is a good CPython-aligned safety net; make it rely on atomic RMW.
The control-flow looks right (mark finalized before calling __del__ to prevent reentrancy/double-finalize), but it depends on set_gc_finalized() being an atomic OR (see prior comment).

1324-1347: Type bootstrap: gc_bits included in partially_init! is correct.
This keeps the “all fields explicit” invariant for the unsafe bootstrap; good catch updating both type_type_ptr and object_type_ptr initializers.

131-145: No layout validation issues found. The gc_bits field addition is safe because:

PyInner<T> has #[repr(C)], ensuring stable and well-defined memory layout

All offset and size calculations use compile-time macros (offset_of!, size_of!) that automatically adjust to struct changes; no hard-coded offsets exist

SIZEOF_PYOBJECT_HEAD is computed via size_of::<PyInner<()>>(), which correctly includes the new field

gc_bits access patterns use proper atomic operations (load(Relaxed)), safe for concurrent access

The concern about implicit offset/layout assumptions is not applicable here.

crates/vm/src/object/core.rs

github-actions · 2026-01-10T09:52:42Z

Code has been automatically formatted

The code in this PR has been formatted using cargo fmt --all.
Please pull the latest changes before pushing again:

git pull origin gc-finalized

* impl gc finialized * Auto-format: cargo fmt --all --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

youknowone marked this pull request as ready for review January 10, 2026 09:11

coderabbitai bot reviewed Jan 10, 2026

View reviewed changes

crates/vm/src/object/core.rs Show resolved Hide resolved

crates/vm/src/object/core.rs Show resolved Hide resolved

impl gc finialized

7f1a713

youknowone force-pushed the gc-finalized branch from daec095 to 7f1a713 Compare January 10, 2026 09:52

Auto-format: cargo fmt --all

9b3532d

youknowone merged commit 9e225f5 into RustPython:main Jan 10, 2026
12 of 13 checks passed

youknowone deleted the gc-finalized branch January 10, 2026 10:50

terryluan12 pushed a commit to terryluan12/RustPython that referenced this pull request Jan 15, 2026

impl gc finialized (RustPython#6689)

f43bcbf

* impl gc finialized * Auto-format: cargo fmt --all --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

impl gc finialized #6689

impl gc finialized #6689

Uh oh!

youknowone commented Jan 10, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 10, 2026 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

impl gc finialized #6689

impl gc finialized #6689

Uh oh!

Conversation

youknowone commented Jan 10, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Jan 10, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 10, 2026 •

edited

Loading