[THRIFT-4237] Go TServerSocket Race Conditions

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.10.0
• Fix Version/s: 0.11.0
• Component/s: Go - Library
• Labels: concurrency
• Assignee: Zach Wasserman
• Reporter: Zach Wasserman
• Created: 27/Jun/17 00:03
• Updated: 14/Dec/17 13:56
• Resolved: 08/Jul/17 13:37

Description

Run the following test with go test -race -run TestSocketConcurrency:

func TestSocketConcurrency(t *testing.T) {
	host := "127.0.0.1"
	port := 9090
	addr := fmt.Sprintf("%s:%d", host, port)

	socket := CreateServerSocket(t, addr)
	go func() { socket.Listen() }()
	go func() { socket.Interrupt() }()
}

This will result in an error from the race detector. It looks like:

go test -v -race -run TestSocketConcurrency
=== RUN   TestSocketConcurrency
--- PASS: TestSocketConcurrency (0.00s)
==================
WARNING: DATA RACE
Write at 0x00c420016870 by goroutine 7:
  git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Listen()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket.go:63 +0x134
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency.func1()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:50 +0x38

Previous read at 0x00c420016870 by goroutine 8:
  git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Close()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket.go:114 +0x7b
  git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Interrupt()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket.go:123 +0x6b
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency.func2()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:51 +0x38

Goroutine 7 (running) created at:
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:50 +0x1a8
  testing.tRunner()
      /usr/local/Cellar/go/1.8/libexec/src/testing/testing.go:657 +0x107

Goroutine 8 (running) created at:
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:51 +0x1ca
  testing.tRunner()
      /usr/local/Cellar/go/1.8/libexec/src/testing/testing.go:657 +0x107
==================
==================
WARNING: DATA RACE
Write at 0x00c420016870 by goroutine 8:
  git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Close.func1()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket.go:112 +0x3b
  git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Close()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket.go:117 +0xe6
  git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Interrupt()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket.go:123 +0x6b
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency.func2()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:51 +0x38

Previous read at 0x00c420016870 by goroutine 7:
  git.apache.org/thrift.git/lib/go/thrift.(*TServerSocket).Listen()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket.go:56 +0x48
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency.func1()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:50 +0x38

Goroutine 8 (running) created at:
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:51 +0x1ca
  testing.tRunner()
      /usr/local/Cellar/go/1.8/libexec/src/testing/testing.go:657 +0x107

Goroutine 7 (finished) created at:
  git.apache.org/thrift.git/lib/go/thrift.TestSocketConcurrency()
      /Users/zwass/dev/go/src/git.apache.org/thrift.git/lib/go/thrift/server_socket_test.go:50 +0x1a8
  testing.tRunner()
      /usr/local/Cellar/go/1.8/libexec/src/testing/testing.go:657 +0x107
==================
FAIL
exit status 1
FAIL	git.apache.org/thrift.git/lib/go/thrift	0.186s

I believe this is because Interrupt()
(https://github.com/apache/thrift/blob/master/lib/go/thrift/server_socket.go#L120) and Listen() (https://github.com/apache/thrift/blob/master/lib/go/thrift/server_socket.go#L55) both read and write p.listener, but only Interrupt() locks the mutex.

Issue Links

• links to: GitHub Pull Request #1300
• links to: GitHub Pull Request #1304