XiaoFeiASK commented on code in PR #1163:
URL:
https://github.com/apache/incubator-seata-go/pull/1163#discussion_r3959515849
##########
pkg/remoting/grpc/grpc_remoting.go:
##########
@@ -108,6 +118,9 @@ func (g *GrpcRemoting) sendAsync(channel *Channel, msg
message.RpcMessage, callb
if callback != nil {
return callback(msg, resp)
}
+ // Nothing is going to wait for this one, so it would stay in the map
for
+ // the lifetime of the process.
+ g.futures.Delete(msg.ID)
Review Comment:
When `callback == nil`, this path still stores a future in the shared map. A
response reuses the server-generated request ID, which may collide with the ID
of an in-flight client request and overwrite its future. The client request
would then be unable to receive its response and eventually time out. Please
only create and store a future when `callback != nil.`
##########
pkg/remoting/getty/getty_remoting.go:
##########
@@ -105,6 +113,9 @@ func (g *GettyRemoting) sendAsync(session getty.Session,
msg message.RpcMessage,
if callback != nil {
return callback(msg, resp)
}
+ // Nothing is going to wait for this one, so it would stay in the map
for
Review Comment:
This has the same issue as the gRPC implementation: callback-less messages
do not wait for a response, but still overwrite an entry in the shared futures
map. Since response IDs come from the server and may collide with locally
generated request IDs, this can replace the future of an in-flight client
request. Please only create and store a future when `callback != nil`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
