FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix _overlapped segfault and missing _winapi constants by youknowone · Pull Request #6907 · RustPython/RustPython · GitHub

/ RustPython Public

Fix _overlapped segfault and missing _winapi constants - #6907

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:winapi
Jan 30, 2026
ConversationCommits (1)ChecksFiles changed
Merged

Fix _overlapped segfault and missing _winapi constants#6907
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:winapi

Conversation

youknowone commented Jan 30, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member
  • 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 _overlapped segfault and missing _winapi constants
652eb1a 
- 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 Bot commented Jan 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
📝 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 marked this pull request as ready for review January 30, 2026 21:34
youknowone merged commit 050faa3 into RustPython:main Jan 30, 2026
11 of 13 checks passed
youknowone deleted the winapi branch January 30, 2026 21:39
This was referenced Feb 4, 2026
Merged
Merged
coderabbitai Bot mentioned this pull request Feb 22, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

Back | FazBrowse Home | New Git URL