fix: per-connection locking to prevent cross-session deadlock during long prompts

[ruan330]

Problem

SessionPool::with_connection() acquires a global write lock on the connections HashMap and holds it for the entire duration of stream_prompt() — which can run for minutes during long tool calls (flutter build, test suites, git operations).

One busy session blocks all other sessions, cleanup tasks, and new session creation. The entire broker becomes unresponsive.

Reproduction

1. Open Thread A, ask the agent to run a long command (e.g., flutter build linux)
2. While Thread A is busy, open Thread B and send any message
3. Thread B hangs indefinitely — get_or_create waits for the write lock

Root cause

HashMap::get_mut requires &mut self, forcing a write lock on the entire map to access one connection. The lock is held inside with_connection for the full streaming duration:

pub async fn with_connection(&self, thread_id: &str, f: F) -> Result<R> {
    let mut conns = self.connections.write().await;  // global write lock
    let conn = conns.get_mut(thread_id)?;
    f(conn).await  // held for minutes
}

Solution

Per-connection locking

Replace HashMap<String, AcpConnection> with HashMap<String, Arc<Mutex<AcpConnection>>>.

The pool RwLock is now only held briefly for lookups (read lock, milliseconds). Each connection has its own Mutex, so sessions operate independently:

pub async fn get_connection(&self, thread_id: &str) -> Result<Arc<Mutex<AcpConnection>>> {
    let conns = self.connections.read().await;  // brief read lock
    conns.get(thread_id).cloned()
        .ok_or_else(|| anyhow!("no connection"))
}

// Caller:
let conn_arc = pool.get_connection(thread_key).await?;
let mut conn = conn_arc.lock().await;  // only locks THIS connection

Alive check instead of unbounded blocking

The notification loop (while let rx.recv().await) blocks forever if the prompt response is never received. This is dangerous because the per-connection lock is held during this time.

Replace with a tokio::select! that checks process liveness every 30 seconds:

loop {
    tokio::select! {
        msg = rx.recv() => { /* process notification */ },
        _ = tokio::time::sleep(Duration::from_secs(30)) => {
            if !conn.alive() {
                warn!("agent process died during prompt");
                break;
            }
            continue; // alive → keep waiting
        }
    }
}

This correctly handles:

• Long tool calls (process alive → wait indefinitely) ✅
• Process crash (process dead → break in ≤30s) ✅
• No false timeouts on legitimate long operations ✅

Why Arc<Mutex> over alternatives

Alternative	Problem
Increase timeout / add idle timeout	Doesn't fix cross-session blocking; false timeouts on long tool calls
Checkout pattern (remove from map, use, return)	Connection lost on panic; can't detect "checked out" state for future prompt queueing
Keep with_connection but use RwLock per connection	Unnecessary complexity — Mutex is correct for exclusive access
Arc<Mutex<AcpConnection>>	Standard connection pool pattern; correct granularity; enables future prompt queueing via try_lock()

The Arc<Mutex> approach also naturally supports future enhancements: callers can use try_lock() to detect busy connections and queue prompts via ACP's promptQueueing capability instead of blocking.

Changes

File	Lines	What
pool.rs	+54/-43	Arc<Mutex<AcpConnection>> storage; get_connection() returns Arc clone via brief read lock; remove with_connection()
discord.rs	+100/-94	stream_prompt() uses get_connection() + per-connection lock; tokio::select! alive check loop; explicit drop(conn) after prompt_done()

Testing

Tested with claude-agent-acp backend, two concurrent sessions:

• Thread A running flutter build linux (5+ minutes) — Thread B responds normally ✅
• Thread A running long Agent subagent task — Thread B creates new session and responds ✅
• Status API responsive while sessions are busy ✅
• Process crash detected within 30 seconds ✅

Closes #58

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7fd8a48342

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid blocking on connection mutex under pool write lock

cleanup_idle now acquires the global pool write lock and then awaits conn_arc.lock(). During any long-running prompt, that mutex is held for minutes, so the cleanup task can end up holding the write lock for the same duration. While this happens, all get_or_create/get_connection calls are blocked, which reintroduces the cross-session stall this change was meant to fix (the cleanup loop in main.rs runs every 60s, so this can happen regularly under load).

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Release pool read lock before awaiting session mutex

In get_or_create, the read lock on connections is held while waiting on conn_arc.lock(). If a second message arrives for a thread that is already processing a long prompt, this read lock can be held for minutes; writers are then blocked, so creating/rebuilding sessions for other threads is delayed behind that one busy thread. This creates a cross-session bottleneck whenever same-thread requests overlap.

Useful? React with 👍 / 👎.

[thepagent]

Thanks for tackling #58 — the Arc<Mutex<AcpConnection>> approach is the right pattern and the alive-check loop is a solid addition.

Two things to address before merging (Codex flagged both as well):

1. cleanup_idle holds write lock while awaiting per-connection Mutex (P1)

let mut conns = self.connections.write().await;  // write lock held
for (key, conn_arc) in conns.iter() {
    let conn = conn_arc.lock().await;  // ← blocks if session is streaming
}

If any session is mid-prompt, this blocks the write lock for minutes — re-introducing the cross-session stall. Fix: collect stale keys under a read lock first (clone the Arc, lock outside the pool lock), then take a write lock only for removal.

2. get_or_create read lock held during alive check (P2)

Same pattern — the read lock is held while awaiting conn_arc.lock(). If a thread is busy, this blocks all writers. Fix: clone the Arc, drop the read lock, then lock the connection.

let conn_arc = {
    let conns = self.connections.read().await;
    conns.get(thread_id).cloned()
};
if let Some(arc) = conn_arc {
    if arc.lock().await.alive() {
        return Ok(());
    }
}

The core change (per-connection locking + get_connection) is good. Just need to fix the lock ordering in these two spots.

[ruan330]

Closing — superseded by #77 which includes this fix along with alive check, drain window, and startup cleanup.