From 76d14c3bf93bea541ec7a89be7d463dce9f0950d Mon Sep 17 00:00:00 2001 From: ssmithx Date: Thu, 23 Jul 2026 10:50:34 +0000 Subject: [PATCH] fix(mesh): flash RPC could hang forever if the listener wouldn't stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Traced a real "Operation failed" report to its root: MeshService::stop() ran synchronously inside start_flash_job, BEFORE the background task was even spawned — so the RPC call itself blocked on it. The mesh listener's reconnect/multi-candidate-probe loop doesn't check its shutdown signal between candidates, and in this incident it was mid a Meshcore-then- Meshtastic probe cycle when the flash request came in, so stop() never returned. The HTTP request timed out client-side ("connection closed before message completed" in the server log), the frontend surfaced a generic "Operation failed", and — worse — the job had already been inserted into the single-job-guard slot before stop() ran, so with nothing ever spawned to complete it, every subsequent flash attempt failed with "already in progress" until a full service restart. The existing MAX_JOB_DURATION ceiling didn't help here: it only wraps run_flash(), which is reached AFTER stop() returns — so a hang in stop() itself was completely unguarded. Fixed by moving the stop() call inside the spawned background task with its own 20s timeout. The RPC call now always returns immediately once the job is registered, and a slow-to-stop listener fails the job cleanly with a clear message instead of hanging the request and wedging every future attempt behind it. Co-Authored-By: Claude Sonnet 5 --- core/archipelago/src/mesh/flash.rs | 51 ++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/core/archipelago/src/mesh/flash.rs b/core/archipelago/src/mesh/flash.rs index 33d1eaa5..9b6b66a9 100644 --- a/core/archipelago/src/mesh/flash.rs +++ b/core/archipelago/src/mesh/flash.rs @@ -108,6 +108,15 @@ const POST_FLASH_SETTLE_DELAY: std::time::Duration = std::time::Duration::from_s /// multi-hundred-MB transfer. const MAX_JOB_DURATION: std::time::Duration = std::time::Duration::from_secs(15 * 60); +/// How long to wait for MeshService::stop() to release the serial port +/// before giving up. Confirmed live 2026-07-23: the listener's own +/// reconnect/multi-candidate-probe loop doesn't check its shutdown signal +/// between candidates, so stop() can take a while (or, if the loop is +/// wedged, never return) — 20s comfortably covers a normal handshake-probe +/// cycle without leaving a flash request hanging indefinitely if the +/// listener genuinely won't let go. +const STOP_LISTENER_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); + /// Live state for the one flash job that can run at a time. A single global /// slot is sufficient because flashing needs exclusive serial access to the /// one port being flashed — there is no meaningful concept of two concurrent @@ -308,19 +317,41 @@ pub async fn start_flash_job( let job = FlashJob::new(board, family, path.clone()); *handle.write().await = Some(Arc::clone(&job)); - // esptool/archy-rnodeconf need exclusive serial access — release the - // listener's hold on the port before touching it. MeshService::stop() - // already recreates the command channel so a later start() is safe. - { - let mut svc = mesh_service.write().await; - if let Some(s) = svc.as_mut() { - s.stop().await; - } - } - let bg_job = Arc::clone(&job); let bg_service = Arc::clone(mesh_service); let task = tokio::spawn(async move { + // esptool/archy-rnodeconf need exclusive serial access — release + // the listener's hold on the port before touching it. This USED + // TO run synchronously in start_flash_job before the job was even + // spawned, blocking the RPC call itself on s.stop().await — a real + // 2026-07-23 incident: the mesh listener was mid a multi-candidate + // reconnect/probe sequence that doesn't check its shutdown signal + // between candidates, so stop() never returned. The HTTP request + // timed out client-side ("Operation failed"), while the job + // (already inserted into `handle`) was permanently wedged — nothing + // had been spawned yet to ever mark it done, so every later flash + // attempt failed with "already in progress" until a full restart. + // Now this runs inside the spawned task with its own bounded + // timeout, so the RPC call always returns immediately regardless, + // and a slow-to-stop listener fails the job cleanly instead of + // hanging everything downstream of it forever. + let stop_result = tokio::time::timeout(STOP_LISTENER_TIMEOUT, async { + let mut svc = bg_service.write().await; + if let Some(s) = svc.as_mut() { + s.stop().await; + } + }) + .await; + if stop_result.is_err() { + let err = anyhow::anyhow!( + "Mesh listener did not release the serial port within {}s — it may still be mid a reconnect attempt. Try again once mesh.status shows the device idle, or restart the archipelago service if this persists.", + STOP_LISTENER_TIMEOUT.as_secs() + ); + bg_job.push_log(format!("ERROR: {err:#}")).await; + bg_job.fail(&err).await; + return; + } + // Outer ceiling on top of run_flash's own internal timeouts — // belt-and-suspenders so that no future hang (network, subprocess, // anything) can ever wedge the single-flash-job guard permanently