fix(kernel): harden task lifecycle (panics, locks, races)#4122
Merged
Conversation
Five related kernel-side fixes around spawned-task lifecycle and shared-state safety, surfaced by #3753, #3740, #3739, #3445, #3447. * persist_runs_async (workflow): outer JoinError from spawn_blocking was logged-and-swallowed. Now returns Result and the two execute callers propagate the persist failure into the run result instead of pretending success. Closes #3753. * supervised_spawn module + spawn_logged consolidation: introduce spawn_supervised(name, future) that catch_unwinds the future and emits an error! with the task name on panic. Existing in-file spawn_logged becomes a thin alias. Migrated ~14 critical fire-and-forget sites (inbox watcher + dispatch, whatsapp gateway supervisor, session stream fan-out, auto_dream scheduler/dispatch /run, owner notify, auto-memorize, several daemon-loop tickers inside kernel boot, mcp connect/reconnect, a2a discover, approval resolution, memory event publish, cron agent turn, staggered background-agent start). Remaining raw tokio::spawn calls fall into three buckets that don't fit fire-and-forget: tests, handles collected for join (Vec<JoinHandle> in MCP key probing), and closures whose JoinHandle is the contract (background.start_agent). Partially addresses #3740. * running_tasks insert overwrite: kept the existing atomic insert-then-abort-displaced pattern (correct under DashMap shard lock) and documented it. Closes #3739. * running_tasks insert-after-spawn race: gave RunningTask a unique task_id (Uuid). The task's own cleanup uses remove_if(... task_id matches ...) so a slow predecessor can never delete a successor's entry, and the registration site skips insert when handle.is_finished() so a task that completed before we got to register doesn't leave a stale AbortHandle behind. Closes #3445. * budget_config RwLock poison: read/write paths already used unwrap_or_else(|p| p.into_inner()) but did so silently. Added a tracing::warn on the recovery path so operators can see when a poisoned-then-recovered lock is masking an upstream panic. Closes #3447.
The module-level doc had a paragraph where one line started with "+ \`catch_unwind\`". Markdown parsers (and rustdoc/clippy) read the "+ " as a list-item bullet, then flag the next three continuation lines as `doc_lazy_continuation` (list items need indent). Replacing "+" with "plus" keeps the prose identical and lets the paragraph parse as plain text again.
- kernel/mod.rs: document the residual is_finished() race window and explain why it is harmless (AbortHandle::abort on a finished task is no-op; next turn for the same session overwrites the entry). - supervised_spawn.rs: add doc note that on panic the future is dropped and not retried — this wrapper is a diagnostic aid, not a restart mechanism.
houko
force-pushed
the
fix/kernel-task-lifecycle
branch
from
April 30, 2026 22:26
2b9edd8 to
ac8513b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five kernel-side fixes around spawned-task lifecycle, lock recovery,
and per-session task tracking.
persist_runs_async: stop swallowing the outerJoinErrorfromspawn_blocking. Now returnsResult<(), String>and bothexecute_run/resume_runpropagate persistence failures into therun result. Closes persist_runs_async swallows JoinError from spawn_blocking — workflow persistence panics never surface #3753.
spawn_supervisedhelper in a newsupervised_spawnmodule:wraps a fire-and-forget future in
AssertUnwindSafe(...).catch_unwind()and emits a tagged
error!on panic. Existingspawn_loggedbecomes a thin alias. ~14 critical-path fire-and-forget sites
migrated (inbox, whatsapp gateway, session stream fan-out, auto_dream,
cron agent turns, mcp connect/reconnect, a2a discover, approval
resolution, memory event publish, daemon-loop tickers around audit
trim / metering cleanup / memory consolidation+decay / GC sweep /
session retention / upload cleanup / heartbeat / local provider
probe / staggered background-agent start). Remaining raw
tokio::spawncalls are in three buckets that don't fitfire-and-forget: test code, handles collected for join, and
closures whose
JoinHandleis the contract. Partially closes tokio::spawn at ~30 sites swallows panics — no JoinHandle awaited, supervisor never notified #3740.running_tasksoverwrite (running_tasks DashMap.insert overwrites previous task — stop_agent_run can't kill the orphaned LLM call #3739): kept the existing atomicinsert-then-abort-displaced pattern under the DashMap shard lock
and documented why it's correct. Closes running_tasks DashMap.insert overwrites previous task — stop_agent_run can't kill the orphaned LLM call #3739.
running_tasksinsert-after-spawn race (running_tasks insert-after-spawn race leaves stale abort handle #3445): gaveRunningTaska uniquetask_id(Uuid). Cleanup usesremove_if(... task_id matches ...)so a slow predecessor can'tevict a successor, and the registration site skips insert when
handle.is_finished()so a task that completed before we got toregister can't leave a stale
AbortHandle. Closes running_tasks insert-after-spawn race leaves stale abort handle #3445.budget_configpoison (budget_config RwLock .unwrap() panics on poison, breaking every LLM turn #3447): theRwLockread/write pathsalready recovered with
unwrap_or_else(|p| p.into_inner()), butsilently. Added
tracing::warnso operators can see when anupstream panic poisoned the lock. Closes budget_config RwLock .unwrap() panics on poison, breaking every LLM turn #3447.
Test plan
(e.g. inbox watcher) — verify
error!shows up withtask=inbox_watchersend_message_fullcalls for the same(agent, session)and verify the olderAbortHandleis the oneaborted, and that cleanup of the older task does not evict the
new entry
RwLockwrite critical section (in atest) and verify the next
budget_config()call logs a warnand proceeds with the recovered state