The watchdog and the end of the call race by construction: a cancellation
landing as the reply arrives closes a conn the call had already finished
with, and c.conn still pointed at the closed socket.
It was survivable before this. A write to a closed socket is
errWriteLost, which re-dials and retries, and that retry is safe because
nothing was sent. So this buys one round trip, not a correctness fix, and
the new test says so rather than pretending to catch a break.
Review on PR #188. The deferred unlock made it unclear where the lock
was released, and it held connMu across conn.Close(), which contradicts
the invariant stated one line above it: the field accesses only. A close
on a tcp conn can block, and connMu is on the path of every call.
ipc.Client serialises every call on one mutex, and mavweb routed 28
handlers plus /api/chat through the shared client. A turn is bounded
only by phraser.timeout, 60s in deploy, so a page load behind one could
wait that long.
/models already had its own connection for this reason. Chat gets the
third one. A failed dial logs and falls back to the shared client, which
is how it behaved before.
/api/ptt needs nothing here: it proxies to the voice port and never
touches this client.
A connection pool inside ipc.Client is the general form and stays
unbuilt until a second module is measured queueing.
serveConn dispatched under context.Background(), so a dispatch in flight
during shutdown could not be told to stop and closeGrace could only
abandon it. The server now carries a context, Close cancels it, and each
conn derives its own so nothing outlives the connection.
A zero-value Server built outside Listen falls back to Background; two
wiring tests do that.
Client.Close read c.conn with no lock while roundtrip re-dialed and
dropped it, which -race caught on the new test. The conn field now has
its own mutex, held only across a read or an assignment, so Close and
the watchdog reach the connection without queueing behind the call they
are interrupting.
roundtrip set no connection deadline and call checked the context once,
before sending, so a daemon that read the frame and stopped answering
parked the caller for as long as the socket stayed open.
The deadline comes from the caller's context, falling back to 120s, the
convention internal/voice/client.go already had. A watchdog closes the
conn on ctx.Done(); it closes rather than calling drop, because drop
wants c.mu and call is holding it.
The retry split is unchanged and now also declines a retry the caller
has stopped waiting for. A cancelled mutation stays ErrAmbiguousOutcome,
because it may have committed. A cancelled read reports the cancellation.
Three tests against a server that accepts and never answers. There was
no test for this, which is why it went unnoticed.
voice.Replier.Reply had no context, so llmReplier phrased under
context.Background() and the only bound on a reply was phraser.timeout,
60s in deploy. Both call sites already held a context.
The stub ignores it: it makes no model call.
claude