-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix asyncio related compiler/library issues #6837
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.
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
Conversation
📝 WalkthroughWalkthroughRefactors finally/unwind resolution to prefer outer handlers, updates context copy to accept a VM and deep-clone internal hamt state, extends socket getaddrinfo to accept bytes/text for host/port, adds SSL partial-write and ZeroReturn semantics, and makes generator/coroutine frame getters return Option. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/stdlib/src/ssl/compat.rs (1)
1744-1782: Fix partial-write retry bug: return partial success instead of deferring to bad-write-retry check.rustls 0.23's
Writer::write()can return partial writes (accepted bytes <data.len()) when internal buffer limits are exceeded. Currently, if such a partial write occurs (line 1760) and latersend_all_bytesreturnsWantWrite,WantRead, orTimeout(lines 1810–1828), the partial count is preserved inwrite_buffered_len(line 1781). On retry with the original full buffer, the condition at line 1837 (already_buffered != data.len()) incorrectly triggers a "bad write retry" error, violating OpenSSL's partial-write retry semantics.Instead, when a partial write is followed by a
WANT_*orTimeouterror path, return the partial success immediately rather than attempting to re-buffer on retry:🛠️ Suggested approach: surface partial writes on WANT_* paths
Track whether only a prefix was accepted (
wrote_partial), and onWantWrite/WantRead/Timeout, return the partial count instead of deferring the retry:let mut bytes_written_to_rustls = 0usize; + let mut wrote_partial = false; if already_buffered == 0 { // ... write plaintext to rustls ... match writer.write(data) { ... } + wrote_partial = bytes_written_to_rustls < data.len(); *socket.write_buffered_len.lock() = bytes_written_to_rustls; } else if already_buffered != data.len() { // bad write retry check } loop { // ... send TLS records ... match send_all_bytes(...) { Err(SslError::WantWrite) => { + if wrote_partial { + *socket.write_buffered_len.lock() = 0; + return Ok(bytes_written_to_rustls); + } return Err(SslError::WantWrite); } Err(SslError::WantRead) => { + if wrote_partial { + *socket.write_buffered_len.lock() = 0; + return Ok(bytes_written_to_rustls); + } // ... existing logic ... } Err(e @ SslError::Timeout(_)) => { + if wrote_partial { + *socket.write_buffered_len.lock() = 0; + return Ok(bytes_written_to_rustls); + } return Err(e); } } }(Also applies to: 1857–1871)
🤖 Fix all issues with AI agents
In `@crates/codegen/src/compile.rs`:
- Around line 1531-1554: The current loop unconditionally skips all
FBlockType::FinallyTry blocks when searching for a handler (in the block that
computes (handler, stack_depth, preserve_lasti) from self.code_stack.last()),
which can omit outer finally handlers during return unwinding; change the
condition so you only skip the finally block that is the one currently being
unwound (the block that was just removed/initiated the return) rather than all
FinallyTry blocks — i.e., remove the blanket matches!(fblock.fb_type,
FBlockType::FinallyTry) continue and instead detect the unwound block (for
example by comparing fblock.fb_stack_depth or another identifier against the
unwinding context) and skip only that specific fblock while allowing outer
FinallyTry fblocks with fb_handler to be selected via the existing
fblock.fb_handler, fblock.fb_stack_depth + 1, and fblock.fb_preserve_lasti
logic.
🧹 Nitpick comments (2)
crates/stdlib/src/socket.rs (2)
2788-2792: Add tests for bytes-like host/port inputs.
The public API now accepts bytes-like objects; coverage for bytes host/port (including invalid UTF‑8) would lock in the intended behavior.
2815-2833: Clarify the UTF‑8 requirement for byte hosts in the inline comment.
The implementation validates UTF‑8 and errors otherwise, so “bytes used as-is” can read as accepting arbitrary bytes.Proposed comment tweak
- // Encode host: str uses IDNA encoding, bytes used as-is + // Encode host: str uses IDNA encoding; bytes must be valid UTF-8 and bypass IDNA
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.