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.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+11
-1
@@ -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():
|
||||
|
||||
Reference in New Issue
Block a user