Aias00 opened a new issue, #3530:
URL: https://github.com/apache/dubbo-go/issues/3530
### Problem
`ExchangeClient.AsyncRequest` adds a `PendingResponse` to the process-global
`pendingResponses` map and returns immediately after the write succeeds. Unlike
the sync `Request` path, the async path does **not** enforce the `timeout` on
the pending response: the getty layer returns immediately for callback requests
(`if !request.TwoWay || response.Callback != nil { return nil }`), so the
`timeout` argument is effectively ignored for async.
The only removal sites for an in-flight async pending are:
1. `Response.Handle` when the server reply arrives (the normal path).
2. `AsyncRequest`'s own error branch — only fires if the *write* failed.
If the write succeeds and the connection subsequently drops before the
server replies (network blip, server restart, session timeout evict), `getty`'s
`removeSession`/`close` close sessions but never iterate or fail the in-flight
`pendingResponses`. The pending entry leaks permanently in the global
`sync.Map`, and the user-supplied `common.AsyncCallback` is never invoked — the
caller's `RPCResult` is never populated, so the invocation hangs from the
caller's perspective.
This is distinct from #3440, which added cleanup on the *write-error* path;
this issue is the *connection-drop / no-reply* path.
### Current behavior
```go
// remoting/exchange_client.go AsyncRequest
rsp := NewPendingResponse(request.ID)
rsp.Callback = callback
AddPendingResponse(rsp)
err := client.client.Request(request, timeout, rsp)
if err != nil {
RemovePendingResponse(SequenceType(request.ID)) // only the
write-error path cleans up
result.Err = err
return err
}
result.Rest = rsp.response
return nil // no timeout enforcement; no removal/callback on no-reply
```
`getty` `removeSession`/`close` do not reference `pendingResponses`.
### Expected behavior
If no reply arrives within `timeout`, the async pending should be removed
from the map and the `AsyncCallback` invoked with a timeout error — the same
`timeout` contract the sync path honors. The reply path (`Response.Handle`) and
the timeout must not double-invoke the callback.
### Suggested approach
- After a successful `client.client.Request`, install
`time.AfterFunc(timeout, ...)` that calls `RemovePendingResponse(seq)`. If it
returns non-nil (reply has not arrived), invoke
`pr.Callback(pr.GetCallResponse())` with a timeout error.
- `RemovePendingResponse` is an atomic load-and-delete (`sync.Map`), so the
timer and the reply path cannot both win the pending; the loser gets `nil` and
skips the callback — exactly-once.
- Guard `timeout > 0` (a zero timeout means "no timeout", preserving the old
wait-forever behavior).
### Acceptance criteria
- [ ] On no-reply within `timeout`, the async pending is removed and the
`AsyncCallback` is invoked with a timeout error.
- [ ] The reply path and the timeout never both invoke the callback
(exactly-once via atomic removal).
- [ ] `timeout == 0` preserves the previous behavior (no timer).
- [ ] Regression test covers the no-reply timeout path (distinct from the
write-error cleanup test).
--
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]
