Skip to content

[5895] added code to raise error in sleep if invalid value #5907

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

Open
DominicPrince2003 wants to merge 3 commits into
base: main
Choose a base branch
from
+40 −5
Open

[5895] added code to raise error in sleep if invalid value #5907

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
435159e
[5895] added code to raise error in sleep if invalid value
DominicPrince2003 Jul 6, 2025
cdff22f
Update vm/src/stdlib/time.rs
DominicPrince2003 Jul 6, 2025
a9f152c
changed max sleep for not unix systems
DominicPrince2003 Jul 6, 2025
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
Prev Previous commit
Next Next commit
Update vm/src/stdlib/time.rs 
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
  • Loading branch information
@DominicPrince2003 @gemini-code-assist
DominicPrince2003 and gemini-code-assist[bot] authored Jul 6, 2025
commit cdff22fc76766aeb57032eba713a0c0cad1e9fab
6 changes: 3 additions & 3 deletions vm/src/stdlib/time.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,9 +715,9 @@ mod platform {
} else {
return Err(vm.new_type_error("sleep() argument must be a number"));
};
if !secs.is_finite() || secs < 0.0 || secs > u64::MAX as f64 {
return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
}
if !secs.is_finite() || secs < 0.0 || secs > u64::MAX as f64 {
return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
}
let dur = Duration::from_secs_f64(secs);
Comment on lines +708 to +721
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 6, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential precision issue with u64::MAX conversion (Unix implementation).

Same precision issue exists in the Unix implementation as in the non-Unix version.

Apply the same fix as suggested for the non-Unix implementation:

-    if !secs.is_finite() || secs < 0.0 || secs > u64::MAX as f64 {
-        return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
-    }
+    if !secs.is_finite() {
+        return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
+    }
+    if secs < 0.0 {
+        return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
+    }
+    // Check for values that would overflow when converted to Duration
+    if secs > Duration::MAX.as_secs_f64() {
+        return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
+    }
📝 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
fn sleep(secs: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
// this is basically std::thread::sleep, but that catches interrupts and we don't want to;
// Try to get as float first, if that fails try as integer
let secs = if let Ok(float_val) = f64::try_from_object(vm, secs.clone()) {
float_val
} else if let Ok(int_val) = i64::try_from_object(vm, secs) {
int_val as f64
} else {
return Err(vm.new_type_error("sleep() argument must be a number"));
};
if !secs.is_finite() || secs < 0.0 || secs > u64::MAX as f64 {
return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
}
let dur = Duration::from_secs_f64(secs);
let secs = if let Ok(float_val) = f64::try_from_object(vm, secs.clone()) {
float_val
} else if let Ok(int_val) = i64::try_from_object(vm, secs) {
int_val as f64
} else {
return Err(vm.new_type_error("sleep() argument must be a number"));
};
if !secs.is_finite() {
return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
}
if secs < 0.0 {
return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
}
// Check for values that would overflow when converted to Duration
if secs > Duration::MAX.as_secs_f64() {
return Err(vm.new_value_error("sleep length must be a non-negative finite number"));
}
let dur = Duration::from_secs_f64(secs);
🤖 Prompt for AI Agents 
In vm/src/stdlib/time.rs around lines 708 to 721, the code compares secs to
u64::MAX as f64, which can cause precision issues. To fix this, replace the
comparison with a check against u64::MAX as u128 converted to f64 or use a safer
method to ensure the comparison is accurate without precision loss, similar to
the fix applied in the non-Unix implementation.

let ts = TimeSpec::from(dur);
let res = unsafe { libc::nanosleep(ts.as_ref(), std::ptr::null_mut()) };
Expand Down