Repository navigation
Fix closed socket descriptors on 32-bit Windows - #9432
Conversation
Return the Python-facing -1 sentinel explicitly for closed sockets instead of widening Windows INVALID_SOCKET. Keep live descriptor conversions unchanged and cover fileno, repeated detach, and repr with a regression test. Implementation, investigation, validation, and focused review assisted by Codex. Assisted-by: Codex:GPT-5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Closed-socket results and test crates/stdlib/src/socket.rs |
detach and fileno return -1 when the socket is absent. INVALID_SOCKET is now defined only on Windows. A test checks repeated calls and confirms the representation reports fd=-1. |
Priority:
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Bug fix
Possibly related PRs
- RustPython/RustPython#9266: Updates
selectto consume the integer returned byfileno(), which is affected by the closed-socket return value.
Suggested reviewers: 1ndahous3, kyokuping
Merge Risk: 
2e17f
-
The change corrects closed-socket descriptors on 32-bit Windows without changing live-socket handling. No actionable merge-blocking risk remains after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: fixing closed socket descriptors on 32-bit Windows. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
The closed-socket descriptor fix leaves INVALID_SOCKET referenced only by the Windows raw-socket validation branch. Remove its unused Unix definition so Linux and macOS Clippy pass with -D warnings, without changing descriptor validation or CI settings. Investigation, implementation, validation, and focused automated review assisted by Codex. Assisted-by: Codex:GPT-5
Merging this PR will not alter performance
Comparing 
Footnotes
|
Summary
Fix closed socket descriptors on 32-bit Windows. Widening the unsigned
INVALID_SOCKETvalue toi64produced4294967295on x86 instead of Python's-1sentinel.-1explicitly fromfileno()anddetach()when no socket is owned; this also fixes closed-socketrepr().fileno(), repeateddetach(), andrepr().Only
crates/stdlib/src/socket.rsis changed. No C API, ctypes, PyO3, CI, or stdlib test modifications.Validation
test_socketandtest_asyncio.test_sock_lowlevelpass (783 tests run, 516 skipped per architecture).-D warnings, separate C API Clippy, formatting, and pre-commit hooks pass.pythonXY.libis unavailable; C API is unchanged and outside this fix.Implementation, investigation, validation, and focused review were assisted by Codex. The review was an automated focused review, not human review.
Summary by CodeRabbit
detachandfilenonow return-1for closed sockets across platforms, rather than a platform-specific sentinel value.-1through repeated calls to both methods. Their displayed representation reflects the same descriptor value.CI follow-up
Linux and macOS Clippy reported an unused Unix
INVALID_SOCKETdefinition after the closed-socket fix. Restrict the constant to Windows, its only remaining code consumer. No CI workflow or lint suppression changes.Verified with native Linux (WSL)
cargo clippy --locked -p rustpython-stdlib --features threading --lib -- -D warnings, Windows workspace tests, and Windows workspace all-targets Clippy with-D warnings(same exclusions as above). GitHub CI is rerunning for the new commit.