From f2d9a026db453899e5bb50f101b1f5f1a9f91ddf Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 13 Sep 2026 15:40:50 +0200 Subject: fix: hold every background task, in both codebases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit asyncio keeps only a weak reference to a task, so a coroutine started with `asyncio.ensure_future(...)` whose result is discarded can be collected while it is still running: the loop logs "Task was destroyed but it is pending!" and the work simply does not happen. No error reaches the caller, and what is lost is whatever that coroutine was in the middle of. The node already had a guard for this, written after an abandoned stream task lost a transcode slot for good — and it read one file, `webrtc_server.py`, because that is where the defect was found. Outside that file there were nineteen sites: the hub's `chat_notify` (a notification for every member of a group), the indexer's debounce (every real-time index update), eleven in `daemon.py` including the SIGHUP reload and each enrichment pass, two in `ops.py`, and five in the loopback API. `meshbay_common.background.spawn()` is the one door. It holds the task, drops it when it finishes, and logs what it raised under the coroutine's own name — an exception in a task nobody awaits was otherwise reported by asyncio at collection time, out of context or not at all. A peer session's `_spawn` stays as it is: that one can also *cancel* what it holds, which a module-level holder cannot, because a session ends and a process does not. `test_background_tasks.py` walks every package's source and refuses a discarded handle. It parses rather than greps, so an assignment, a comprehension or an await is not mistaken for one, and it was checked against a deliberate reintroduction. A guard that stops at the edge of the file where the bug was found is a guard against that bug, not against its class. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UMxEQadpzPkYLFf5CYKhpW --- .../src/meshbay_common/background.py | 70 ++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 packages/meshbay-common/src/meshbay_common/background.py (limited to 'packages/meshbay-common/src') diff --git a/packages/meshbay-common/src/meshbay_common/background.py b/packages/meshbay-common/src/meshbay_common/background.py new file mode 100644 index 0000000..1f25dfd --- /dev/null +++ b/packages/meshbay-common/src/meshbay_common/background.py @@ -0,0 +1,70 @@ +""" +Background work that outlives the call which started it. + +**asyncio keeps only a weak reference to a task.** A coroutine started with +`asyncio.ensure_future(...)` whose result is thrown away can therefore be +collected while it is still running: the loop logs "Task was destroyed but it is +pending!" and the work simply does not happen. There is no error at the caller, +nothing fails, and what was lost is whatever that coroutine was in the middle of +— a notification never written, a file never indexed, a reload half applied. + +`spawn()` is the one door for that shape. The reference in `_running` is what +keeps the task alive; the done callback is what stops the set growing. + +Two things it deliberately does **not** replace: + +- A task whose handle the caller keeps — the daemon's service tasks, the + indexer's reconcile loop — is already held, and being held is the whole point. +- A task scoped to something that can end, where the owner must be able to + **cancel** it: a peer session's spawns die with the session + (`WebRTCPeerSession._spawn`), and an enrichment run is tracked by its own + module. Those keep their own sets, because a module-level one can only ever + grow and forget, never shut down. + +It also logs what a discarded task swallowed. An exception in a task nobody +awaits is reported by asyncio at collection time, out of context and often not +at all; here it is logged where it happened, naming the work. +""" + +from __future__ import annotations + +import asyncio +import logging +from typing import Any, Coroutine + +log = logging.getLogger(__name__) + +# Strong references to everything in flight. Module-level on purpose: these are +# tasks with no owner to hold them, which is exactly why they need this. +_running: set[asyncio.Task] = set() + + +def spawn(coro: Coroutine[Any, Any, Any], *, what: str = "") -> asyncio.Task: + """Run a coroutine in the background and hold on to it until it finishes. + + The work names itself in the log: `what` defaults to the coroutine's own + qualified name, so no call site has to carry a label for the sake of a line + nobody reads until something breaks. Pass one only where the coroutine's + name is not the useful answer. + """ + if not what: + what = getattr(coro, "__qualname__", "") or repr(coro) + task = asyncio.ensure_future(coro) + _running.add(task) + task.add_done_callback(lambda t: _finished(t, what)) + return task + + +def _finished(task: asyncio.Task, what: str) -> None: + _running.discard(task) + if task.cancelled(): + return + exc = task.exception() + if exc is not None: + log.error("Background task %s failed: %s", what or "(unnamed)", exc, + exc_info=exc) + + +def pending() -> int: + """How many background tasks are in flight. For diagnostics and tests.""" + return len(_running) -- cgit v1.2.3