posix, fcntl, io: re-issue interrupted syscalls instead of reporting EINTR - #1255
Conversation
…EINTR `wrap_oserror(..., eintr_retry=True)` runs the pending Python signal handlers and re-issues the call; the loop ends only when a handler raises. These sites reported the interruption directly. interp_posix.py: open, stat/fstat by descriptor, fchdir, fsync, fdatasync, waitpid, mkfifo, mknod, fstatvfs, chmod, lchmod, fchmod, chown, lchown, fchown, sched_yield, sched_get_priority_max, sched_get_priority_min, sched_rr_get_interval, sched_getscheduler, sched_setscheduler, sched_getparam, sched_setparam. `posix.wait` is `posix.waitpid(-1, 0)` and takes that call's loop. chmod reads an ENOTSUP/EOPNOTSUPP refusal of `follow_symlinks=False` as the modifier being unavailable rather than as an OS error, and reads it ahead of the retry (interp_posix.py:1247-1251). interp_fcntl.py: fcntl, flock and lockf take their descriptor through `space.c_filedescriptor_w` and retry through `_raise_error_maybe`. ioctl takes its descriptor the same way; it reports through `_raise_error_always` and does not retry. interp_fileio.py:135-147 `_open_fd`: the same loop around the open in `open_raw_file`. Assisted-by: Claude
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThe interpreter now retries selected OS-level ChangesSystem-call interruption handling
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to The change retries interrupted system calls, but sandbox-backed open paths still return EINTR as an error without retrying, leaving callers with inconsistent behavior and avoidable interruption failures. The PR is not merge-ready until those paths are updated or the exception is explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant PythonAPI
participant InterpreterWrapper
participant OperatingSystem
PythonAPI->>InterpreterWrapper: Invoke POSIX or fcntl operation
InterpreterWrapper->>OperatingSystem: Execute system call
OperatingSystem-->>InterpreterWrapper: EINTR or operation result
InterpreterWrapper->>OperatingSystem: Retry interrupted operation
OperatingSystem-->>InterpreterWrapper: Final result or OS error
InterpreterWrapper-->>PythonAPI: Return value or converted exception
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 963f3bf). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)
4. Structural adaptationsNone. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 963f3bf60d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if unsafe { libc::sched_rr_get_interval(pid, &mut interval) } == -1 { | ||
| return Err(io_err(std::io::Error::last_os_error(), "")); | ||
| // interp_posix.py:3062-3068 `sched_rr_get_interval`: retry on EINTR. | ||
| loop { |
There was a problem hiding this comment.
Match PyPy's one-shot scheduler control flow
When sched_rr_get_interval is interrupted and the Python signal handler returns normally, pypy/module/posix/interp_posix.py:3065-3070 has only one try/except: wrap_oserror(..., eintr_retry=True) returns and the function falls through to None. This added loop instead reissues the syscall and returns a float; the same divergence occurs in the adjacent sched_getscheduler, sched_getparam, sched_setscheduler, and sched_setparam ports. Preserve PyPy's control-flow shape unless each site is recorded and supported as a CPython 3.14 spec deviation.
AGENTS.md reference: AGENTS.md:L249-L254
Useful? React with 👍 / 👎.
wrap_oserror(..., eintr_retry=True)runs the pending Python signal handlers and re-issues the call; the loop ends only when a handler raises. These sites reported the interruption directly.interp_posix.py—open,stat/fstatby descriptor,fchdir,fsync,fdatasync,waitpid,mkfifo,mknod,fstatvfs,chmod,lchmod,fchmod,chown,lchown,fchown,sched_yield,sched_get_priority_max,sched_get_priority_min,sched_rr_get_interval,sched_getscheduler,sched_setscheduler,sched_getparam,sched_setparam.posix.waitisposix.waitpid(-1, 0)and takes that call's loop.chmodreads anENOTSUP/EOPNOTSUPPrefusal offollow_symlinks=Falseas the modifier being unavailable rather than as an OS error, and reads it ahead of the retry (interp_posix.py:1247-1251).interp_fcntl.py—fcntl,flockandlockftake their descriptor throughspace.c_filedescriptor_wand retry through_raise_error_maybe.ioctltakes its descriptor the same way; it reports through_raise_error_alwaysand does not retry.interp_fileio.py:135-147 _open_fd— the same loop around the open inopen_raw_file.Verification (arm64 linux container, dynasm)
test_eintr: 25 tests, OK (skipped=7). Before this change the suite died on its first test.test_fcntl: 8 errors → 3. The remaining three are allfcntl.fcntl(fd, cmd, bytes), the buffer form the module documents as unimplemented; they fail identically without this change.Not from this change
test.test_iodeadlocks on linux atPyBufferedRandomTest.test_writes_and_reads—W_Lock::acquireon a_pyiothreading.Lock, zero CPU, 8/8 runs. A control binary built with these three files reverted deadlocks 8/8 as well, andPYRE_NO_JIT=1clears it (0/2), so it is a pre-existing JIT-only defect onmain. It goes unnoticed becausebaseline.jsonrecordstest.test_ioasIMPORTERROR, and a module recorded non-PASS is never executed by the runner.— authored by Claude
Summary by CodeRabbit