Skip to content

Handle EINTR retry in os.write() (PEP 475)#7482

Merged
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:eintr
Mar 25, 2026
Merged

Handle EINTR retry in os.write() (PEP 475)#7482
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:eintr

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Mar 23, 2026 •
edited by coderabbitai bot
Loading

Add EINTR retry loop to os.write(), matching the existing pattern in os.read() and os.readinto(). Remove the expectedFailure marker from test_write in _test_eintr.py.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced reliability of file write operations by properly handling signal interruption.

  • Refactor

    • Improved performance of dictionary element extraction operations.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 23, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 03589555-5d90-46d7-963c-06bb740d03a6

📥 Commits

Reviewing files that changed from the base of the PR and between 372280e and b2aef25.

⛔ Files ignored due to path filters (2)
  • Lib/test/_test_eintr.py is excluded by !Lib/**
  • Lib/test/_test_multiprocessing.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/vm/src/stdlib/os.rs
  • crates/vm/src/vm/mod.rs
📝 Walkthrough

Walkthrough

Updated the write function in os._os to handle EINTR errors with retry logic via signal checks, and added fast-path optimizations in VirtualMachine::extract_elements_with for dict-like inputs using snapshot methods.

Changes

Cohort / File(s) Summary
EINTR Handling in OS Write
crates/vm/src/stdlib/os.rs		 Modified os._os.write to return PyResult<usize> instead of io::Result<usize>. Added logic to retry on EINTR errors by calling vm.check_signals()? and continuing the write loop, while converting non-EINTR errors to Python exceptions via e.into_pyexception(vm).
Dict Fast Paths in VirtualMachine
crates/vm/src/vm/mod.rs		 Added type-specific fast paths in extract_elements_with for dict-like inputs (dict, dict_keys_type, dict_values_type). Uses snapshot methods (keys_vec(), values_vec()) for atomic extraction. Imported PyDictKeys to enable downcasting for dict keys branch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A signal knocks upon the write,
We pause and check, then hoist to height,
Dict keys snapshot, swift and lean,
Fast paths dance—the cleanest seen,
Errors caught, no more to fight! 🏃‍♂️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% 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 PR title 'Handle EINTR retry in os.write() (PEP 475)' directly and specifically describes the primary change in the changeset, which updates the os.write() function to handle EINTR interrupts with proper retry logic.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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
Handle EINTR retry in os.write() (PEP 475)
5b77f1d 
Add EINTR retry loop to os.write(), matching the existing
pattern in os.read() and os.readinto(). Remove the
expectedFailure marker from test_write in _test_eintr.py.
@youknowone
Add atomic snapshot for dict/dict_keys in extract_elements
b2aef25 
Add fast paths for dict and dict_keys types in
extract_elements_with, matching _list_extend() in CPython
Objects/listobject.c. Each branch takes an atomic snapshot
under a single read lock, preventing race conditions from
concurrent dict mutation without the GIL.

Remove expectedFailure from test_thread_safety.
@youknowone youknowone marked this pull request as ready for review March 25, 2026 04:56
@youknowone youknowone merged commit 6b5c5a9 into RustPython:main Mar 25, 2026
19 checks passed
@youknowone youknowone deleted the eintr branch March 25, 2026 05:02
Copilot AI pushed a commit that referenced this pull request Mar 25, 2026
@youknowone
Handle EINTR retry in os.write() (PEP 475) (#7482)
4293350 
* Handle EINTR retry in os.write() (PEP 475)

Add EINTR retry loop to os.write(), matching the existing
pattern in os.read() and os.readinto(). Remove the
expectedFailure marker from test_write in _test_eintr.py.

* Add atomic snapshot for dict/dict_keys in extract_elements

Add fast paths for dict and dict_keys types in
extract_elements_with, matching _list_extend() in CPython
Objects/listobject.c. Each branch takes an atomic snapshot
under a single read lock, preventing race conditions from
concurrent dict mutation without the GIL.

Remove expectedFailure from test_thread_safety.
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

@youknowone