From bd5337261617d26b34798505753385a918bc8c9a Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 23:05:53 +0400 Subject: [PATCH] Close takes connMu by hand, not by defer (V-638) 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. --- internal/ipc/client.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 7deb6ed..ba983b0 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -117,15 +117,19 @@ func Dial(path string) (*Client, error) { // Close closes the connection out from under a call in flight, on purpose: a // shutdown must not wait out a parked read. It takes connMu and never c.mu, so // it cannot block behind the call it is interrupting. +// +// The lock is taken and released by hand, around the two field accesses and +// nothing else. The socket close happens outside it, because a close on a tcp +// conn can block and connMu is on the path of every call. func (c *Client) Close() error { c.connMu.Lock() - defer c.connMu.Unlock() - if c.conn == nil { + conn := c.conn + c.conn = nil + c.connMu.Unlock() + if conn == nil { return nil } - err := c.conn.Close() - c.conn = nil - return err + return conn.Close() } // DialWait is Dial with patience: it retries with capped backoff until the