fix(mesh): orphaned listener task could race a fresh one on the same port

Traced why a device that's alive and USB-enumerating correctly could still
never complete a single protocol handshake, indefinitely: journal logs
showed genuinely concurrent connection attempts on the same port
(duplicate "Opened serial port"/"Starting X handshake" lines within
microseconds of each other, from what should be sequential probe steps) —
two independent listener sessions were racing on the same tty, each
corrupting the other's reads/writes.

Root cause was in MeshService::stop() combined with mesh::flash's earlier
STOP_LISTENER_TIMEOUT fix: stop() calls `self.listener_handle.take()`
(clearing the field to None immediately) and then awaits the handle with
no bound of its own. When a caller wraps the whole stop() call in a
timeout (as the flash job does, to avoid hanging the RPC response), a slow
listener — mid multi-candidate probe when the shutdown signal arrives —
would cause that outer timeout to cancel the await. But by then
`listener_handle` was already None, so MeshService believed the listener
was stopped, while the actual task kept running, orphaned (dropping a
JoinHandle does not abort the task). A later start() then spawned a
genuinely new listener, racing the orphaned one forever on the same port.

Fixed at the source: stop() now awaits its own handle through a mutable
reference (not by value) with its own bounded timeout, so on timeout it
still owns the handle and can call .abort() on it directly — guaranteeing
the task is actually gone before stop() returns, regardless of how any
caller wraps it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
ssmithx 2026-07-23 18:39:33 +00:00
parent 7547d03166
commit b14af20d1a

View File

@ -39,6 +39,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";
@ -976,8 +984,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();