Skip to content

Commit

Permalink
go: Make socketConn.Close thread-safe
Browse files Browse the repository at this point in the history
Client: go

We used to rely on setting the connection inside TSocket/TSSLSocket as
nil after Close is called to mark the connection as closed, but that is
not thread safe and causing TSocket.Close/TSSLSocket.Close cannot be
called concurrently. Use an atomic int to mark closure instead.
  • Loading branch information
fishy committed Jan 9, 2022
1 parent 9d7d627 commit 39d7278
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 19 deletions.
10 changes: 1 addition & 9 deletions lib/go/thrift/socket.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,15 +194,7 @@ func (p *TSocket) IsOpen() bool {

// Closes the socket.
func (p *TSocket) Close() error {
// Close the socket
if p.conn != nil {
err := p.conn.Close()
if err != nil {
return err
}
p.conn = nil
}
return nil
return p.conn.Close()
}

//Returns the remote address of the socket.
Expand Down
13 changes: 12 additions & 1 deletion lib/go/thrift/socket_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ package thrift

import (
"net"
"sync/atomic"
)

// socketConn is a wrapped net.Conn that tries to do connectivity check.
type socketConn struct {
net.Conn

buffer [1]byte
closed int32
}

var _ net.Conn = (*socketConn)(nil)
Expand Down Expand Up @@ -64,7 +66,7 @@ func wrapSocketConn(conn net.Conn) *socketConn {
// It's the same as the previous implementation of TSocket.IsOpen and
// TSSLSocket.IsOpen before we added connectivity check.
func (sc *socketConn) isValid() bool {
return sc != nil && sc.Conn != nil
return sc != nil && sc.Conn != nil && atomic.LoadInt32(&sc.closed) == 0
}

// IsOpen checks whether the connection is open.
Expand Down Expand Up @@ -100,3 +102,12 @@ func (sc *socketConn) Read(p []byte) (n int, err error) {

return sc.Conn.Read(p)
}

func (sc *socketConn) Close() error {
if !sc.isValid() {
// Already closed
return net.ErrClosed
}
atomic.StoreInt32(&sc.closed, 1)
return sc.Conn.Close()
}
10 changes: 1 addition & 9 deletions lib/go/thrift/ssl_socket.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,7 @@ func (p *TSSLSocket) IsOpen() bool {

// Closes the socket.
func (p *TSSLSocket) Close() error {
// Close the socket
if p.conn != nil {
err := p.conn.Close()
if err != nil {
return err
}
p.conn = nil
}
return nil
return p.conn.Close()
}

func (p *TSSLSocket) Read(buf []byte) (int, error) {
Expand Down

0 comments on commit 39d7278

Please sign in to comment.