From 1b700e5ed8adc3442b65862b496dfbce95f20f4a Mon Sep 17 00:00:00 2001 From: Ting Yuan Date: Fri, 21 May 2021 20:54:06 +0800 Subject: [PATCH] Fix the goroutine leak in StubServer --- (If this PR fixes a github issue, please add `Fixes #`) Fixes #13023 (or if this PR is one task of a github issue, please add `Master Issue: #` to link to the master issue) Master Issue: #13023 *Motivation* Currently, StubServer can stop itself (by Stop()) before it start its grpc server (created by Start()). This race condition may lead to a goroutine leak mentioned by #13023. *Modifications* This PR add a channel to force the Stop() started after Start() *Verify this change* Please pick either of following options. - This change is already covered by existing tests, such as *(please describe tests)*. Test/TestEtcdGrpcResolver --- pkg/grpc_testing/stub_server.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/grpc_testing/stub_server.go b/pkg/grpc_testing/stub_server.go index e7e8c49d4..50b385476 100644 --- a/pkg/grpc_testing/stub_server.go +++ b/pkg/grpc_testing/stub_server.go @@ -26,10 +26,14 @@ type StubServer struct { s *grpc.Server cleanups []func() // Lambdas executed in Stop(); populated by Start(). + started chan struct{} } func New(testService testpb.TestServiceServer) *StubServer { - return &StubServer{testService: testService} + return &StubServer{ + testService: testService, + started: make(chan struct{}), + } } // Start starts the server and creates a client connected to it. @@ -50,7 +54,10 @@ func (ss *StubServer) Start(sopts []grpc.ServerOption, dopts ...grpc.DialOption) s := grpc.NewServer(sopts...) testpb.RegisterTestServiceServer(s, ss.testService) - go s.Serve(lis) + go func() { + close(ss.started) + s.Serve(lis) + }() ss.cleanups = append(ss.cleanups, s.Stop) ss.s = s @@ -59,6 +66,7 @@ func (ss *StubServer) Start(sopts []grpc.ServerOption, dopts ...grpc.DialOption) // Stop stops ss and cleans up all resources it consumed. func (ss *StubServer) Stop() { + <-ss.started for i := len(ss.cleanups) - 1; i >= 0; i-- { ss.cleanups[i]() }