8000
  • fix asyncgen finalizer by youknowone · Pull Request #6761 · RustPython/RustPython · GitHub
    [go: up one dir, main page]
    Skip to content

    fix asyncgen finalizer#6761

    Merged
    youknowone merged 1 commit intoRustPython:mainfrom
    youknowone:asyncgen
    Jan 18, 2026
    Merged

    fix asyncgen finalizer#6761
    youknowone merged 1 commit intoRustPython:mainfrom
    youknowone:asyncgen

    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
    @coderabbitai coderabbitai bot mentioned this pull request Feb 1, 2026
    Merged
    @coderabbitai coderabbitai bot mentioned this pull request Feb 11, 2026
    Merged
    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
    0