a cancelled call does not leave its conn for the next one (V-638)

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.

internal/ipc/cancel_test.go

@@ -168,3 +168,25 @@ func TestServerCloseCancelsADispatchInFlight(t *testing.T) {
 		t.Fatal("Close did not cancel the dispatch")
 	}
 }
+
+// The watchdog closes the conn, and it races the end of the call: a
+// cancellation landing as the reply arrives can close a conn the call was
+// already done with. That is survivable either way, because a write to a closed
+// socket is errWriteLost and errWriteLost re-dials and retries, so this test
+// passes with or without the drop in roundtrip's defer. What it pins is that
+// the recovery is real and costs one round trip at most, never an error the
+// caller sees.
+func TestClientSurvivesACancelledCall(t *testing.T) {
+	_, _, cli, _ := newServerWithStore(t)
+
+	for i := 0; i < 20; i++ {
+		ctx, cancel := context.WithCancel(context.Background())
+		go cancel() // races the reply on purpose
+		_, _ = cli.Ping(ctx)
+		cancel()
+
+		if _, err := cli.Ping(context.Background()); err != nil {
+			t.Fatalf("call %d after a cancelled one: %v", i, err)
+		}
+	}
+}

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
@@ -251,8 +255,18 @@ func (c *Client) roundtrip(ctx context.Context, m Method, raw json.RawMessage, r
 	}
 	defer conn.SetDeadline(time.Time{})
 
+	// The watchdog and the end of the call race by construction: a cancellation
+	// landing just as the reply arrives can close a conn this call is already
+	// done with, and c.conn would still point at the closed socket. So a call
+	// whose context ended does not leave the conn behind for the next one,
+	// whichever of the two got there first.
 	done := make(chan struct{})
-	defer close(done)
+	defer func() {
+		close(done)
+		if ctx.Err() != nil {
+			c.drop()
+		}
+	}()
 	go func() {
 		select {
 		case <-ctx.Done():