Skip to content

Commit 939edc0

Browse files
dfawleymenghanl
dfawley
authored and
menghanl
committed
Revert "Use bufconn in end2end tests." (#1381)
This reverts commit c7e2c00. There is a race between Dial and Close that causes goroutine leaks in the end2end tests.
1 parent 71260d2 commit 939edc0

1 file changed

Lines changed: 25 additions & 37 deletions

File tree

‎test/end2end_test.go‎

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ import (
5353
"google.golang.org/grpc/peer"
5454
"google.golang.org/grpc/status"
5555
"google.golang.org/grpc/tap"
56-
"google.golang.org/grpc/test/bufconn"
5756
testpb "google.golang.org/grpc/test/grpc_testing"
5857
)
5958

@@ -355,7 +354,7 @@ const tlsDir = "testdata/"
355354

356355
type env struct {
357356
name string
358-
network string // The type of network such as tcp, unix, bufconn, etc.
357+
network string // The type of network such as tcp, unix, etc.
359358
security string // The security protocol such as TLS, SSH, etc.
360359
httpHandler bool // whether to use the http.Handler ServerTransport; requires TLS
361360
balancer bool // whether to use balancer
@@ -368,19 +367,21 @@ func (e env) runnable() bool {
368367
return true
369368
}
370369

371-
const bufconnNetwork = "bufconn"
370+
func (e env) dialer(addr string, timeout time.Duration) (net.Conn, error) {
371+
return net.DialTimeout(e.network, addr, timeout)
372+
}
372373

373374
var (
374375
tcpClearEnv = env{name: "tcp-clear", network: "tcp", balancer: true}
375376
tcpTLSEnv = env{name: "tcp-tls", network: "tcp", security: "tls", balancer: true}
376377
unixClearEnv = env{name: "unix-clear", network: "unix", balancer: true}
377378
unixTLSEnv = env{name: "unix-tls", network: "unix", security: "tls", balancer: true}
378-
handlerEnv = env{name: "handler-tls", network: bufconnNetwork, security: "tls", httpHandler: true, balancer: true}
379-
noBalancerEnv = env{name: "no-balancer", network: bufconnNetwork, security: "tls", balancer: false}
379+
handlerEnv = env{name: "handler-tls", network: "tcp", security: "tls", httpHandler: true, balancer: true}
380+
noBalancerEnv = env{name: "no-balancer", network: "tcp", security: "tls", balancer: false}
380381
allEnv = []env{tcpClearEnv, tcpTLSEnv, unixClearEnv, unixTLSEnv, handlerEnv, noBalancerEnv}
381382
)
382383

383-
var onlyEnv = flag.String("only_env", "", "Restrict tests to the named environment. Empty means all.")
384+
var onlyEnv = flag.String("only_env", "", "If non-empty, one of 'tcp-clear', 'tcp-tls', 'unix-clear', 'unix-tls', or 'handler-tls' to only run the tests for that environment. Empty means all.")
384385

385386
func listTestEnv() (envs []env) {
386387
if *onlyEnv != "" {
@@ -441,7 +442,6 @@ type test struct {
441442
// srv and srvAddr are set once startServer is called.
442443
srv *grpc.Server
443444
srvAddr string
444-
lis net.Listener
445445

446446
cc *grpc.ClientConn // nil until requested via clientConn
447447
restoreLogs func() // nil unless declareLogNoise is used
@@ -523,12 +523,7 @@ func (te *test) startServer(ts testpb.TestServiceServer) {
523523
la = "/tmp/testsock" + fmt.Sprintf("%d", time.Now().UnixNano())
524524
syscall.Unlink(la)
525525
}
526-
var err error
527-
if te.e.network == bufconnNetwork {
528-
te.lis = bufconn.Listen(1024 * 1024)
529-
} else {
530-
te.lis, err = net.Listen(te.e.network, la)
531-
}
526+
lis, err := net.Listen(te.e.network, la)
532527
if err != nil {
533528
te.t.Fatalf("Failed to listen: %v", err)
534529
}
@@ -561,36 +556,24 @@ func (te *test) startServer(ts testpb.TestServiceServer) {
561556
addr := la
562557
switch te.e.network {
563558
case "unix":
564-
case "bufconn":
565-
addr = te.lis.Addr().String()
566559
default:
567-
_, port, err := net.SplitHostPort(te.lis.Addr().String())
560+
_, port, err := net.SplitHostPort(lis.Addr().String())
568561
if err != nil {
569562
te.t.Fatalf("Failed to parse listener address: %v", err)
570563
}
571564
addr = "localhost:" + port
572565
}
573566

574-
go s.Serve(te.lis)
567+
go s.Serve(lis)
575568
te.srvAddr = addr
576569
}
577570

578-
func (te *test) dialer(addr string, timeout time.Duration) (net.Conn, error) {
579-
if te.lis == nil {
580-
return nil, fmt.Errorf("no listener")
581-
}
582-
if te.e.network == bufconnNetwork {
583-
return te.lis.(*bufconn.Listener).Dial()
584-
}
585-
return net.DialTimeout(te.e.network, addr, timeout)
586-
}
587-
588571
func (te *test) clientConn() *grpc.ClientConn {
589572
if te.cc != nil {
590573
return te.cc
591574
}
592575
opts := []grpc.DialOption{
593-
grpc.WithDialer(te.dialer),
576+
grpc.WithDialer(te.e.dialer),
594577
grpc.WithUserAgent(te.userAgent),
595578
}
596579

@@ -661,7 +644,7 @@ func (te *test) declareLogNoise(phrases ...string) {
661644
}
662645

663646
func (te *test) withServerTester(fn func(st *serverTester)) {
664-
c, err := te.dialer(te.srvAddr, 10*time.Second)
647+
c, err := te.e.dialer(te.srvAddr, 10*time.Second)
665648
if err != nil {
666649
te.t.Fatal(err)
667650
}
@@ -1106,7 +1089,7 @@ func testFailFast(t *testing.T, e env) {
11061089
if grpc.Code(err) == codes.Unavailable {
11071090
break
11081091
}
1109-
t.Logf("%v.EmptyCall(_, _) = _, %v", tc, err)
1092+
fmt.Printf("%v.EmptyCall(_, _) = _, %v", tc, err)
11101093
time.Sleep(10 * time.Millisecond)
11111094
}
11121095
// The client keeps reconnecting and ongoing fail-fast RPCs should fail with code.Unavailable.
@@ -2239,8 +2222,7 @@ func testPeerClientSide(t *testing.T, e env) {
22392222
t.Fatalf("TestService/EmptyCall(_, _) = _, %v, want _, <nil>", err)
22402223
}
22412224
pa := peer.Addr.String()
2242-
switch e.network {
2243-
case "unix", "bufconn":
2225+
if e.network == "unix" {
22442226
if pa != te.srvAddr {
22452227
t.Fatalf("peer.Addr = %v, want %v", pa, te.srvAddr)
22462228
}
@@ -3986,8 +3968,12 @@ func TestDialWithBlockErrorOnBadCertificates(t *testing.T) {
39863968
te.startServer(&testServer{security: te.e.security})
39873969
defer te.tearDown()
39883970

3989-
var err error
3990-
te.cc, err = grpc.Dial(te.srvAddr, grpc.WithTransportCredentials(clientAlwaysFailCred{}), grpc.WithBlock(), grpc.WithDialer(te.dialer))
3971+
var (
3972+
err error
3973+
opts []grpc.DialOption
3974+
)
3975+
opts = append(opts, grpc.WithTransportCredentials(clientAlwaysFailCred{}), grpc.WithBlock())
3976+
te.cc, err = grpc.Dial(te.srvAddr, opts...)
39913977
if err != errClientAlwaysFailCred {
39923978
te.t.Fatalf("Dial(%q) = %v, want %v", te.srvAddr, err, errClientAlwaysFailCred)
39933979
}
@@ -4449,16 +4435,18 @@ func (ss *stubServer) FullDuplexCall(stream testpb.TestService_FullDuplexCallSer
44494435

44504436
// Start starts the server and creates a client connected to it.
44514437
func (ss *stubServer) Start() error {
4452-
lis := bufconn.Listen(1024 * 1024)
4453-
dialer := func(string, time.Duration) (net.Conn, error) { return lis.Dial() }
4438+
lis, err := net.Listen("tcp", "localhost:0")
4439+
if err != nil {
4440+
return fmt.Errorf(`net.Listen("tcp", "localhost:0") = %v`, err)
4441+
}
44544442
ss.cleanups = append(ss.cleanups, func() { lis.Close() })
44554443

44564444
s := grpc.NewServer()
44574445
testpb.RegisterTestServiceServer(s, ss)
44584446
go s.Serve(lis)
44594447
ss.cleanups = append(ss.cleanups, s.Stop)
44604448

4461-
cc, err := grpc.Dial(lis.Addr().String(), grpc.WithInsecure(), grpc.WithBlock(), grpc.WithDialer(dialer))
4449+
cc, err := grpc.Dial(lis.Addr().String(), grpc.WithInsecure(), grpc.WithBlock())
44624450
if err != nil {
44634451
return fmt.Errorf("grpc.Dial(%q) = %v", lis.Addr().String(), err)
44644452
}

0 commit comments

Comments
 (0)