apache / rocketmq-client-go

Apache RocketMQ go client
https://rocketmq.apache.org/
Apache License 2.0
1.3k stars 419 forks source link

[ISSUE #1114] fix: response future should close channel before callback #1113

Closed twz915 closed 10 months ago

twz915 commented 10 months ago

What is the purpose of the change

When ctx time out, r.ctx.Done() returns (line 63->65), causing a normal response to return an error as well

https://github.com/apache/rocketmq-client-go/blob/7ae83c49351a96d78ebccec5dd545f0c05e3d514/internal/remote/future.go#L60-L66

So we should close(responseFuture.Done) before responseFuture.executeInvokeCallback()

https://github.com/apache/rocketmq-client-go/blob/7ae83c49351a96d78ebccec5dd545f0c05e3d514/internal/remote/remote_client.go#L232-L236

A simple way to reproduce this is to add time.Sleep(time.Second*3) before if r.callback != nil, then callback return get resp.Err, which is not nil (because the context timeout, resp.Err was changed to ctx timeout error)

https://github.com/apache/rocketmq-client-go/blob/7ae83c49351a96d78ebccec5dd545f0c05e3d514/internal/remote/future.go#L47-L53

that is, r was changed by context timeout

https://github.com/apache/rocketmq-client-go/blob/7ae83c49351a96d78ebccec5dd545f0c05e3d514/internal/remote/future.go#L63-L66

So response future should close channel before callback (that is, put the callback at the end of the entire calling process)