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 29, 2023
1 parent 9c1366e commit 8a0991a
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions pkg/mcs/tso/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ func (s *Server) startGRPCServer(l net.Listener) {
defer logutil.LogPanic()
defer s.serverLoopWg.Done()

log.Info("[tso] grpc server starts serving", zap.String("address", l.Addr().String()))
err := s.grpcServer.Serve(l)
if s.IsClosed() {
log.Info("[tso] grpc server stopped")
Expand All @@ -406,6 +407,7 @@ func (s *Server) startHTTPServer(l net.Listener) {
defer logutil.LogPanic()
defer s.serverLoopWg.Done()

log.Info("http server starts serving", zap.String("address", l.Addr().String()))
err := s.httpServer.Serve(l)
if s.IsClosed() {
log.Info("http server stopped")
Expand All @@ -414,7 +416,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 +443,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 +464,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 +560,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 8a0991a

Please sign in to comment.