Skip to content

fix asyncgen finalizer #6761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
youknowone merged 1 commit into from
Jan 18, 2026
Merged

fix asyncgen finalizer #6761

youknowone merged 1 commit into from
Jan 18, 2026
+12 −3

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 17, 2026 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed async generator finalization to ensure finalizers are properly invoked during cleanup operations, including destructor paths
    • Enhanced error handling for finalizer execution with improved error routing to exception handlers
    • Strengthened state management to accurately track generator closure status throughout the lifecycle

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 17, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

This change enhances async generator finalization by ensuring finalizers are properly invoked, managing generator state closure during termination, and routing finalizer errors to the VM's unraisable hook. The destructor now unconditionally attempts finalizer calls and updates error messaging.

Changes

Cohort / File(s) Summary
Async generator finalizer and state management
crates/vm/src/builtins/asyncgenerator.rs		 Adds finalizer invocation with error handling via vm.run_unraisable; updates destructor to always call finalizers and changes error message; explicitly sets ag.inner.closed = true and state to Closed in termination paths (yield_close, check_error)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • fanninpm
  • ShaharNaveh

Poem

🐰 A finalizer's tale, so precise and fine,
State closure marks the end of the line,
Error handling routed, with care divine,
Async generators dance in perfect design!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix asyncgen finalizer' directly and clearly summarizes the main change: fixing finalizer handling in async generators. It is specific, concise, and accurately reflects the primary objective of the changeset.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

@youknowone youknowone marked this pull request as ready for review January 18, 2026 05:08
@youknowone youknowone changed the title fix asyncgen fix asyncgen finalizer Jan 18, 2026
@youknowone youknowone merged commit 7c7b824 into RustPython:main Jan 18, 2026
13 checks passed
@youknowone youknowone deleted the asyncgen branch January 18, 2026 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone