Skip to content

Commit bd04e32

Browse files
author
Bryan C. Mills
committed
internal/jsonrpc2_v2: eliminate a potential Accept/Dial race in TestIdleTimeout
The only explanation I can think of for the failure in https://go.dev/issue/49387#issuecomment-1303979877 is that maybe the idle timeout started as soon as conn1 was closed, without waiting for conn2 to be closed. That might be possible if the connection returned by Dial was still in the server's accept queue, but never actually accepted. To eliminate that possibility, we can send an RPC on that connection and wait for a response, as we already do with conn1. Since the conn1 RPC succeeded, we know that the connection is non-idle, conn2 should be accepted, and the request on conn2 should succeed unconditionally. Fixes golang/go#49387 (hopefully for real this time). Change-Id: Ie3e74f91d322223d82c000fdf1f3a0ed08afd20d Reviewed-on: https://go-review.googlesource.com/c/tools/+/448096 gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
1 parent d41a43b commit bd04e32

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

internal/jsonrpc2_v2/serve_test.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,25 @@ func TestIdleTimeout(t *testing.T) {
6666
return false
6767
}
6868

69+
// Since conn1 was successfully accepted and remains open, the server is
70+
// definitely non-idle. Dialing another simultaneous connection should
71+
// succeed.
6972
conn2, err := jsonrpc2.Dial(ctx, listener.Dialer(), jsonrpc2.ConnectionOptions{})
7073
if err != nil {
7174
conn1.Close()
72-
if since := time.Since(idleStart); since < d {
73-
t.Fatalf("conn2 failed to connect while non-idle: %v", err)
74-
}
75-
t.Log("jsonrpc2.Dial:", err)
75+
t.Fatalf("conn2 failed to connect while non-idle after %v: %v", time.Since(idleStart), err)
7676
return false
7777
}
78+
// Ensure that conn2 is also accepted on the server side before we close
79+
// conn1. Otherwise, the connection can appear idle if the server processes
80+
// the closure of conn1 and the idle timeout before it finally notices conn2
81+
// in the accept queue.
82+
// (That failure mode may explain the failure noted in
83+
// https://go.dev/issue/49387#issuecomment-1303979877.)
84+
ac = conn2.Call(ctx, "ping", nil)
85+
if err := ac.Await(ctx, nil); !errors.Is(err, jsonrpc2.ErrMethodNotFound) {
86+
t.Fatalf("conn2 broken while non-idle after %v: %v", time.Since(idleStart), err)
87+
}
7888

7989
if err := conn1.Close(); err != nil {
8090
t.Fatalf("conn1.Close failed with error: %v", err)

0 commit comments

Comments
 (0)