fishy commented on code in PR #3220:
URL: https://github.com/apache/thrift/pull/3220#discussion_r2440736623
##########
lib/go/thrift/server_socket.go:
##########
@@ -110,7 +110,9 @@ func (p *TServerSocket) Open() error {
}
func (p *TServerSocket) Addr() net.Addr {
- if p.listener != nil {
+ p.mu.RLock()
+ defer p.mu.RUnlock()
+ if p.IsListening() {
Review Comment:
If you look at the comment about mu at
https://github.com/apache/thrift/blob/d9a97c1610372eeb6db9f1a54e590a0a89067aea/lib/go/thrift/server_socket.go#L33-L34,
it's only supposed to protect `interrupted` field.
I guess that has evolved over the years, and we already have existing code
to use `mu` to also protect `listener`.
so this is the time we should update the comment to reflect that. please
move the `listener` field to the same group with `mu` and `interrupted`, and
also update the comment.
--
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]
