From 0e6e6b9ccb177a26d2f53c67c1afa1fb0026716b Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sun, 6 Jul 2025 12:37:37 +0900 Subject: [PATCH 1/4] fix(time): Handle negative sleep values --- vm/src/convert/try_from.rs | 16 +++++++++++++--- vm/src/stdlib/time.rs | 39 ++++++++++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/vm/src/convert/try_from.rs b/vm/src/convert/try_from.rs index 7eb1c9c00d..d2d83b36e7 100644 --- a/vm/src/convert/try_from.rs +++ b/vm/src/convert/try_from.rs @@ -3,6 +3,7 @@ use crate::{ builtins::PyFloat, object::{AsObject, PyObject, PyObjectRef, PyPayload, PyRef, PyResult}, }; +use malachite_bigint::Sign; use num_traits::ToPrimitive; /// Implemented by any type that can be created from a Python object. @@ -124,10 +125,19 @@ impl<'a, T: PyPayload> TryFromBorrowedObject<'a> for &'a Py { impl TryFromObject for std::time::Duration { fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { if let Some(float) = obj.payload::() { - Ok(Self::from_secs_f64(float.to_f64())) + let f = float.to_f64(); + if f < 0.0 { + return Err(vm.new_value_error("negative duration")); + } + Ok(Self::from_secs_f64(f)) } else if let Some(int) = obj.try_index_opt(vm) { - let sec = int? - .as_bigint() + let int = int?; + let bigint = int.as_bigint(); + if bigint.sign() == Sign::Minus { + return Err(vm.new_value_error("negative duration")); + } + + let sec = bigint .to_u64() .ok_or_else(|| vm.new_value_error("value out of range"))?; Ok(Self::from_secs(sec)) diff --git a/vm/src/stdlib/time.rs b/vm/src/stdlib/time.rs index 12a47fca87..67e9648f7b 100644 --- a/vm/src/stdlib/time.rs +++ b/vm/src/stdlib/time.rs @@ -90,8 +90,25 @@ mod decl { #[cfg(not(unix))] #[pyfunction] - fn sleep(dur: Duration) { + fn sleep(seconds: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let dur = seconds.try_into_value::(vm).map_err(|e| { + if e.class().is(vm.ctx.exceptions.value_error) { + // Check if this is a "negative duration" error by examining the args + if let Some(args) = e.args().first() { + if let Ok(s) = args.str(vm) { + if s.as_str() == "negative duration" { + return vm.new_value_error("sleep length must be non-negative"); + } + } + } + e + } else { + e + } + })?; + std::thread::sleep(dur); + Ok(()) } #[cfg(not(target_os = "wasi"))] @@ -525,7 +542,7 @@ mod platform { use super::decl::{SEC_TO_NS, US_TO_NS}; #[cfg_attr(target_os = "macos", allow(unused_imports))] use crate::{ - PyObject, PyRef, PyResult, TryFromBorrowedObject, VirtualMachine, + AsObject, PyObject, PyObjectRef, PyRef, PyResult, TryFromBorrowedObject, VirtualMachine, builtins::{PyNamespace, PyStrRef}, convert::IntoPyException, }; @@ -691,8 +708,22 @@ mod platform { } #[pyfunction] - fn sleep(dur: Duration, vm: &VirtualMachine) -> PyResult<()> { - // this is basically std::thread::sleep, but that catches interrupts and we don't want to; + fn sleep(seconds: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let dur = seconds.try_into_value::(vm).map_err(|e| { + if e.class().is(vm.ctx.exceptions.value_error) { + // Check if this is a "negative duration" error by examining the args + if let Some(args) = e.args().first() { + if let Ok(s) = args.str(vm) { + if s.as_str() == "negative duration" { + return vm.new_value_error("sleep length must be non-negative"); + } + } + } + e + } else { + e + } + })?; let ts = TimeSpec::from(dur); let res = unsafe { libc::nanosleep(ts.as_ref(), std::ptr::null_mut()) }; From 3aa18a964e943d98bde5e43e7f14d347007f3367 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sun, 6 Jul 2025 12:50:38 +0900 Subject: [PATCH 2/4] refactor: simplify error handling in time.sleep using and_then --- vm/src/stdlib/time.rs | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/vm/src/stdlib/time.rs b/vm/src/stdlib/time.rs index 67e9648f7b..f9f2337592 100644 --- a/vm/src/stdlib/time.rs +++ b/vm/src/stdlib/time.rs @@ -93,18 +93,13 @@ mod decl { fn sleep(seconds: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let dur = seconds.try_into_value::(vm).map_err(|e| { if e.class().is(vm.ctx.exceptions.value_error) { - // Check if this is a "negative duration" error by examining the args - if let Some(args) = e.args().first() { - if let Ok(s) = args.str(vm) { - if s.as_str() == "negative duration" { - return vm.new_value_error("sleep length must be non-negative"); - } + if let Some(s) = e.args().first().and_then(|arg| arg.str(vm).ok()) { + if s.as_str() == "negative duration" { + return vm.new_value_error("sleep length must be non-negative"); } } - e - } else { - e } + e })?; std::thread::sleep(dur); @@ -711,18 +706,13 @@ mod platform { fn sleep(seconds: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let dur = seconds.try_into_value::(vm).map_err(|e| { if e.class().is(vm.ctx.exceptions.value_error) { - // Check if this is a "negative duration" error by examining the args - if let Some(args) = e.args().first() { - if let Ok(s) = args.str(vm) { - if s.as_str() == "negative duration" { - return vm.new_value_error("sleep length must be non-negative"); - } + if let Some(s) = e.args().first().and_then(|arg| arg.str(vm).ok()) { + if s.as_str() == "negative duration" { + return vm.new_value_error("sleep length must be non-negative"); } } - e - } else { - e } + e })?; let ts = TimeSpec::from(dur); From 72d612c3ee4159ac3747af26f88fc3fe83770075 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Mon, 7 Jul 2025 00:05:03 +0900 Subject: [PATCH 3/4] time: merge platform-specific sleep implementations --- vm/src/stdlib/time.rs | 45 +++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/vm/src/stdlib/time.rs b/vm/src/stdlib/time.rs index f9f2337592..5481126c1d 100644 --- a/vm/src/stdlib/time.rs +++ b/vm/src/stdlib/time.rs @@ -34,7 +34,7 @@ unsafe extern "C" { #[pymodule(name = "time", with(platform))] mod decl { use crate::{ - PyObjectRef, PyResult, TryFromObject, VirtualMachine, + PyObjectRef, PyResult, TryFromObject, VirtualMachine, AsObject, builtins::{PyStrRef, PyTypeRef}, function::{Either, FuncArgs, OptionalArg}, types::PyStructSequence, @@ -88,7 +88,6 @@ mod decl { duration_since_system_now(vm) } - #[cfg(not(unix))] #[pyfunction] fn sleep(seconds: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let dur = seconds.try_into_value::(vm).map_err(|e| { @@ -102,7 +101,23 @@ mod decl { e })?; - std::thread::sleep(dur); + #[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); + let res = unsafe { libc::nanosleep(ts.as_ref(), std::ptr::null_mut()) }; + let interrupted = res == -1 && nix::Error::last_raw() == libc::EINTR; + + if interrupted { + vm.check_signals()?; + } + } + + #[cfg(not(unix))] + { + std::thread::sleep(dur); + } + Ok(()) } @@ -702,30 +717,6 @@ mod platform { get_clock_time(ClockId::CLOCK_MONOTONIC, vm) } - #[pyfunction] - fn sleep(seconds: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let dur = seconds.try_into_value::(vm).map_err(|e| { - if e.class().is(vm.ctx.exceptions.value_error) { - if let Some(s) = e.args().first().and_then(|arg| arg.str(vm).ok()) { - if s.as_str() == "negative duration" { - return vm.new_value_error("sleep length must be non-negative"); - } - } - } - e - })?; - - let ts = TimeSpec::from(dur); - let res = unsafe { libc::nanosleep(ts.as_ref(), std::ptr::null_mut()) }; - let interrupted = res == -1 && nix::Error::last_raw() == libc::EINTR; - - if interrupted { - vm.check_signals()?; - } - - Ok(()) - } - #[cfg(not(any( target_os = "illumos", target_os = "netbsd", From 60163b200f71d22088b11ab9d4cb4763e17c0a26 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Mon, 7 Jul 2025 00:14:22 +0900 Subject: [PATCH 4/4] remove unused imports --- vm/src/stdlib/time.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vm/src/stdlib/time.rs b/vm/src/stdlib/time.rs index 5481126c1d..0de8648c12 100644 --- a/vm/src/stdlib/time.rs +++ b/vm/src/stdlib/time.rs @@ -34,7 +34,7 @@ unsafe extern "C" { #[pymodule(name = "time", with(platform))] mod decl { use crate::{ - PyObjectRef, PyResult, TryFromObject, VirtualMachine, AsObject, + AsObject, PyObjectRef, PyResult, TryFromObject, VirtualMachine, builtins::{PyStrRef, PyTypeRef}, function::{Either, FuncArgs, OptionalArg}, types::PyStructSequence, @@ -552,7 +552,7 @@ mod platform { use super::decl::{SEC_TO_NS, US_TO_NS}; #[cfg_attr(target_os = "macos", allow(unused_imports))] use crate::{ - AsObject, PyObject, PyObjectRef, PyRef, PyResult, TryFromBorrowedObject, VirtualMachine, + PyObject, PyRef, PyResult, TryFromBorrowedObject, VirtualMachine, builtins::{PyNamespace, PyStrRef}, convert::IntoPyException, };