Skip to content

Commit

Permalink
Fix Data Race in TestServerRegisterTestSuite
Browse files Browse the repository at this point in the history
==================
WARNING: DATA RACE
Read at 0x00c000406fd0 by goroutine 718:
  github.com/tikv/pd/pkg/mcs/tso/server.(*Server).stopHTTPServer.func1()
      /home/runner/work/pd/pd/pkg/mcs/tso/server/server.go:465 +0xde

Previous write at 0x00c000406fd0 by goroutine 508:
  github.com/tikv/pd/pkg/mcs/tso/server.(*Server).startGRPCAndHTTPServers()
      /home/runner/work/pd/pd/pkg/mcs/tso/server/server.go:436 +0x907
  github.com/tikv/pd/pkg/mcs/tso/server.(*Server).startServer.func1()
      /home/runner/work/pd/pd/pkg/mcs/tso/server/server.go:561 +0x58

Goroutine 718 (running) created at:
  github.com/tikv/pd/pkg/mcs/tso/server.(*Server).stopHTTPServer()
      /home/runner/work/pd/pd/pkg/mcs/tso/server/server.go:462 +0x24d
  github.com/tikv/pd/pkg/mcs/tso/server.(*Server).Close()
      /home/runner/work/pd/pd/pkg/mcs/tso/server/server.go:193 +0x13c
  github.com/tikv/pd/tests/integrations/mcs/discovery_test.(*serverRegisterTestSuite).checkServerPrimaryChange()
      /home/runner/work/pd/pd/tests/integrations/mcs/discovery/register_test.go:139 +0x37b
  github.com/tikv/pd/tests/integrations/mcs/discovery_test.(*serverRegisterTestSuite).TestServerPrimaryChange()
      /home/runner/work/pd/pd/tests/integrations/mcs/discovery/register_test.go:116 +0x3d

Signed-off-by: Bin Shi <binshi.bing@gmail.com>
  • Loading branch information
binshi-bing committed May 28, 2023
1 parent 9c1366e commit cbc6737
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions pkg/mcs/tso/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func (s *Server) startHTTPServer(l net.Listener) {
}
}

func (s *Server) startGRPCAndHTTPServers(l net.Listener) {
func (s *Server) startGRPCAndHTTPServers(serverReadyChan chan<- struct{}, l net.Listener) {
defer logutil.LogPanic()
defer s.serverLoopWg.Done()

Expand All @@ -441,6 +441,7 @@ func (s *Server) startGRPCAndHTTPServers(l net.Listener) {
s.serverLoopWg.Add(1)
go s.startHTTPServer(s.httpListener)

serverReadyChan <- struct{}{}
if err := mux.Serve(); err != nil {
if s.IsClosed() {
log.Info("[tso] mux stopped serving", errs.ZapError(err))
Expand All @@ -461,14 +462,14 @@ func (s *Server) stopHTTPServer() {
ch := make(chan struct{})
go func() {
defer close(ch)
log.Warn("[tso] http server graceful shutdown timeout, forcing close")
s.httpServer.Shutdown(ctx)
}()

select {
case <-ch:
case <-ctx.Done():
// Took too long, manually close open transports
log.Warn("[tso] http server graceful shutdown timeout, forcing close")
s.httpServer.Close()
// concurrent Graceful Shutdown should be interrupted
<-ch
Expand Down Expand Up @@ -557,8 +558,11 @@ func (s *Server) startServer() (err error) {
return err
}

serverReadyChan := make(chan struct{})
defer close(serverReadyChan)
s.serverLoopWg.Add(1)
go s.startGRPCAndHTTPServers(s.muxListener)
go s.startGRPCAndHTTPServers(serverReadyChan, s.muxListener)
<-serverReadyChan

// Run callbacks
log.Info("triggering the start callback functions")
Expand Down

0 comments on commit cbc6737

Please sign in to comment.