apache / apache/rocketmq

[Bug] Async request future remains registered after synchronous send failure

Open
#10,696 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
22.6k
Forks
12k
Avg merge
3d 1h
Merged PRs (30d)
27

Description

### 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.

Contributor guide

Open the contributing guide

Research direction

Start with the three asynchronous request overloads and RequestFutureHolder, then compare their cleanup with the synchronous request overloads' finally-block behavior. Add regression coverage for synchronous failures in the default, selector, and explicit-queue paths, plus successful asynchronous handoff; done means the exact future is removed only after synchronous failure and retained on success.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend-api-design, distributed-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.