ai-yang opened a new issue, #10696:
URL: https://github.com/apache/rocketmq/issues/10696
### Before Creating the Bug Report
- [x] I found a bug, not just a question.
- [x] I searched existing GitHub Issues, Discussions, and pull requests and
believe this is not a duplicate.
- [x] I confirmed that this bug belongs to `apache/rocketmq`.
### Runtime platform environment
- OS: Ubuntu 22.04, Linux 5.15.0-185-generic, x86_64
### RocketMQ version
- Branch: `develop`
- Commit: `00e45b8a6db23efbe756d0306f10716156cfd4dd`
### JDK Version
- OpenJDK 11.0.31
### Describe the Bug
The three asynchronous request-reply overloads that accept a
`RequestCallback` register a `RequestResponseFuture` in the singleton
`RequestFutureHolder` before invoking the underlying send method:
- `request(Message, RequestCallback, long)`
- `request(Message, MessageQueueSelector, Object, RequestCallback, long)`
- `request(Message, MessageQueue, RequestCallback, long)`
If `sendDefaultImpl`, `sendSelectImpl`, or `sendKernelImpl` throws
synchronously before returning, the exception is propagated to the caller but
the registered future is not removed.
The stale future remains in `requestFutureTable` until a later timeout scan.
Besides retaining the request and callback longer than necessary, the timeout
path can execute the callback after the caller has already observed the
synchronous exception.
The synchronous request overloads already avoid this by removing their
future in a `finally` block. This gap is also distinct from #10613 / #10614,
which addressed premature and duplicate callbacks after send initiation.
### Steps to Reproduce
1. Start a producer and invoke an asynchronous request with a callback.
2. Make send initiation fail synchronously, for example:
- no route is available for the topic;
- the supplied `MessageQueueSelector` throws;
- an explicit queue resolves to no broker address.
3. Catch the resulting `MQClientException`.
4. Inspect `RequestFutureHolder.getInstance().getRequestFutureTable()` using
the message correlation ID.
A minimal regression assertion is:
```java
assertThrows(MQClientException.class,
() -> producerImpl.request(message, requestCallback, timeout));
assertFalse(RequestFutureHolder.getInstance()
.getRequestFutureTable()
.containsKey(correlationId));
```
The final assertion fails on the current `develop` branch for all three
callback overloads.
### What Did You Expect to See?
When send initiation throws synchronously, the exact `RequestResponseFuture`
registered by that invocation should be removed immediately. The original
exception should still be propagated, and no timeout callback should be
delivered for a request that was never successfully handed off.
A normally returned asynchronous send invocation must continue to retain its
future until reply, send failure, or timeout processing claims it.
### What Did You See Instead?
The API throws the synchronous send exception, but the future remains
registered and can later be processed as an expired request.
### Additional Context
A race-safe cleanup can use `ConcurrentHashMap.remove(correlationId,
requestResponseFuture)` so that a replacement value under the same correlation
ID is not removed. Regression coverage should include the default, selector,
and explicit-queue overloads, plus a normal-path assertion that the future is
retained after a successful asynchronous handoff.
--
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]
