Skip to content

Fix closerange empty ranges before unsigned conversion - #9031

Draft
youknowdot wants to merge 3 commits into
RustPython:mainfrom
youknowdot:fix/closerange-empty-range
Draft

youknowdot wants to merge 3 commits into
RustPython:mainfrom
youknowdot:fix/closerange-empty-range

Conversation

@youknowdot

@youknowdot youknowdot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix empty os.closerange() ranges before converting the exclusive upper bound to an inclusive unsigned bound. After clamping the lower bound, equal bounds must return without calling close_range(). Otherwise zero or negative upper bounds can close every descriptor.

Rebased onto fd5b30eb5e6c1dd24bb98a305b1c5f47fa64820d. This includes main's Python 3.15 TextIOWrapper assertion correction (#9082). The branch also carries the shared PyO3 0.30 benchmark import fix needed by strict Clippy. No Lib/ changes or Rust behavioral regression tests are added.

Regression coverage and CPython reference limitation

The existing stdlib_os.py snippet runs the closerange regression in a child process. Every RustPython no-op case remains active: (0, 0), (-1, 0), positive equal bounds, positive reversed bounds, and (-2, -1). The single-descriptor case also verifies adjacent descriptors remain open.

The exact setup-python CPython 3.15.0 Linux artifact reproduces python/cpython#159182, with upstream fix #159183 still open. Unlike the earlier revision, this revision explicitly excludes the three documented zero/negative-upper-bound cases only on CPython 3.15.0 final, Linux, with HAVE_CLOSE_RANGE. All other reference cases still execute. RustPython and Python 3.15.1+ are not excluded. The temporary exclusion cites the upstream issue and fix.

Validation

  • Normal commit hooks passed for every local commit; all-file hooks passed before the final small reference guard, whose normal hooks also passed.
  • Exact CI CPython 3.15.0 artifact: original closerange regression failed with child exit 120; guarded regression passes. Trace confirmed precisely the three documented exclusions and both unaffected no-op cases executed. Single-descriptor assertions also pass.
  • Rebased stdlib_io.py passes the exact CPython reference artifact.
  • The full local CPython snippet run is not a clean pass: 176 passed and four failed before stopping an unrelated GC-stress test. stdlib_os reaches a later sandbox-specific chown failure (EINVAL instead of PermissionError); three multiprocessing failures were not investigated. No unrelated assertions were changed.
  • Strict workspace/all-targets Clippy passed with warnings denied on this revision.
  • Fresh RustPython debug build: focused closerange regression passes. Trace confirms all five original no-op cases execute with zero exclusions; single-descriptor assertions pass. Full stdlib_os.py reaches the same later sandbox chown failure as CPython.
  • Full local workspace/C-API unit suites were not run; hosted CI for this exact head is pending.

AI assistance: OpenAI Codex rebased, implemented, reviewed, and tested this revision under maintainer direction. Model version unavailable.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • 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.

@youknowdot
youknowdot force-pushed the fix/closerange-empty-range branch 2 times, most recently from 7c27a9f to 0a3d05a Compare October 11, 2026 03:01
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_mmap.py (TODO: 19)

dependencies:

dependent tests: (2 tests)

  • mmap: test_os
    • multiprocessing.shared_memory: test_genericalias

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowdot
youknowdot force-pushed the fix/closerange-empty-range branch from 0a3d05a to 7ad6487 Compare October 11, 2026 03:21
@youknowone
youknowone changed the base branch from main to stack/python-3.15-8950 October 11, 2026 03:26
@youknowdot
youknowdot force-pushed the fix/closerange-empty-range branch from 7ad6487 to e1a5bb2 Compare October 11, 2026 04:07
@youknowone
youknowone changed the base branch from stack/python-3.15-8950 to main October 11, 2026 04:08
@youknowdot
youknowdot force-pushed the fix/closerange-empty-range branch from e1a5bb2 to 462147d Compare October 11, 2026 04:14
Return for equal bounds after clamping the lower bound. Retain subprocess-isolated Python regressions for empty, reversed, negative, and one-descriptor ranges. CPython builds with HAVE_CLOSE_RANGE retain the reference defect; do not weaken the regression.

Assisted-by: OpenAI Codex:model version unavailable
Import PyCodeInput from the types module, the canonical location in PyO3 0.30. The old ffi re-export is deprecated and fails Clippy with warnings denied.

Assisted-by: OpenAI Codex:model version unavailable
Keep every RustPython closerange regression active. Exclude only the three documented negative or zero-upper-bound cases on CPython 3.15.0 final Linux builds with HAVE_CLOSE_RANGE, pending python/cpython#159183 for issue #159182. Positive empty, reversed and one-descriptor reference cases remain active; later versions run all cases.

Assisted-by: OpenAI Codex:model version unavailable
@youknowdot
youknowdot force-pushed the fix/closerange-empty-range branch from 462147d to 5bc1814 Compare October 16, 2026 04:13
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