X Tutup
Skip to content

Fix Overlapped I/O by boxing OVERLAPPED struct and accept overlapped arg positionally#7198

Merged
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:overlapped
Feb 22, 2026
Merged

Fix Overlapped I/O by boxing OVERLAPPED struct and accept overlapped arg positionally#7198
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:overlapped

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Feb 22, 2026

Two fixes in _winapi:

  1. Box the OVERLAPPED struct in OverlappedInner to ensure it stays at a stable heap address. Previously, into_pyobject() moved the struct after ReadFile/WriteFile had given the OS a pointer to it, causing GetOverlappedResult to read stale data (returning err=0 instead of ERROR_MORE_DATA for zero-byte reads on message pipes).

  2. Change ReadFile, WriteFile, ConnectNamedPipe overlapped parameter from #[pyarg(named)] to #[pyarg(any)] so it can be passed both positionally and as keyword argument.

Summary by CodeRabbit

  • Refactor
    • Enhanced Windows API functions (ConnectNamedPipe, WriteFile, ReadFile) to accept arguments in both positional and keyword formats for improved flexibility.
    • Improved internal stability of Windows I/O operations.

…arg positionally

Two fixes in _winapi:

1. Box the OVERLAPPED struct in OverlappedInner to ensure it stays at
   a stable heap address. Previously, into_pyobject() moved the struct
   after ReadFile/WriteFile had given the OS a pointer to it, causing
   GetOverlappedResult to read stale data (returning err=0 instead of
   ERROR_MORE_DATA for zero-byte reads on message pipes).

2. Change ReadFile, WriteFile, ConnectNamedPipe overlapped parameter
   from #[pyarg(named)] to #[pyarg(any)] so it can be passed both
   positionally and as keyword argument.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 22, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

Refactors OverlappedInner to box the OVERLAPPED field for stable heap addressing. Updates all call sites to dereference the boxed value. Changes Python-exposed API argument attributes on ConnectNamedPipeArgs, WriteFileArgs, and ReadFileArgs to accept flexible argument styles (positional and keyword).

Changes

Cohort / File(s) Summary
Windows I/O Overlapped Structure
crates/vm/src/stdlib/_winapi.rs (OverlappedInner)
Boxes the overlapped field and updates all 6 call sites (GetOverlappedResult, CancelIoEx, ConnectNamedPipe) to dereference via &*inner.overlapped or &mut *inner.overlapped.
Python API Argument Signatures
crates/vm/src/stdlib/_winapi.rs (ConnectNamedPipeArgs, WriteFileArgs, ReadFileArgs)
Changes argument attributes from #[pyarg(positional)] to #[pyarg(any)] to allow both positional and keyword argument styles; updates overlapped arguments with defaults (#[pyarg(any, optional)] or #[pyarg(any, default = false)]).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A box for stability, dereferenced with care,
OVERLAPPED fields heap-bound, forever anchored there.
Python arguments now flex both ways,
Positional and keyword, brighter days!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures both main changes: boxing the OVERLAPPED struct for stability and accepting overlapped arguments positionally.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@youknowone youknowone marked this pull request as ready for review February 22, 2026 13:05
@youknowone youknowone merged commit b48f72d into RustPython:main Feb 22, 2026
12 of 13 checks passed
@youknowone youknowone deleted the overlapped branch February 22, 2026 13:14
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

X Tutup