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.
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user