fishy commented on code in PR #2883:
URL: https://github.com/apache/thrift/pull/2883#discussion_r1399509531
##########
lib/go/thrift/simple_server.go:
##########
@@ -270,6 +274,7 @@ func (p *TSimpleServer) Stop() error {
}
<-ctx.Done()
+ p.stopChan = nil
Review Comment:
this does nothing, it's immediately overwritten by the next line
##########
lib/go/thrift/simple_server.go:
##########
@@ -356,6 +361,7 @@ func (p *TSimpleServer) processRequests(client TTransport)
(err error) {
ok, err := processor.Process(ctx, inputProtocol, outputProtocol)
if errors.Is(err, ErrAbandonRequest) {
err := client.Close()
+ client = nil
Review Comment:
this does not work as you expected.
here `client` is the local arg of `processRequests` function, setting it to
`nil` only affects that copy of `client`. it does not affect the `client` you
are checking on line 214, which is at the caller of `processRequests`.
the whole nil checking regarding client is also unnecessary, if `client` is
already closed, closing it again will only give you an already closed error,
which you ignored anyways (on line 215).
--
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]
