fishy commented on a change in pull request #2497:
URL: https://github.com/apache/thrift/pull/2497#discussion_r780701275
##########
File path: lib/go/thrift/socket.go
##########
@@ -195,8 +195,9 @@ func (p *TSocket) IsOpen() bool {
// Closes the socket.
func (p *TSocket) Close() error {
// Close the socket
- if p.conn != nil {
- err := p.conn.Close()
+ conn := p.conn
Review comment:
Race detector detected issue in this change and the issue went away
after I cherry-picked #2500 in:
```
~/work/thrift/lib/go/thrift$ go test -race
==================
WARNING: DATA RACE
Write at 0x00c0002c2260 by goroutine 74:
github.com/apache/thrift/lib/go/thrift.(*TSocket).Close()
/home/fishy/work/thrift/lib/go/thrift/socket.go:203 +0x79
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).innerAccept.func2()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:215 +0x1a1
Previous read at 0x00c0002c2260 by goroutine 107:
github.com/apache/thrift/lib/go/thrift.(*TSocket).Close()
/home/fishy/work/thrift/lib/go/thrift/socket.go:198 +0x32
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).processRequests·dwrap·14()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:307 +0x48
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).processRequests()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:349 +0x8ce
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).innerAccept.func1()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:203 +0x108
Goroutine 74 (running) created at:
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).innerAccept()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:208 +0x396
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).AcceptLoop()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:224 +0xee
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).Serve()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:241 +0xbd
github.com/apache/thrift/lib/go/thrift.TestNoHangDuringStopFromClientNoDataSendDuringAcceptLoop·dwrap·30()
/home/fishy/work/thrift/lib/go/thrift/simple_server_test.go:191 +0x39
Goroutine 107 (finished) created at:
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).innerAccept()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:200 +0x296
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).AcceptLoop()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:224 +0xee
github.com/apache/thrift/lib/go/thrift.(*TSimpleServer).Serve()
/home/fishy/work/thrift/lib/go/thrift/simple_server.go:241 +0xbd
github.com/apache/thrift/lib/go/thrift.TestNoHangDuringStopFromClientNoDataSendDuringAcceptLoop·dwrap·30()
/home/fishy/work/thrift/lib/go/thrift/simple_server_test.go:191 +0x39
==================
--- FAIL: TestNoHangDuringStopFromClientNoDataSendDuringAcceptLoop (0.02s)
testing.go:1152: race detected during execution of test
FAIL
exit status 1
FAIL github.com/apache/thrift/lib/go/thrift 2.597s
~/work/thrift/lib/go/thrift$ git cherry-pick 1387ff557
[pr/2497 c356299ba] go: Make socketConn.Close thread-safe
Date: Sat Jan 8 01:03:57 2022 -0800
3 files changed, 14 insertions(+), 19 deletions(-)
~/work/thrift/lib/go/thrift$ go test -race
PASS
ok github.com/apache/thrift/lib/go/thrift 2.468s
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
