Skip to content

Start socket timeout after the sock snapshot - #8791

Merged
youknowone merged 4 commits into
RustPython:mainfrom
youknowone:fix/sendall-deadline-stw-test
Sep 23, 2026
Merged

Start socket timeout after the sock snapshot#8791
youknowone merged 4 commits into
RustPython:mainfrom
youknowone:fix/sendall-deadline-stw-test

Conversation

@youknowone

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

Copy link
Copy Markdown
Member

Follow-up to the macOS CGI flake still seen on #8780.

test_large_content_length_truncated sets the handler timeout to 0.001s. Under high load (load average ~20) the first CI pass hits ConnectionResetError: [Errno 54] Connection reset by peer; the isolated rerun passes.

#8786 started the timeout clock after sock_snapshot. The #8786 review follow-up created the Deadline before that snapshot so sendall could reuse one clock. Scheduling delay then ate the 1ms budget, poll saw an expired deadline, and Darwin reset the connection before the CGI status line was sent.

sock_op now snapshots first and starts the clock there. sendall still keeps one Deadline for the whole write.

Verified:

  • cargo test -p rustpython-host_env wait_fd
  • cargo check -p rustpython-stdlib --features threading

Assisted-by: Grok:4.6

Summary by CodeRabbit

  • Bug Fixes
    • Socket operations with short timeouts now allow the full configured duration, without counting delays before the operation begins.
    • Waiting and non-blocking socket operations now handle retry behavior more consistently when an operation cannot proceed immediately.

@youknowone
Drop racy STOP_BIT assertion from eval-breaker test
f1fef34 
The isolated test asserted a clear process-wide eval breaker, and
cleared STOP_BIT itself. Parallel STW tests flicker that word, so
Windows rust tests failed after RustPython#8786.

Assisted-by: Grok:4.6
@youknowone
Merge branch 'main' into fix/sendall-deadline-stw-test
26731c2 
Keep the per-thread eval-breaker test. Drop the STOP_BIT retry
from RustPython#8788 and the unused stop_bit_set helper.

Assisted-by: Grok:4.6
@youknowone
Merge remote-tracking branch 'upstream/main' into fix/sendall-deadlin…
aaf1809 
…e-stw-test
@youknowone
Start socket timeout after the sock snapshot
e613aef 
A 1ms CGI handler timeout was expiring during lock/scheduling
delay before poll, so macOS closed the connection with RST
under load. sendall still reuses one deadline for the write.

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

chatgpt-codex-connector Bot commented Sep 23, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: ec21cab1-d527-4b4f-b070-a13840ab1070

📥 Commits

Reviewing files that changed from the base of the PR and between e4984d5 and e613aef.

📒 Files selected for processing (1)
  • crates/stdlib/src/socket.rs

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

📝 Walkthrough

Walkthrough

Socket timeout handling now keeps the configured timeout separate from the deadline. The shared loop initializes the deadline after taking a socket snapshot. Both sock_op and sendall pass a mutable deadline through the loop.

Changes

Socket timeout handling

Layer / File(s) Summary
Lazy deadline initialization and retry handling
crates/stdlib/src/socket.rs		 sock_op and sendall pass the configured timeout and a mutable deadline to sock_op_timeout_err. The loop initializes the deadline after taking a socket snapshot and retries WouldBlock when a timeout is configured.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e613a

The supplied evidence identifies no remaining issue that should block merging this socket-timeout change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 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 and concisely describes the main change: starting the socket timeout after taking the socket snapshot.
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 a new PR

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 3ac2afd into RustPython:main Sep 23, 2026
30 checks passed
@youknowone
youknowone deleted the fix/sendall-deadline-stw-test branch September 23, 2026 07:59
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