Replace subtraction of Instants and Durations with saturating subtractions (#12549)

# Description
Duration can not be negative, and an underflow causes a panic.

This should fix #12539 as from what I can tell that bug was caused in
`nu-explore:📟:events` from subtracting durations, but I figured
this might be more widespread, and saturating to zero generally makes
sense.

I also added the relevant clippy lint to try to prevent this from
happening in the future. I can't think of a reason we would ever want to
subtract durations without checking first.

cc @fdncred

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
This commit is contained in:
Devyn Cairns 2024-04-17 05:25:16 -07:00 committed by GitHub
parent 410f3c5c8a
commit 13160b3ec3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 15 additions and 9 deletions

View file

@ -10,7 +10,7 @@ env:
NUSHELL_CARGO_PROFILE: ci NUSHELL_CARGO_PROFILE: ci
NU_LOG_LEVEL: DEBUG NU_LOG_LEVEL: DEBUG
# If changing these settings also change toolkit.nu # If changing these settings also change toolkit.nu
CLIPPY_OPTIONS: "-D warnings -D clippy::unwrap_used" CLIPPY_OPTIONS: "-D warnings -D clippy::unwrap_used -D clippy::unchecked_duration_subtraction"
concurrency: concurrency:
group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }} group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }}

View file

@ -63,7 +63,10 @@ impl Command for TimeIt {
let end_time = Instant::now(); let end_time = Instant::now();
let output = Value::duration((end_time - start_time).as_nanos() as i64, call.head); let output = Value::duration(
end_time.saturating_duration_since(start_time).as_nanos() as i64,
call.head,
);
Ok(output.into_pipeline_data()) Ok(output.into_pipeline_data())
} }

View file

@ -47,7 +47,7 @@ impl UIEvents {
} }
let time_spent = now.elapsed(); let time_spent = now.elapsed();
let rest = self.tick_rate - time_spent; let rest = self.tick_rate.saturating_sub(time_spent);
Self { tick_rate: rest }.next() Self { tick_rate: rest }.next()
} }

View file

@ -106,7 +106,7 @@ pub fn collect_proc(interval: Duration, _with_thread: bool) -> Vec<ProcessInfo>
let curr_stat = curr_proc.stat().ok(); let curr_stat = curr_proc.stat().ok();
let curr_status = curr_proc.status().ok(); let curr_status = curr_proc.status().ok();
let curr_time = Instant::now(); let curr_time = Instant::now();
let interval = curr_time - prev_time; let interval = curr_time.saturating_duration_since(prev_time);
let ppid = curr_proc.stat().map(|p| p.ppid).unwrap_or_default(); let ppid = curr_proc.stat().map(|p| p.ppid).unwrap_or_default();
let curr_proc = ProcessTask::Process(curr_proc); let curr_proc = ProcessTask::Process(curr_proc);
@ -203,7 +203,8 @@ impl ProcessInfo {
let curr_time = cs.utime + cs.stime; let curr_time = cs.utime + cs.stime;
let prev_time = ps.utime + ps.stime; let prev_time = ps.utime + ps.stime;
let usage_ms = (curr_time - prev_time) * 1000 / procfs::ticks_per_second(); let usage_ms =
curr_time.saturating_sub(prev_time) * 1000 / procfs::ticks_per_second();
let interval_ms = let interval_ms =
self.interval.as_secs() * 1000 + u64::from(self.interval.subsec_millis()); self.interval.as_secs() * 1000 + u64::from(self.interval.subsec_millis());
usage_ms as f64 * 100.0 / interval_ms as f64 usage_ms as f64 * 100.0 / interval_ms as f64

View file

@ -93,7 +93,7 @@ pub fn collect_proc(interval: Duration, _with_thread: bool) -> Vec<ProcessInfo>
let curr_res = pidrusage::<RUsageInfoV2>(pid).ok(); let curr_res = pidrusage::<RUsageInfoV2>(pid).ok();
let curr_time = Instant::now(); let curr_time = Instant::now();
let interval = curr_time - prev_time; let interval = curr_time.saturating_duration_since(prev_time);
let ppid = curr_task.pbsd.pbi_ppid as i32; let ppid = curr_task.pbsd.pbi_ppid as i32;
let proc = ProcessInfo { let proc = ProcessInfo {
@ -383,7 +383,7 @@ impl ProcessInfo {
self.curr_task.ptinfo.pti_total_user + self.curr_task.ptinfo.pti_total_system; self.curr_task.ptinfo.pti_total_user + self.curr_task.ptinfo.pti_total_system;
let prev_time = let prev_time =
self.prev_task.ptinfo.pti_total_user + self.prev_task.ptinfo.pti_total_system; self.prev_task.ptinfo.pti_total_user + self.prev_task.ptinfo.pti_total_system;
let usage_ticks = curr_time - prev_time; let usage_ticks = curr_time.saturating_sub(prev_time);
let interval_us = self.interval.as_micros(); let interval_us = self.interval.as_micros();
let ticktime_us = mach_ticktime() / 1000.0; let ticktime_us = mach_ticktime() / 1000.0;
usage_ticks as f64 * 100.0 * ticktime_us / interval_us as f64 usage_ticks as f64 * 100.0 * ticktime_us / interval_us as f64

View file

@ -198,7 +198,7 @@ pub fn collect_proc(interval: Duration, _with_thread: bool) -> Vec<ProcessInfo>
let priority = get_priority(handle); let priority = get_priority(handle);
let curr_time = Instant::now(); let curr_time = Instant::now();
let interval = curr_time - prev_time; let interval = curr_time.saturating_duration_since(prev_time);
let mut all_ok = true; let mut all_ok = true;
all_ok &= command.is_some(); all_ok &= command.is_some();
@ -1059,7 +1059,7 @@ impl ProcessInfo {
let curr_time = self.cpu_info.curr_sys + self.cpu_info.curr_user; let curr_time = self.cpu_info.curr_sys + self.cpu_info.curr_user;
let prev_time = self.cpu_info.prev_sys + self.cpu_info.prev_user; let prev_time = self.cpu_info.prev_sys + self.cpu_info.prev_user;
let usage_ms = (curr_time - prev_time) / 10000u64; let usage_ms = curr_time.saturating_sub(prev_time) / 10000u64;
let interval_ms = self.interval.as_secs() * 1000 + u64::from(self.interval.subsec_millis()); let interval_ms = self.interval.as_secs() * 1000 + u64::from(self.interval.subsec_millis());
usage_ms as f64 * 100.0 / interval_ms as f64 usage_ms as f64 * 100.0 / interval_ms as f64
} }

View file

@ -48,6 +48,7 @@ export def clippy [
-- --
-D warnings -D warnings
-D clippy::unwrap_used -D clippy::unwrap_used
-D clippy::unchecked_duration_subtraction
) )
if $verbose { if $verbose {
@ -73,6 +74,7 @@ export def clippy [
-- --
-D warnings -D warnings
-D clippy::unwrap_used -D clippy::unwrap_used
-D clippy::unchecked_duration_subtraction
) )
} catch { } catch {