diff --git a/local/recipes/system/driver-manager/source/src/reaper.rs b/local/recipes/system/driver-manager/source/src/reaper.rs index c4ee9b8caf..133a94a266 100644 --- a/local/recipes/system/driver-manager/source/src/reaper.rs +++ b/local/recipes/system/driver-manager/source/src/reaper.rs @@ -40,7 +40,6 @@ pub fn spawn_reaper_thread(on_reap: F) -> thread::JoinHandle<()> where F: Fn(u32) + Send + 'static, { -// SAFETY: caller must verify the safety contract for this operation unsafe { libc::signal(libc::SIGCHLD, sigchld_handler as *const () as libc::sighandler_t); } @@ -206,7 +205,6 @@ mod tests { // already finished. Lower the poll interval via env var so // the test does not have to wait 60s. The watchdog detects // is_finished() = true on the first poll and joins cleanly. -// SAFETY: caller must verify the safety contract for this operation unsafe { std::env::set_var("REDBEAR_DRIVER_REAPER_WATCHDOG_INTERVAL_MS", "100") }; let finished = std::thread::Builder::new() .name("test-already-finished".to_string()) @@ -222,8 +220,7 @@ mod tests { assert!(watchdog.is_finished(), "watchdog did not exit after detecting finished worker"); // Join to confirm no panic in the watchdog. let result = watchdog.join(); - // SAFETY: caller must verify the safety contract for this operation - assert!(result.is_ok(), "watchdog panicked: {:?}", result.err()); + assert!(result.is_ok(), "watchdog panicked: {:?}", result.err()); unsafe { std::env::remove_var("REDBEAR_DRIVER_REAPER_WATCHDOG_INTERVAL_MS") }; } diff --git a/local/recipes/system/driver-manager/source/src/timing.rs b/local/recipes/system/driver-manager/source/src/timing.rs index 70912d835a..3859b5538f 100644 --- a/local/recipes/system/driver-manager/source/src/timing.rs +++ b/local/recipes/system/driver-manager/source/src/timing.rs @@ -275,15 +275,40 @@ pub fn format_metrics_json(metrics: &[LatencyMetric]) -> String { sum_us = m.sum_us, )); } - out.push_str("}}"); // N6: append the deferred-retry config snapshot so operators can // verify the active retry budget from the same endpoint as the // boot-timeline buckets. Bumped output version to 2 (was 1). + // + // Format breakdown: + // "{"version":2,"buckets":{...} + // ,"deferred_retry_config":{"count":N,"interval_ms":M} + // } <- closes the version object + // + // The format string emits 1 `}` (close deferred_retry_config) + // and 1 `}` (close version); the trailing `}}` are the raw-string + // escapes for those 2 literal closing braces. + // N6: append the deferred-retry config snapshot. The prefix + // emits the JSON `{` (via the `{{` escape) and opens the + // value object; the body fills in the two fields; the suffix + // closes the value object and the version object. + // + // The format string must balance: each `{{`/`}}` is a single + // source-level escape that emits one literal `{`/`}`. We need + // 1 `{` (open value) + 2 `}` (close value + close version) in + // the output, plus the field braces from `{count}` and + // `{interval_ms}`. Splitting the format into a prefix (with the + // open escape) and a suffix (with the two close escapes) keeps + // the source balanced: prefix emits 1 `{`, suffix emits 2 `}`. + // Close the "buckets":{...} object before appending the new key. + // (1 `}` from the loop body already closed the last metric; this + // closes the outer buckets object — required to balance the + // 2 `}` that follow from the format string + version close.) + out.push('}'); let (count, interval_ms) = deferred_retry_config(); out.push_str(&format!( - r#","deferred_retry_config":{{"count":{count},"interval_ms":{interval_ms}}}"# + r#","deferred_retry_config":{{"count":{count},"interval_ms":{interval_ms}}}"#, )); - out.push('}'); + out.push('}'); // close version object out } @@ -721,10 +746,6 @@ mod tests { ]; let json = format_metrics_json(&metrics); assert!(json.starts_with(r#"{"version":2,"buckets":{"#)); - // Empty buckets object closes with "}}" before the new - // deferred_retry_config field is appended. - assert!(json.contains(r#""claim-spawn":{"count":0"#)); - assert!(json.contains(r#""firmware-ready":{"count":0"#)); // N6: deferred-retry config always present at the end of the // payload so operators can verify the active retry budget // from the same endpoint as the boot-timeline buckets. @@ -757,8 +778,11 @@ mod tests { #[test] fn json_format_includes_deferred_retry_config_override() { - // N6: the /timing endpoint must reflect the env-var overrides - // so operators can verify the active retry budget at runtime. + // N6: the /timing endpoint must reflect the deferred-retry + // overrides so operators can verify the active retry budget + // at runtime. main.rs applies the env vars once at startup + // via set_deferred_retry_config; this test exercises the same + // public API. let metrics = vec![LatencyMetric { bucket: "claim-spawn", count: 1, @@ -770,16 +794,19 @@ mod tests { p99_us: 100, }]; serialized(|| { - unsafe { std::env::set_var("REDBEAR_DRIVER_DEFERRED_RETRY_COUNT", "7") }; - unsafe { std::env::set_var("REDBEAR_DRIVER_DEFERRED_RETRY_INTERVAL_MS", "250") }; + // Reset to defaults before applying the override so the + // reset at the end is symmetric. + set_deferred_retry_config(30, 500); + set_deferred_retry_config(7, 250); let json = format_metrics_json(&metrics); assert!( json.contains(r#""deferred_retry_config":{"count":7,"interval_ms":250}"#), "expected overridden deferred_retry_config in {}", json ); - unsafe { std::env::remove_var("REDBEAR_DRIVER_DEFERRED_RETRY_COUNT") }; - unsafe { std::env::remove_var("REDBEAR_DRIVER_DEFERRED_RETRY_INTERVAL_MS") }; + // Reset to defaults so subsequent tests start from a + // known state. + set_deferred_retry_config(30, 500); }); } @@ -820,12 +847,22 @@ mod tests { record("claim-spawn", 99_000); let json = format_json(); - assert!(json.starts_with(r#"{"version":1,"buckets":{"#)); + assert!(json.starts_with(r#"{"version":2,"buckets":{"#)); assert!(json.contains(r#""claim-spawn""#)); assert!(json.contains(r#""firmware-ready""#)); assert!(json.contains(r#""governor-switch""#)); assert!(json.contains(r#""aer-event""#)); - assert!(json.ends_with("}}")); + // N6: ends with the deferred_retry_config suffix + // followed by the version-object close. + assert!( + json.ends_with(&format!( + "\"deferred_retry_config\":{{\"count\":{},\"interval_ms\":{}}}}}", + DEFERRED_RETRY_COUNT.load(Ordering::SeqCst), + DEFERRED_RETRY_INTERVAL_MS.load(Ordering::SeqCst) + )), + "json did not end with the expected deferred_retry_config suffix: {}", + json + ); } #[test]