[
https://issues.apache.org/jira/browse/THRIFT-4237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16078415#comment-16078415
]
Zach Wasserman commented on THRIFT-4237:
----------------------------------------
Eric, I'd encourage you to review it anyway. Always happier to get more eyes on
it. Go code pretty much always does just what it looks like. The only thing I'd
add is that {{defer}} statements are executed when the enclosing function
returns.
Should I rebase to fix up the tests?
> Go TServerSocket Race Conditions
> --------------------------------
>
> Key: THRIFT-4237
> URL: https://issues.apache.org/jira/browse/THRIFT-4237
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
> Affects Versions: 0.10.0
> Reporter: Zach Wasserman
> Assignee: Zach Wasserman
> Labels: concurrency
> Fix For: 0.11.0
>
>
> Run the following test with {{go test -race -run TestSocketConcurrency}}:
> {code}
> 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() }()
> }
> {code}
> This will result in an error from the race detector. It looks like:
> {code}
> 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
> {code}
> 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.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
