Skip to content

Avoid epoll alias ownership aborts - #9250

Open
youknowdot wants to merge 1 commit into
RustPython:mainfrom
youknowdot:fix/epoll-aliased-fd-ownership
Open

youknowdot wants to merge 1 commit into
RustPython:mainfrom
youknowdot:fix/epoll-aliased-fd-ownership

Conversation

@youknowdot

@youknowdot youknowdot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix the debug-profile abort exposed by the coverage run after #9234. select.epoll.fromfd() deliberately aliases its caller's descriptor, but RustPython represented both aliases as OwnedFd. Closing one and later dropping the other violated Rust's exclusive-ownership contract and aborted the process.

  • Store Python-visible epoll descriptors in a raw-descriptor holder. Explicit close still reports errors; implicit destruction performs a best-effort close.
  • Preserve descriptor aliasing and existing locking. Do not duplicate descriptors or change global Python-to-fd conversion.
  • Call raw epoll_ctl and epoll_wait through host_env, using a transparent host-only wrapper over libc's native event layout, including Redox padding. No artificial BorrowedFd lifetime is asserted for descriptors supplied by Python.
  • Use CPython-style ceiling-millisecond conversion and reject positive timeouts beyond signed 32-bit milliseconds. Retain the existing outer EINTR/deadline handling.

The patch changes only crates/host_env/src/select.rs and crates/stdlib/src/select.rs. No Lib changes, test skips, assertion edits or CI workflow changes.

Reference and scope

Source commit 75346b7f12 is based on main b4a1ad232f8eb68523ce687183ce08958d4b6744. Its Rust/Cargo source bytes match the locally tested pre-commit tree. Main subsequently advanced to ebb6a78521 with disjoint reflected-multiplication changes; local results are not attributed to that newer merge. The original ownership patch rebased without conflicts; the final follow-up keeps the native event type behind a transparent host-only wrapper to satisfy the existing stdlib lint boundary. Compare pinned CPython 3.15.0 selectmodule.c.

Existing unrelated differences remain: fromfd(-1) raises EBADF in RustPython while CPython's shared constructor creates an epoll; poll(maxevents=0) reaches the syscall and reports EINVAL in RustPython while CPython raises ValueError. These are not hidden by the focused probe.

Validation

  • The sealed pre-repair dev binary reproduces the exact IO-safety SIGABRT (exit signal 6, 0.117 seconds) on a bounded alias/drop case using only a descriptor created by that process.
  • Pinned CPython 3.15.0 passes all four groups of the focused own-epoll/own-pipe diagnostic (0.034 seconds).
  • A fresh locked/offline dev CLI build on the stated base plus this patch passes. Debug symbols are disabled to fit the environment, matching CI's symbol setting; debug assertions, source optimizations, features and overflow checks are not disabled.
  • The final matching candidate passes all four focused groups in 0.366 seconds: lifecycle, alias/drop, alias/explicit-close, and owned-pipe register/modify/unregister with returned descriptor/event verification. Additional checks cover zero/tiny/ready-only-negative timeouts, invalid maxevents and positive timeout overflow. Each probe has a ten-second external deadline, disabled core dumps and a bounded output log.
  • Independent static review found no blocking issue in ownership, locking, native event layout, packed-field value access, Redox padding or timeout conversion. This is source review, not cross-platform runtime validation.
  • Strict Clippy passed for root, host_env and stdlib library/binary targets with -D warnings. Normal configured file hooks passed (merge conflict, rustfmt and cspell; non-applicable hooks skipped normally).

These are focused runtime results, not a full test_epoll pass. The canonical module's shared setup opens sockets, so it was not run locally under the current execution boundary. Full current-source workspace/CAPI tests, all-target lint, instrumented coverage, signal-interruption runtime, and non-Linux runtime validation remain unrun. Earlier ImportError native-suite results belong to different source and are not reused as this patch's validation.

Local full native gates required by the repository guidance remain outstanding; this draft does not present focused checks as their substitute.

AI assistance

Assisted implementation, source review and automated validation by OpenAI Codex. This reports automated checks, not additional human testing. The commit includes an Assisted-by trailer; the exact model version is unavailable.

Summary by CodeRabbit

  • Bug Fixes
    • Improved epoll support on Linux, Android, and Redox platforms.
    • Improved handling of polling timeouts, including long durations and interrupted waits.
    • Epoll events now report their associated data and flags more reliably.
    • Invalid file descriptors are rejected with an appropriate error.
    • Improved error reporting for epoll creation, registration, modification, removal, and waiting operations.

Keep Python-visible epoll descriptors in a raw host wrapper, preserving fromfd aliasing and explicit close errors without OwnedFd drop aborts. Use native epoll event layout behind a transparent host-only type and align positive timeout overflow with CPython milliseconds.

Assisted-by: OpenAI Codex:model-version-unavailable
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T14:04:48.386168Z 75346b7 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: e9f13898-30d7-477a-8267-996e123fa831
📥 Commits

Reviewing files that changed from the base of the PR and between ebb6a78 and 75346b7.

📒 Files selected for processing: 2
  • crates/host_env/src/select.rs
  • crates/stdlib/src/select.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

The host epoll API now uses raw descriptor wrappers and libc event operations. The Python epoll wrapper uses these interfaces and converts polling timeouts to signed milliseconds.

Changes: Epoll API update

Layer / File(s) Summary
Host epoll wrappers and operations
crates/host_env/src/select.rs
The host module adds Event and EpollFd wrappers. Creation, closing, and control operations use raw descriptors and libc calls.
Host epoll wait
crates/host_env/src/select.rs
The wait function accepts an integer timeout, calls epoll_wait, and maps interruptions and other errors.
Python epoll integration
crates/stdlib/src/select.rs
PyEpoll stores EpollFd and passes raw descriptors to host operations. Timeout conversion rounds up to milliseconds and checks overflow. Event data and flags use the host accessors.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to 75346

No actionable merge-blocking issue is established for the epoll change; it is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 31.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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 describes the main change: preventing ownership-related aborts caused by aliased epoll descriptors.
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
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant