You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
P1/P2 follow-ups from the concurrency audit. P0 siblings: #5125 (agent_send transitive cycle), #5126 (channel_send mirror deadlock).
Sub-items
kill_agent leaks in-flight streaming tasks and never aborts them — crates/librefang-kernel/src/kernel/agent_runtime.rs:542-619 (vs. suspend_agent at :210, which does call stop_agent_run). kill_agent_with_purge removes registry/scheduler entries but never invokes stop_agent_run(agent_id). The matching GC cleanup at crates/librefang-kernel/src/kernel/accessors.rs:1065-1071 removes running_tasks entries for dead agents but the loop never calls e.value().abort.abort() — abort handles are dropped, not fired. Killed agent's in-flight LLM stream continues burning tokens until the provider returns. Fix: call stop_agent_run(agent_id) before scheduler/registry removal; in GC cleanup, fire abort() on the handle before removing the map entry.
std::env::set_var inside spawn_blocking is UB on Rust 1.74+ — crates/librefang-api/src/secrets_env.rs:98-106, crates/librefang-api/src/routes/skills.rs:2841-2845,3234-3238 — comments claim // SAFETY: spawn_blocking serialises the env mutation but spawn_blocking dispatches across the blocking thread pool and does NOT serialize. Two concurrent route invocations (channel-bridge reload + secret PATCH) can each call unsafe { std::env::set_var(...) } on different blocking threads — Rust 1.74's std::env::set_var docs declare this undefined behavior in the presence of any concurrent reader. Fix: serialize through a tokio::sync::Mutex<()> held across the set_var, or move env mutations to startup-only and use a process-internal env-var registry for runtime changes.
AppState.bridge_manager reload drops old BridgeManager without .stop() under load — crates/librefang-api/src/channel_bridge.rs:4035-4039 — Arc::try_unwrap(old) only succeeds when no other strong references exist; any in-flight routes/agents.rs:6347 (state.bridge_manager.load_full() held across bm.push_message(...).await) keeps the Arc alive, so try_unwrap returns Err, the if let Ok(Some(...)) arm is skipped, and the old BridgeManager's tokio tasks are never stopped — they leak until Arc strong count hits 1 (potentially never on a busy channel). Fix: always call old.stop() regardless of try_unwrap's outcome; stop() should be safe to call on the still-shared Arc (it abort()s tasks, doesn't move).
Summary
P1/P2 follow-ups from the concurrency audit. P0 siblings: #5125 (
agent_sendtransitive cycle), #5126 (channel_sendmirror deadlock).Sub-items
kill_agentleaks in-flight streaming tasks and never aborts them —crates/librefang-kernel/src/kernel/agent_runtime.rs:542-619(vs.suspend_agentat:210, which does callstop_agent_run).kill_agent_with_purgeremoves registry/scheduler entries but never invokesstop_agent_run(agent_id). The matching GC cleanup atcrates/librefang-kernel/src/kernel/accessors.rs:1065-1071removesrunning_tasksentries for dead agents but the loop never callse.value().abort.abort()— abort handles are dropped, not fired. Killed agent's in-flight LLM stream continues burning tokens until the provider returns. Fix: callstop_agent_run(agent_id)before scheduler/registry removal; in GC cleanup, fireabort()on the handle before removing the map entry.std::env::set_varinsidespawn_blockingis UB on Rust 1.74+ —crates/librefang-api/src/secrets_env.rs:98-106,crates/librefang-api/src/routes/skills.rs:2841-2845,3234-3238— comments claim// SAFETY: spawn_blocking serialises the env mutationbutspawn_blockingdispatches across the blocking thread pool and does NOT serialize. Two concurrent route invocations (channel-bridge reload + secret PATCH) can each callunsafe { std::env::set_var(...) }on different blocking threads — Rust 1.74'sstd::env::set_vardocs declare this undefined behavior in the presence of any concurrent reader. Fix: serialize through atokio::sync::Mutex<()>held across theset_var, or move env mutations to startup-only and use a process-internal env-var registry for runtime changes.AppState.bridge_managerreload drops oldBridgeManagerwithout.stop()under load —crates/librefang-api/src/channel_bridge.rs:4035-4039—Arc::try_unwrap(old)only succeeds when no other strong references exist; any in-flightroutes/agents.rs:6347(state.bridge_manager.load_full()held acrossbm.push_message(...).await) keeps the Arc alive, sotry_unwrapreturnsErr, theif let Ok(Some(...))arm is skipped, and the oldBridgeManager's tokio tasks are never stopped — they leak until Arc strong count hits 1 (potentially never on a busy channel). Fix: always callold.stop()regardless oftry_unwrap's outcome;stop()should be safe to call on the still-shared Arc (itabort()s tasks, doesn't move).