Skip to content

panic: send on closed channel — elicitation events channel closed outside the bridge lock during stream teardown #3069

Description

@aheritier

Summary

The runtime can crash with panic: send on closed channel when an in-flight elicitation send races the teardown of a stream's events channel. The root cause is a lifecycle defect in elicitationBridge: sends are synchronized against channel swaps, but the channel close happens outside the bridge's lock, so a close can land on a channel that a concurrent sender is still parked on.

This is a general concurrency bug, not specific to any one feature. It is reachable any time two stream lifecycles overlap — nested sub-sessions (background agents, transferred tasks, fork-mode skills) or a second top-level session created with /fork while the first is still running.

Stack trace (abridged)

panic: send on closed channel

goroutine 253016 [running]:
runtime.(*elicitationBridge).send                pkg/runtime/elicitation.go:83
runtime.(*LocalRuntime).elicitationHandler       pkg/runtime/elicitation.go:136
userprompt.(*ToolSet).userPrompt                 pkg/tools/builtin/userprompt/userprompt.go:88
... toolexec dispatcher (runToolset → invoke → run → approveAndRun) ...
runtime.(*LocalRuntime).processToolCalls         pkg/runtime/tool_dispatch.go:51
runtime.(*LocalRuntime).runTurn                  pkg/runtime/loop.go:640
runtime.(*LocalRuntime).runStreamLoop            pkg/runtime/loop.go:434
created by runtime.(*LocalRuntime).RunStream in goroutine 253015
                                                 pkg/runtime/loop.go:205

The panic fires at pkg/runtime/elicitation.go:83b.ch <- ev inside elicitationBridge.send, reached from a userprompt tool call running inside processToolCallsrunTurnrunStreamLoop.

How elicitation channel ownership works

elicitationBridge owns the events channel that the MCP elicitation handler sends to. Each RunStream swaps in its own channel on entry and swaps the previous one back on exit, so nested streams don't lose the parent's pipe:

// loop.go — runStreamLoop entry
prevElicitationCh := r.elicitation.swap(events)
// elicitation.go — send and swap are serialized by an RWMutex
func (b *elicitationBridge) send(ev Event) error {
    b.mu.RLock()
    defer b.mu.RUnlock()
    if b.ch == nil {
        return errNoElicitationChannel
    }
    b.ch <- ev          // <-- panics if this channel was closed elsewhere
    return nil
}

func (b *elicitationBridge) swap(ch chan Event) chan Event {
    b.mu.Lock()
    defer b.mu.Unlock()
    prev := b.ch
    b.ch = ch
    return prev
}

The bridge's doc comment claims the RWMutex preserves the invariant that "the channel reference held by an in-flight sender stays open until that sender finishes." That guarantee only holds against swap.

Root cause

The channel is closed in finalizeEventChannel (pkg/runtime/loop.go), which runs as the deferred teardown of every runStreamLoop:

func (r *LocalRuntime) finalizeEventChannel(ctx, sess, reason, prevElicitationCh, events chan Event) {
    r.elicitation.swap(prevElicitationCh) // swap the ref out of the bridge
    defer close(events)                   // <-- closed OUTSIDE the bridge lock
    ...
    events <- StreamStopped(...)
}

swap(prevElicitationCh) removes events from the bridge, then close(events) runs with no coordination with send's read-lock. So the sequence that panics is:

  1. Stream A's elicitationHandler calls send, takes RLock, sees a valid b.ch, and parks on b.ch <- ev (buffer full or unbuffered).
  2. Stream A (or the parent) reaches finalizeEventChannel, calls swap(prev) — which does not block on the RLock because the sender already passed the lock and is now blocked on the channel op, not the mutex — and then close(events).
  3. The parked send in step 1 resumes on a now-closed channel → panic.

The RWMutex serializes sendswap but never gates close, so the "stays open until the sender finishes" invariant is not actually enforced.

Triggers (in order of likelihood)

  • Nested sub-sessions: background agents, transfer_task, fork-mode skills — each pushes/pops an elicitation channel, so teardown of an inner stream races a send on the bridge.
  • /fork while a session is running: spawns a second top-level RunStream loop concurrently with the first, so two independent teardowns can interleave with an in-flight elicitation. This is the path that surfaced the crash above, but it is one trigger among several, not the cause.

Existing test gap

TestElicitationBridge_SwapWaitsForInflightSenders proves swap waits for in-flight senders, but there is no coverage of close vs an in-flight send — exactly the unguarded path.

Proposed fix

  1. Move the channel close behind the bridge lock so it is mutually exclusive with send (a bridge-owned restoreAndClose/closeChannel that restores the previous channel, emits StreamStopped, and closes under the write lock).
  2. Add a defensive recover in send that converts a send-on-closed-channel panic into errNoElicitationChannel, so any future regression degrades to a declined elicitation instead of crashing the process.
  3. Add a -race regression test covering close-vs-in-flight-send.

Environment

  • Observed on macOS (arm64).
  • Reproduced against the current main runtime.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/agentFor work that has to do with the general agent loop/agentic features of the apparea/mcpMCP protocol, MCP tool servers, integrationarea/skillsSkills system and custom slash commands

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions