Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Feb 3, 2026

GC bit operations (_PyObject_GC_TRACK/UNTRACK equivalent):

  • Add set_gc_bit() helper for atomic GC bit manipulation
  • Add set_gc_tracked() / clear_gc_tracked() methods
  • Update is_gc_tracked() to use GcBits::TRACKED flag
  • Call set_gc_tracked() in track_object()
  • Call clear_gc_tracked() for static types (they are immortal)

tp_clear infrastructure (for breaking reference cycles):

  • Add try_clear_obj() function to call payload's try_clear
  • Add clear field to PyObjVTable
  • Add clear() method to PyType's Traverse impl

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced garbage collection system to more effectively detect and break circular references, improving memory management and reducing potential memory leaks.
    • Improved object deallocation process to ensure proper cleanup of child references before memory is freed.
    • Strengthened object tracking mechanisms to better identify objects requiring garbage collection.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

This PR introduces garbage collection cycle detection and breaking mechanisms for RustPython's VM. It adds a clear traversal hook to extract child references from objects, implements GC bit tracking operations on PyObject, integrates automatic object tracking during creation, and modifies deallocation to run tp_clear before freeing memory.

Changes

Cohort / File(s) Summary
GC Traversal Clear Hook
crates/vm/src/builtins/type.rs, crates/vm/src/object/traverse_object.rs
Introduces clear callback infrastructure to PyObjVTable with optional clear field; PyType now implements a traversal hook to collect referenced objects during GC clearing.
GC Bit Management & Deallocation
crates/vm/src/object/core.rs
Adds atomic GC bit operations (set_gc_bit, set_gc_tracked, clear_gc_tracked); implements try_clear_obj to extract child references; modifies default_dealloc to invoke tp_clear before memory release and extract collected edges.
Object Tracking Integration
crates/vm/src/gc_state.rs, crates/vm/src/object/core.rs
Calls set_gc_tracked() during object tracking in gc_state::track_object; extends PyRef<T>::new_ref to automatically track objects in GC based on traversal capability, instance dict presence, and heap-type status; explicitly untracks weakref_type after initialization.

Sequence Diagram

sequenceDiagram
    participant Code as Client Code
    participant PyRef as PyRef Creation
    participant PyObj as PyObject
    participant GCState as GC State
    participant Dealloc as Deallocation
    
    Code->>PyRef: new_ref(payload, type, dict)
    PyRef->>PyObj: Create with payload
    PyRef->>GCState: Check traversal capability
    alt Has Traverse + Dict/HeapType
        PyRef->>GCState: track_object()
        GCState->>PyObj: set_gc_tracked()
        PyObj->>PyObj: Mark GC bit
    end
    
    Note over GCState: GC Cycle Detection Triggered
    GCState->>PyObj: Invoke clear callback
    PyObj->>PyObj: Call tp_clear (via try_clear_obj)
    PyObj-->>GCState: Return child references
    GCState->>GCState: Process references for cycles
    
    Code->>Dealloc: Object deallocation
    Dealloc->>PyObj: default_dealloc()
    Dealloc->>PyObj: tp_clear() if not cleared
    Dealloc->>PyObj: Extract collected edges
    Dealloc->>Dealloc: Free memory
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • ShaharNaveh

Poem

🐰 A rabbit's ode to cycles broke,
With clear hooks and GC strokes,
Bits are tracked with care so fine,
Memory freed in perfect line,
No more cycles to provoke!

🚥 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 accurately summarizes the main changes: adding GC infrastructure including tracking bits and tp_clear functionality across multiple files.
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.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

GC bit operations (_PyObject_GC_TRACK/UNTRACK equivalent):
- Add set_gc_bit() helper for atomic GC bit manipulation
- Add set_gc_tracked() / clear_gc_tracked() methods
- Update is_gc_tracked() to use GcBits::TRACKED flag
- Call set_gc_tracked() in track_object()
- Call clear_gc_tracked() for static types (they are immortal)

tp_clear infrastructure (for breaking reference cycles):
- Add try_clear_obj() function to call payload's try_clear
- Add clear field to PyObjVTable
- Add clear() method to PyType's Traverse impl
@youknowone youknowone marked this pull request as ready for review February 3, 2026 10:30
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@crates/vm/src/builtins/type.rs`:
- Around line 424-427: When marking static types immortal, in addition to
calling new_type.as_object().clear_gc_tracked() you must also remove the object
from the VM's GC tracking sets in GcState so it no longer appears in
gc.get_objects or generation counts; update the same block (the one handling
static types) to call the same removal logic used for weakref_type (i.e., remove
the object from the tracking sets / GcState tracking collections) immediately
after clear_gc_tracked() so is_gc_tracked() and the GC sets stay consistent.

Comment on lines +424 to +427
// Static types are not tracked by GC.
// They are immortal and never participate in collectable cycles.
new_type.as_object().clear_gc_tracked();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Untrack static types from GC state as well.

Clearing only the TRACKED bit leaves the type in GcState sets, so gc.get_objects/generation counts can still include it and diverge from is_gc_tracked(). Please also remove it from the tracking sets (similar to weakref_type).

💡 Suggested fix
-        new_type.as_object().clear_gc_tracked();
+        let obj = new_type.as_object();
+        unsafe { crate::gc_state::gc_state().untrack_object(NonNull::from(obj)); }
+        obj.clear_gc_tracked();
📝 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.

Suggested change
// Static types are not tracked by GC.
// They are immortal and never participate in collectable cycles.
new_type.as_object().clear_gc_tracked();
// Static types are not tracked by GC.
// They are immortal and never participate in collectable cycles.
let obj = new_type.as_object();
unsafe { crate::gc_state::gc_state().untrack_object(NonNull::from(obj)); }
obj.clear_gc_tracked();
🤖 Prompt for AI Agents
In `@crates/vm/src/builtins/type.rs` around lines 424 - 427, When marking static
types immortal, in addition to calling new_type.as_object().clear_gc_tracked()
you must also remove the object from the VM's GC tracking sets in GcState so it
no longer appears in gc.get_objects or generation counts; update the same block
(the one handling static types) to call the same removal logic used for
weakref_type (i.e., remove the object from the tracking sets / GcState tracking
collections) immediately after clear_gc_tracked() so is_gc_tracked() and the GC
sets stay consistent.

@youknowone youknowone merged commit f71fe9b into RustPython:main Feb 3, 2026
13 checks passed
@youknowone youknowone deleted the gc-infra branch February 3, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant