diff --git a/core/archipelago/src/mesh/mod.rs b/core/archipelago/src/mesh/mod.rs index 4b2b76bf..ae2d9389 100644 --- a/core/archipelago/src/mesh/mod.rs +++ b/core/archipelago/src/mesh/mod.rs @@ -38,6 +38,14 @@ use tokio::sync::watch; use tracing::{error, info, warn}; const MESH_CONFIG_FILE: &str = "mesh-config.json"; + +/// How long `MeshService::stop()` waits for the listener task to notice its +/// shutdown signal and exit gracefully before force-aborting it. See +/// `stop()`'s doc comment for the real incident this guards against: without +/// a hard abort fallback, a slow-to-notice listener could be left running +/// forever, orphaned, racing a later independently-started listener on the +/// same serial port. +const LISTENER_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(15); const MESH_IGNORED_RADIO_FILE: &str = "mesh-ignored-radio-contacts.json"; const MESH_CONTACTS_FILE: &str = "mesh-contacts.json"; @@ -967,8 +975,36 @@ impl MeshService { if let Some(tx) = self.shutdown_tx.take() { let _ = tx.send(true); } - if let Some(handle) = self.listener_handle.take() { - let _ = handle.await; + if let Some(mut handle) = self.listener_handle.take() { + // Bounded wait for graceful shutdown, with a hard abort as + // fallback — confirmed live 2026-07-23: a caller-side timeout + // wrapping stop() (mesh::flash's STOP_LISTENER_TIMEOUT) cancelled + // this await when the listener was slow to notice its shutdown + // signal (mid multi-candidate probe), but `.take()` above had + // already cleared `listener_handle` to None — so MeshService + // believed it was stopped while the task kept running, orphaned + // (dropping a JoinHandle does not abort the task it points to). + // A later start() then spawned a second, fully independent + // listener session racing the orphaned one on the same serial + // port — neither could ever get a clean response, so every + // mesh.configure/probe against that device failed indefinitely + // even though the device itself was fine. + // + // Awaiting `&mut handle` (not `handle` by value) is what makes + // the fallback possible: the Future is polled through the + // reference, so if the timeout fires, this task's own `handle` + // binding is still ours to call `.abort()` on afterward — + // unlike moving `handle` into the timeout future outright, which + // would drop (and thus orphan) it on timeout with nothing left + // to abort. + if tokio::time::timeout(LISTENER_SHUTDOWN_TIMEOUT, &mut handle) + .await + .is_err() + { + warn!("Mesh listener did not shut down gracefully in time — aborting it"); + handle.abort(); + let _ = handle.await; + } } if let Some(handle) = self.deadman_handle.take() { handle.abort();