diff --git a/internal/ipc/cancel_test.go b/internal/ipc/cancel_test.go index 88db143..084d7b7 100644 --- a/internal/ipc/cancel_test.go +++ b/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) + } + } +} diff --git a/internal/ipc/client.go b/internal/ipc/client.go index ba983b0..08c885c 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -255,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():