From 5e59421e929ed00cd8ba37874503af2971c546d6 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Fri, 26 Jun 2026 17:56:24 +0300 Subject: [PATCH] test(plugin): drop Dispatch floor in timeout test; doc stream-close path The call-timeout test asserted the real 10 s Dispatch floor, adding 10 s+ to every suite run. Add a cfg(test)-only OCS_PLUGIN_TEST_FLOOR_SECS seam so the test fires at its 1 s base instead; production still enforces the hard safety floors. Test now finishes in ~1 s. Also document in mark_dead that the host-side socket is closed indirectly by killing the child (which unblocks and drops the detached reader thread's Stream), not by the take() in mark_dead itself. Co-Authored-By: Claude Opus 4.8 --- crates/ocs_plugin_api/src/process.rs | 35 ++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/ocs_plugin_api/src/process.rs b/crates/ocs_plugin_api/src/process.rs index 05b5b11d0..4f8da2285 100644 --- a/crates/ocs_plugin_api/src/process.rs +++ b/crates/ocs_plugin_api/src/process.rs @@ -48,7 +48,20 @@ fn call_timeout() -> Duration { /// Per-request-kind timeout floors. The user-configured default is raised to /// these minima so that no request kind can be configured into an unsafe value. fn request_timeout(kind: &'static str) -> Duration { - let base = call_timeout(); + base_max_floor(call_timeout(), kind) +} + +fn base_max_floor(base: Duration, kind: &'static str) -> Duration { + // Tests lower the floor via OCS_PLUGIN_TEST_FLOOR_SECS so the suite does not + // wait out the real 10 s Dispatch minimum. The seam is compiled in only + // under cfg(test); production always enforces the safety floors below. + #[cfg(test)] + if let Some(secs) = std::env::var("OCS_PLUGIN_TEST_FLOOR_SECS") + .ok() + .and_then(|s| s.parse().ok()) + { + return base.max(Duration::from_secs(secs)); + } let floor = match kind { "GetManifest" | "GetRibbon" => Duration::from_secs(5), "Dispatch" => Duration::from_secs(10), @@ -373,6 +386,12 @@ fn shutdown_error() -> PluginError { /// blocking the caller. After this the process is considered dead and any /// further IPC will fail. fn mark_dead(stream: &Mutex>, child: &Mutex>) { + // On a timeout the live `Stream` is owned by the detached reader thread, so + // this `take()` usually clears an already-`None` slot. The host end is not + // closed here directly: killing the child below shuts its socket end, which + // unblocks the reader thread's `recv` and lets it drop the `Stream`. If the + // kill fails the reader thread can stay parked until the OS tears the socket + // down, but the process is still treated as dead for all further IPC. let _ = stream.lock().unwrap_or_else(|e| e.into_inner()).take(); if let Some(child) = child.lock().unwrap_or_else(|e| e.into_inner()).take() { reap(child); @@ -734,7 +753,11 @@ mod timeout_tests { fn dispatch_call_timeout_marks_process_dead() { let _env_guard = ENV_LOCK.lock().expect("env lock"); let prev = std::env::var("OCS_PLUGIN_CALL_TIMEOUT_SECS").ok(); + let prev_floor = std::env::var("OCS_PLUGIN_TEST_FLOOR_SECS").ok(); std::env::set_var("OCS_PLUGIN_CALL_TIMEOUT_SECS", "1"); + // Drop the Dispatch floor to 0 so the test fires at the 1 s base instead + // of waiting out the real 10 s safety floor. + std::env::set_var("OCS_PLUGIN_TEST_FLOOR_SECS", "0"); let (process, runner_stream) = fake_process(); let _runner = thread::spawn(move || { @@ -765,11 +788,11 @@ mod timeout_tests { "expected Dispatch timeout, got {result:?}" ); assert!( - elapsed >= Duration::from_secs(10), - "timeout should respect the 10 s Dispatch floor: {elapsed:?}" + elapsed >= Duration::from_secs(1), + "timeout should respect the 1 s base: {elapsed:?}" ); assert!( - elapsed < Duration::from_secs(12), + elapsed < Duration::from_secs(3), "timed out too slowly: {elapsed:?}" ); assert!(!process.is_alive(), "process should be marked dead"); @@ -782,6 +805,10 @@ mod timeout_tests { Some(v) => std::env::set_var("OCS_PLUGIN_CALL_TIMEOUT_SECS", v), None => std::env::remove_var("OCS_PLUGIN_CALL_TIMEOUT_SECS"), } + match prev_floor { + Some(v) => std::env::set_var("OCS_PLUGIN_TEST_FLOOR_SECS", v), + None => std::env::remove_var("OCS_PLUGIN_TEST_FLOOR_SECS"), + } } #[test]