Honor asyncgen ag_closed and wasm BIO EOF by youknowone · Pull Request #8718 · RustPython/RustPython · GitHub
Skip to content

Honor asyncgen ag_closed and wasm BIO EOF - #8718

Merged
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:host-env-win-ffi
Sep 15, 2026
Merged

Honor asyncgen ag_closed and wasm BIO EOF#8718
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:host-env-win-ffi

Conversation

@youknowone

@youknowone youknowone commented Sep 15, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Follow-ups that missed the #8716 squash.

  • ag_closed is separate from a finished frame. aclose() sets it before GeneratorExit. asend() already-running closes the awaitable so it does not warn unawaited.
  • Browser _ssl pump feeds MemoryBIO EOF only when rustls has no pending plaintext.

Assisted-by: Grok:grok-4.6

Summary by CodeRabbit

  • Bug Fixes
    • Improved asynchronous generator closure handling, including more reliable finalization and protection against re-entrant awaitable use.
    • Corrected behavior when asynchronous generators finish or are explicitly closed.
    • Prevented SSL connections from sending close notifications while plaintext data remains pending.

@youknowone
address review: drain plaintext before BIO EOF
790293f 
Skip feed_tls([]) while rustls still has application
data, matching the native SSL read EOF order.

Assisted-by: Grok:grok-4.6
@youknowone
Honor asyncgen ag_closed and asend reuse
9b68670 
Keep ag_closed apart from a finished frame. aclose()
sets it before GeneratorExit. asend() already-running
closes the awaitable so it does not warn unawaited.

Assisted-by: Grok:grok-4.6
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown

@coderabbitai

coderabbitai Bot commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: c1f42619-8969-4e85-8508-9c6091055cdb

📥 Commits

Reviewing files that changed from the base of the PR and between 821f18d and 9b68670.

📒 Files selected for processing (2)
  • crates/vm/src/builtins/asyncgenerator.rs
  • crates/wasm/src/ssl.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The change separates async generator closure state from frame completion and updates related awaitable and finalizer behavior. It also delays TLS close-notify processing until buffered plaintext is consumed.

Changes

Async generator lifecycle

Layer / File(s) Summary
Closure state tracking
crates/vm/src/builtins/asyncgenerator.rs		 PyAsyncGen adds and initializes ag_closed. unbox and check_error now keep ag_closed separate from inner.closed.
Awaitable and finalizer behavior
crates/vm/src/builtins/asyncgenerator.rs		 Send and throw operations update awaitable states and handle closed generators. Finalizer and destructor paths use ag_closed.

TLS EOF processing

Layer / File(s) Summary
EOF close-notify ordering
crates/wasm/src/ssl.rs		 PySSLSocket::pump feeds the EOF close-notify input only when no plaintext is pending.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9b686

No merge-blocking behavior issue was identified in the updated async-generator or TLS EOF handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies both primary changes: async-generator ag_closed handling and wasm BIO EOF behavior. It is concise and specific.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@youknowone
youknowone merged commit 823f6c6 into RustPython:main Sep 15, 2026
30 checks passed
@youknowone
youknowone deleted the host-env-win-ffi branch September 15, 2026 13:27
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