Skip to content

Commit e2abc4c

Browse files
youknowdotyouknowdot
authored
Align faulthandler watchdog timeouts with CPython (#9775)
* Align faulthandler watchdog timeouts with CPython Use the shared signed PyTime conversion and platform bounds, accept timeout/signum keywords, and preserve the watchdog deadline across notifications. Add safe numeric-conversion and notification regression coverage. Assisted-by: OpenAI Codex:model version not exposed * Mark watchdog regression module explicitly as test-only Keep the native-platform guard separate so the tests_outside_test_module Clippy lint recognizes the cfg(test) module. Assisted-by: OpenAI Codex:model version not exposed * Separate faulthandler regression snippets Move watchdog timeout and signal-registration probes into stdlib_faulthandler.py, preserving their assertions and guards. Keep the time and signal snippets focused on their own modules. Assisted-by: OpenAI Codex:model version not exposed
1 parent c367705 commit e2abc4c

3 files changed

Lines changed: 272 additions & 18 deletions

File tree

‎crates/stdlib/src/faulthandler.rs‎

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ mod decl {
66
use crate::vm::{
77
AsObject, PyObjectRef, PyResult, VirtualMachine,
88
builtins::PyInt,
9-
function::{ArgIntoBool, ArgIntoFloat, OptionalArg},
9+
function::{ArgIntoBool, OptionalArg},
10+
stdlib::time::pytime,
1011
};
1112
use alloc::sync::Arc;
1213
use core::sync::atomic::{AtomicBool, AtomicI32, Ordering};
@@ -673,7 +674,8 @@ mod decl {
673674
return;
674675
}
675676
let timeout = Duration::from_micros(guard.timeout_us);
676-
cvar.wait_for(&mut guard, timeout);
677+
// A notification or spurious wake does not mean the deadline passed.
678+
cvar.wait_while_for(&mut guard, |state| !state.cancel, timeout);
677679

678680
// Check if cancelled after wait
679681
if guard.cancel {
@@ -797,8 +799,9 @@ mod decl {
797799
#[derive(FromArgs)]
798800
#[allow(unused)]
799801
struct DumpTracebackLaterArgs {
800-
#[pyarg(positional, error_msg = "timeout must be a number (int or float)")]
801-
timeout: ArgIntoFloat,
802+
// Clinic converts repeat/exit before interpreting the timeout object.
803+
#[pyarg(any)]
804+
timeout: PyObjectRef,
802805
#[pyarg(any, default = ArgIntoBool::FALSE)]
803806
repeat: ArgIntoBool,
804807
#[pyarg(any, default, py_default = "sys.stderr")]
@@ -811,20 +814,21 @@ mod decl {
811814

812815
#[pyfunction]
813816
fn dump_traceback_later(args: DumpTracebackLaterArgs, vm: &VirtualMachine) -> PyResult<()> {
814-
let timeout: f64 = args.timeout.into_float();
815-
816-
if timeout <= 0.0 {
817+
let timeout = pytime::from_seconds_object(&args.timeout, pytime::Round::Up, vm)?;
818+
let timeout_us = pytime::Round::Up.divide(timeout, 1_000);
819+
if timeout_us <= 0 {
817820
return Err(vm.new_value_error("timeout must be greater than 0"));
818821
}
819-
820-
let (fd, _file) = get_fd_from_file_opt(args.file, vm)?;
821-
822-
// Convert timeout to microseconds
823-
let timeout_us = (timeout * 1_000_000.0) as u64;
824-
if timeout_us == 0 {
825-
return Err(vm.new_value_error("timeout must be greater than 0"));
822+
pytime::check_thread_timeout(timeout, vm)?;
823+
// CPython also bounds the seconds passed to format_timeout by LONG_MAX.
824+
#[allow(clippy::unnecessary_cast)] // c_long is i32 or i64.
825+
let max_seconds = core::ffi::c_long::MAX as i64;
826+
if timeout_us / 1_000_000 > max_seconds {
827+
return Err(vm.new_overflow_error("timeout value is too large"));
826828
}
829+
let timeout_us = timeout_us as u64;
827830

831+
let (fd, _file) = get_fd_from_file_opt(args.file, vm)?;
828832
let header = format_timeout(timeout_us);
829833

830834
// Snapshot thread frame slots so watchdog can dump tracebacks
@@ -929,7 +933,7 @@ mod decl {
929933
#[derive(FromArgs)]
930934
#[allow(unused)]
931935
struct RegisterArgs {
932-
#[pyarg(positional)]
936+
#[pyarg(any)]
933937
signum: i32,
934938
#[pyarg(any, default, py_default = "sys.stderr")]
935939
file: OptionalArg<PyObjectRef>,
@@ -1095,4 +1099,57 @@ mod decl {
10951099
suppress_crash_report();
10961100
host_faulthandler::raise_exception(args.code, args.flags);
10971101
}
1102+
1103+
#[cfg(test)]
1104+
#[cfg(any(unix, windows))]
1105+
mod tests {
1106+
use super::*;
1107+
use std::sync::mpsc;
1108+
use std::time::Instant;
1109+
1110+
#[test]
1111+
fn watchdog_notification_does_not_expire_timeout() {
1112+
let state = Arc::new((
1113+
Mutex::new(WatchdogState {
1114+
cancel: false,
1115+
fd: 2,
1116+
timeout_us: 60_000_000,
1117+
max_threads: 100,
1118+
repeat: false,
1119+
exit: false,
1120+
header: String::new(),
1121+
#[cfg(feature = "threading")]
1122+
thread_frame_slots: Vec::new(),
1123+
}),
1124+
Condvar::new(),
1125+
));
1126+
let worker_state = Arc::clone(&state);
1127+
let (done_tx, done_rx) = mpsc::channel();
1128+
let worker = thread::spawn(move || {
1129+
watchdog_thread(worker_state);
1130+
done_tx.send(()).unwrap();
1131+
});
1132+
1133+
// Wait for a real waiter so the notification cannot be lost before
1134+
// the watchdog starts. No Python frames are exposed to this worker.
1135+
let deadline = Instant::now() + Duration::from_secs(5);
1136+
let notified = loop {
1137+
if state.1.notify_one() {
1138+
break true;
1139+
}
1140+
if Instant::now() >= deadline {
1141+
break false;
1142+
}
1143+
thread::sleep(Duration::from_millis(1));
1144+
};
1145+
let completed = done_rx.recv_timeout(Duration::from_millis(100));
1146+
1147+
// Always stop and join before asserting, including failure paths.
1148+
state.0.lock().cancel = true;
1149+
state.1.notify_all();
1150+
worker.join().unwrap();
1151+
assert!(notified, "watchdog did not start waiting");
1152+
assert!(matches!(completed, Err(mpsc::RecvTimeoutError::Timeout)));
1153+
}
1154+
}
10981155
}

‎crates/vm/src/stdlib/time/pytime.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ fn from_object(object: &PyObject, scale: i64, round: Round, vm: &VirtualMachine)
147147

148148
/// Check the platform wait limit after the caller has handled negative values
149149
/// and any API-specific indefinite-wait sentinel.
150-
pub(crate) fn check_thread_timeout(nanoseconds: i64, vm: &VirtualMachine) -> PyResult<()> {
150+
pub fn check_thread_timeout(nanoseconds: i64, vm: &VirtualMachine) -> PyResult<()> {
151151
if Round::Up.divide(nanoseconds, 1_000) > THREAD_TIMEOUT_MAX_MICROSECONDS {
152152
Err(vm.new_overflow_error("timeout value is too large"))
153153
} else {

‎extra_tests/snippets/stdlib_faulthandler.py‎

Lines changed: 199 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,210 @@
1+
import faulthandler
2+
import signal
3+
import struct
14
import sys
25
import time
36

47
from testutils import assert_raises
58

9+
# Faulthandler watchdogs use the same signed-nanosecond conversion as sleep.
10+
11+
12+
def rejected_timeout(expected, message, timeout, **kwargs):
13+
try:
14+
with assert_raises(expected) as caught:
15+
faulthandler.dump_traceback_later(timeout, **kwargs)
16+
assert str(caught.exception) == message
17+
finally:
18+
faulthandler.cancel_dump_traceback_later()
19+
20+
21+
# Keyword binding must reach timeout validation before inspecting the file.
22+
try:
23+
with assert_raises(ValueError) as caught:
24+
faulthandler.dump_traceback_later(timeout=0, file=-1)
25+
assert str(caught.exception) == "timeout must be greater than 0"
26+
assert_raises(TypeError, faulthandler.dump_traceback_later)
27+
assert_raises(TypeError, faulthandler.dump_traceback_later, 60, timeout=60)
28+
assert_raises(TypeError, faulthandler.dump_traceback_later, 60, False, -1, False, 1)
29+
finally:
30+
faulthandler.cancel_dump_traceback_later()
31+
32+
33+
# All probes below reject the file before arming a watchdog, including tiny
34+
# positive durations that could expire before a subsequent cancellation.
35+
file_error = "file is not a valid file descriptor"
36+
for timeout in (1e-20, 5e-324, 1e-9):
37+
rejected_timeout(ValueError, file_error, timeout, file=-1)
38+
for timeout in (0, -0.0, -1, -1e-20, -5e-324):
39+
rejected_timeout(ValueError, "timeout must be greater than 0", timeout, file=-1)
40+
rejected_timeout(ValueError, "Invalid value NaN (not a number)", float("nan"), file=-1)
41+
for timeout in (float("inf"), -float("inf"), 10**100, -(10**100), 9223372037):
42+
rejected_timeout(
43+
OverflowError, "timestamp out of range for C PyTime_t", timeout, file=-1
44+
)
45+
for timeout in (None, "60", object()):
46+
rejected_timeout(
47+
TypeError,
48+
"must be real number, not " + type(timeout).__name__,
49+
timeout,
50+
file=-1,
51+
)
52+
53+
54+
calls = []
55+
56+
57+
class TruthValue:
58+
def __init__(self, name, error=None):
59+
self.name = name
60+
self.error = error
61+
62+
def __bool__(self):
63+
calls.append(self.name)
64+
if self.error is not None:
65+
raise self.error
66+
return False
67+
68+
69+
class IndexedTimeout:
70+
def __index__(self):
71+
calls.append("index")
72+
return 60
73+
74+
def __float__(self):
75+
raise AssertionError("timeout must prefer __index__")
76+
77+
78+
class FloatTimeout:
79+
def __float__(self):
80+
calls.append("float")
81+
return 60.0
82+
83+
84+
class InvalidFile:
85+
def fileno(self):
86+
calls.append("fileno")
87+
return -1
88+
89+
def flush(self):
90+
raise AssertionError("invalid descriptor must not be flushed")
91+
92+
93+
for timeout, conversion in ((IndexedTimeout(), "index"), (FloatTimeout(), "float")):
94+
calls.clear()
95+
rejected_timeout(
96+
RuntimeError,
97+
"file.fileno() is not a valid file descriptor",
98+
timeout,
99+
repeat=TruthValue("repeat"),
100+
exit=TruthValue("exit"),
101+
file=InvalidFile(),
102+
)
103+
assert calls == ["repeat", "exit", conversion, "fileno"]
104+
105+
for failing_argument in ("repeat", "exit"):
106+
calls.clear()
107+
failure = RuntimeError(failing_argument + " failed")
108+
rejected_timeout(
109+
RuntimeError,
110+
str(failure),
111+
None,
112+
repeat=TruthValue("repeat", failure if failing_argument == "repeat" else None),
113+
exit=TruthValue("exit", failure if failing_argument == "exit" else None),
114+
file=InvalidFile(),
115+
)
116+
assert calls == (["repeat"] if failing_argument == "repeat" else ["repeat", "exit"])
117+
118+
for timeout, expected, message in (
119+
(0, ValueError, "timeout must be greater than 0"),
120+
(float("nan"), ValueError, "Invalid value NaN (not a number)"),
121+
(float("inf"), OverflowError, "timestamp out of range for C PyTime_t"),
122+
):
123+
calls.clear()
124+
rejected_timeout(
125+
expected,
126+
message,
127+
timeout,
128+
repeat=TruthValue("repeat"),
129+
exit=TruthValue("exit"),
130+
file=InvalidFile(),
131+
)
132+
assert calls == ["repeat", "exit"]
133+
134+
135+
class FailingIndex:
136+
def __index__(self):
137+
raise RuntimeError("index failed")
138+
139+
def __float__(self):
140+
raise AssertionError("failed index must not fall back to float")
141+
142+
143+
class FailingFloat:
144+
def __float__(self):
145+
raise RuntimeError("float failed")
146+
147+
148+
class InvalidIndex:
149+
def __index__(self):
150+
return 60.0
151+
152+
def __float__(self):
153+
raise AssertionError("invalid index must not fall back to float")
154+
155+
156+
rejected_timeout(RuntimeError, "index failed", FailingIndex(), file=-1)
157+
rejected_timeout(RuntimeError, "float failed", FailingFloat(), file=-1)
158+
try:
159+
assert_raises(TypeError, faulthandler.dump_traceback_later, InvalidIndex(), file=-1)
160+
finally:
161+
faulthandler.cancel_dump_traceback_later()
162+
163+
164+
class FloatSubclass(float):
165+
def __float__(self):
166+
raise AssertionError("float subclass override must be bypassed")
167+
168+
169+
class IndexedFloatSubclass(FloatSubclass):
170+
def __index__(self):
171+
return -1
172+
173+
174+
rejected_timeout(ValueError, file_error, FloatSubclass(60), file=-1)
175+
rejected_timeout(
176+
ValueError, "timeout must be greater than 0", IndexedFloatSubclass(60), file=-1
177+
)
178+
179+
# The platform limit and the signed C long seconds limit precede file lookup.
180+
# This is below the signed-nanosecond limit on every tested platform.
181+
if sys.platform == "win32":
182+
rejected_timeout(OverflowError, "timeout value is too large", 4_294_968, file=-1)
183+
elif struct.calcsize("l") == 4:
184+
rejected_timeout(OverflowError, "timeout value is too large", 2**31, file=-1)
185+
else:
186+
rejected_timeout(ValueError, file_error, 9223372036, file=-1)
187+
188+
# Exercise the successful keyword path only with a long timeout, immediately
189+
# cancelled. Wasm targets do not provide the native watchdog thread.
190+
if sys.platform not in ("wasi", "emscripten"):
191+
try:
192+
assert faulthandler.dump_traceback_later(timeout=60) is None
193+
finally:
194+
faulthandler.cancel_dump_traceback_later()
195+
196+
197+
# An invalid file proves register's signum keyword binds without installing
198+
# any signal handler. This API is absent on Windows.
199+
if hasattr(faulthandler, "register") and hasattr(signal, "SIGUSR1"):
200+
with assert_raises(ValueError) as caught:
201+
faulthandler.register(signum=signal.SIGUSR1, file=-1)
202+
assert str(caught.exception) == "file is not a valid file descriptor"
203+
204+
6205
# The watchdog's thread cap is keyword-only and uses Py_ssize_t conversion.
7206
# Its output must distinguish an exact cap from a truncated thread dump.
8207
if sys.platform not in ("wasi", "emscripten"):
9-
import faulthandler
10-
11208
for cap in (None, 1.5):
12209
assert_raises(TypeError, faulthandler.dump_traceback_later, 60, max_threads=cap)
13210
for cap in (sys.maxsize + 1, -sys.maxsize - 2):

0 commit comments

Comments
 (0)