Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 30, 2026

  • Fix from_windows_err using new_exception_empty on OSError subclasses (ConnectionRefusedError, ConnectionAbortedError), which caused segfault in release builds due to debug_assert on type size mismatch
  • Move ConnectPipe from instance method to module-level function
  • Add Destructor for Overlapped to cancel pending I/O on object cleanup
  • Add NMPWAIT_NOWAIT, NMPWAIT_USE_DEFAULT_WAIT, NMPWAIT_WAIT_FOREVER constants to _winapi

Summary by CodeRabbit

  • New Features
    • Added support for connecting to named pipes on Windows.
    • Introduced new pipe wait mode constants for Windows pipe operations.
    • Enhanced cleanup and resource management for Windows I/O operations when objects are garbage-collected.

✏️ Tip: You can customize this high-level summary in your review settings.

- Fix from_windows_err using new_exception_empty on OSError subclasses
  (ConnectionRefusedError, ConnectionAbortedError), which caused segfault
  in release builds due to debug_assert on type size mismatch
- Move ConnectPipe from instance method to module-level function
- Add Destructor for Overlapped to cancel pending I/O on object cleanup
- Add NMPWAIT_NOWAIT, NMPWAIT_USE_DEFAULT_WAIT, NMPWAIT_WAIT_FOREVER
  constants to _winapi
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

This PR refactors Windows overlapped I/O error handling by simplifying errno translation to a generic IO error, adds explicit destructor support to the Overlapped pyclass with cleanup semantics, reintroduces ConnectPipe as a standalone function, and exposes three new pipe-related constants in the Windows API module.

Changes

Cohort / File(s) Summary
Overlapped I/O Refactoring
crates/stdlib/src/overlapped.rs
Reworked error translation to use generic IO errors instead of specific Windows errno mappings; added destructor implementation to Overlapped pyclass with I/O cancellation and resource cleanup; reintroduced ConnectPipe as top-level #[pyfunction]; updated public API surface and import signatures.
Windows API Constants
crates/vm/src/stdlib/winapi.rs
Exposed three new pipe wait constants (NMPWAIT_NOWAIT, NMPWAIT_USE_DEFAULT_WAIT, NMPWAIT_WAIT_FOREVER) in the Windows Pipes namespace.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • new_last_{os,errno}_error #6381: Modifies overlapped.rs Windows error handling and imports, with overlapping changes to errno-based error construction and helper functions.

Poem

🐰 Hops through Windows, cleaning as we go,
Destructors now whisper what I/O can know,
Overlapped paths renewed, ConnectPipe set free,
Constants align—errno flows naturally!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly aligns with the main changes: fixing a segfault in _overlapped module and adding missing constants to _winapi module.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 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 January 30, 2026 21:34
@youknowone youknowone merged commit 050faa3 into RustPython:main Jan 30, 2026
11 of 13 checks passed
@youknowone youknowone deleted the winapi branch January 30, 2026 21:39
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