Skip to content

Commit

Permalink
Fix the goroutine leak in StubServer
Browse files Browse the repository at this point in the history
---

(If this PR fixes a github issue, please add `Fixes #<xyz>`)

Fixes etcd-io#13023

(or if this PR is one task of a github issue, please add `Master Issue: #<xyz>`
to link to the master issue)

Master Issue: etcd-io#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 etcd-io#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
  • Loading branch information
Ting Yuan committed May 21, 2021
1 parent e149da3 commit 1b700e5
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions pkg/grpc_testing/stub_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand All @@ -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]()
}
Expand Down

0 comments on commit 1b700e5

Please sign in to comment.