Fix asyncio related compiler/library issues#6837
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.
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.