Skip to content

Fix allow_threads and EINTR handling #7457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 2 commits into from
Mar 20, 2026
+26 −14
Merged

Fix allow_threads and EINTR handling #7457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34b2946
Fix allow_threads and EINTR handling
youknowone Mar 17, 2026
ba58ffc
Remove expectedFailureIfWindows for testHashComparisonOfMethods
youknowone Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Lib/test/_test_eintr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,6 @@ def test_os_open(self):
class TimeEINTRTest(EINTRBaseTest):
""" EINTR tests for the time module. """

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_sleep(self):
t0 = time.monotonic()
time.sleep(self.sleep_time)
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_class.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,6 @@ def assertNotOrderable(self, a, b):
with self.assertRaises(TypeError):
a >= b

@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AssertionError: 1543448294720 != 1543448295392")
def testHashComparisonOfMethods(self):
# Test comparison and hash of methods
class A:
Expand Down
3 changes: 2 additions & 1 deletion crates/stdlib/src/multiprocessing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ mod _multiprocessing {
remaining.min(poll_ms)
};

let res = unsafe { WaitForSingleObjectEx(self.handle.as_raw(), wait_ms, 0) };
let handle = self.handle.as_raw();
let res = vm.allow_threads(|| unsafe { WaitForSingleObjectEx(handle, wait_ms, 0) });

match res {
WAIT_OBJECT_0 => {
Expand Down
35 changes: 24 additions & 11 deletions crates/vm/src/stdlib/time.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,30 @@ mod decl {

#[cfg(unix)]
{
// this is basically std::thread::sleep, but that catches interrupts and we don't want to;
let ts = nix::sys::time::TimeSpec::from(dur);
// Capture errno inside the closure: attach_thread (called by
// allow_threads on return) can clobber errno via syscalls.
let (res, err) = vm.allow_threads(|| {
let r = unsafe { libc::nanosleep(ts.as_ref(), core::ptr::null_mut()) };
(r, nix::Error::last_raw())
});
let interrupted = res == -1 && err == libc::EINTR;

if interrupted {
// Loop on nanosleep, recomputing the
// remaining timeout after each EINTR so that signals don't
// shorten the requested sleep duration.
use std::time::Instant;
let deadline = Instant::now() + dur;
loop {
Comment on lines +122 to +123
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== sleep deadline computation =="
rg -nP --type=rust 'Instant::now\(\)\s*\+\s*dur|checked_add\(' crates/vm/src/stdlib/time.rs -C2

echo
echo "== Duration conversion path used by time.sleep =="
rg -nP --type=rust 'try_into_value::<Duration>|from_secs_f64|try_from_secs_f64|Duration' crates/vm/src/stdlib/time.rs crates/vm/src -C2

echo
echo "== Existing oversized sleep tests/messages =="
rg -nP --type=rust --type=py 'sleep.*(Overflow|too large|overflow|huge)' -C2

Repository: RustPython/RustPython

Length of output: 29666

Use Instant::checked_add() to prevent panic on overflow.

Line 122 uses unchecked Instant::now() + dur, which will panic if the duration is large enough to overflow the Instant. The Duration conversion at line 101 does not validate that the resulting value is within Instant's safe range. Replace with checked_add() and raise OverflowError.

Proposed fix 
             use std::time::Instant;
-            let deadline = Instant::now() + dur;
+            let deadline = Instant::now()
+                .checked_add(dur)
+                .ok_or_else(|| vm.new_overflow_error("sleep length is too large"))?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let deadline = Instant::now() + dur;
loop {
let deadline = Instant::now()
.checked_add(dur)
.ok_or_else(|| vm.new_overflow_error("sleep length is too large"))?;
loop {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/time.rs` around lines 122 - 123, Replace the unchecked
addition Instant::now() + dur when computing deadline with
Instant::now().checked_add(dur) and handle the None case by returning/raising an
OverflowError; specifically, change the code that assigns deadline (the
Instant::now() + dur expression) to use
Instant::now().checked_add(dur).ok_or_else(|| OverflowError::new("timeout
overflow")) (or the crate's equivalent VM error constructor) before entering the
loop so that overflow is detected and an OverflowError is returned instead of
panicking.

let remaining = deadline.saturating_duration_since(Instant::now());
if remaining.is_zero() {
break;
}
let ts = nix::sys::time::TimeSpec::from(remaining);
let (res, err) = vm.allow_threads(|| {
let r = unsafe { libc::nanosleep(ts.as_ref(), core::ptr::null_mut()) };
(r, nix::Error::last_raw())
});
if res == 0 {
break;
}
if err != libc::EINTR {
return Err(
vm.new_os_error(format!("nanosleep: {}", nix::Error::from_raw(err)))
);
}
// EINTR: run signal handlers, then retry with remaining time
vm.check_signals()?;
}
}
Expand Down
Loading