DATA RACE in the server

[hawkingrei]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

 WARNING: DATA RACE
Read at 0x00c020bc49f0 by goroutine 143:
  github.com/pingcap/tidb/server.(*Server).Run()
      /go/tidb/server/server.go:367 +0x295
  github.com/pingcap/tidb/server.(*basicHTTPHandlerTestSuite).startServer.func1()
      /go/tidb/server/http_handler_test.go:470 +0x39
Previous write at 0x00c020bc49f0 by goroutine 141:
  github.com/pingcap/tidb/server.(*Server).Close()
      /go/tidb/server/server.go:483 +0x1c4
  github.com/pingcap/tidb/server.(*basicHTTPHandlerTestSuite).stopServer()
      /go/tidb/server/http_handler_test.go:486 +0x68
  github.com/pingcap/tidb/server.TestFailpointHandler·dwrap·49()
      /go/tidb/server/http_handler_serial_test.go:438 +0x47
  github.com/pingcap/tidb/server.TestFailpointHandler()
      /go/tidb/server/http_handler_serial_test.go:456 +0x50b
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
Goroutine 143 (running) created at:
  github.com/pingcap/tidb/server.(*basicHTTPHandlerTestSuite).startServer()
      /go/tidb/server/http_handler_test.go:469 +0x807
  github.com/pingcap/tidb/server.TestFailpointHandler()
      /go/tidb/server/http_handler_serial_test.go:448 +0x364
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
Goroutine 141 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1306 +0x726
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1598 +0x99
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1596 +0x7ca
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:1504 +0x9d1
  go.uber.org/goleak.VerifyTestMain()
      /home/prow/go/pkg/mod/go.uber.org/[email protected]/testmain.go:53 +0x59
  github.com/pingcap/tidb/server.TestMain()
      /go/tidb/server/main_test.go:67 +0xd57
  main.main()
      _testmain.go:177 +0x264 

2. What did you expect to see? (Required)

3. What did you see instead (Required)

4. What is your TiDB version? (Required)

[bb7133]

I think it is caused for the following reason:

• TiDB server is started with a socket listener(read of s.socket):

  tidb/server/server.go

  Line 367 in 6756bd2

  go s.startNetworkListener(s.socket, true, errChan)

• The test case tries to close the server, during this process s.socket is set to nil(write of it):

  tidb/server/server.go

  Line 483 in 6756bd2

  s.socket = nil

We have a "waitUntilCustomeServerCanConnect" method to make sure the listener is ready before the running of the rest test codes, but it did check for the connection given by dsn.

So it is possible that in extreme cases, the non-socket listener is ready and makes waitUntilCustomeServerCanConnect passed by the socket listener is not, then we may see a RACE for s.socket.

The s.socket is introduced in #29374 , but I don't think it is necessary:

tidb/server/http_handler_test.go

Line 462 in a696b43

cfg.Socket = fmt.Sprintf("/tmp/%s.sock", t.Name())

I plan to remove it to avoid the race issue. /cc @hawkingrei @dveeden @mjonss @djshow832

[dveeden]

@bb7133 That should be fine. In current master newTestConfig() sets cfg.Socket = "", so anything that doesn't explicitly need a socket doesn't need to set socket names etc.

[github-actions]

Please check whether the issue should be labeled with 'affects-x.y' or 'fixes-x.y.z', and then remove 'needs-more-info' label.