apache / apache/rocketmq

[Bug] Pop long-polling: retry topic message with null properties throws NPE in the reput thread and stalls broker-wide dispatch

Open Beginner friendly
#10,990 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 asking a question, which should be created in [GitHub Discussions](https://github.com/apache/rocketmq/discussions).
- [x] I have searched the [GitHub Issues](https://github.com/apache/rocketmq/issues) and [GitHub Discussions](https://github.com/apache/rocketmq/discussions) of this repository and believe this is not a duplicate.
- [x] I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ.

### Runtime platform environment

- OS: Linux
- Component: Broker (`PopLongPollingService` + `DefaultMessageStore.ReputMessageService`)

### RocketMQ version

- branch: develop
- Git commit id: e348efa66

### JDK Version

JDK 8

### Describe the Bug

`PopLongPollingService#notifyMessageArrivingFromRetry` dereferences the dispatch request's property map without a null check:

```java
private void notifyMessageArrivingFromRetry(String topic, int queueId, Long tagsCode, long msgStoreTime,
byte[] filterBitMap, Map properties) {
String prefix = MixAll.RETRY_GROUP_TOPIC_PREFIX;
String originGroup = properties.get(MessageConst.PROPERTY_ORIGIN_GROUP); // NPE when properties == null
```

A `DispatchRequest` legitimately carries a null properties map: `MessageDecoder.string2messageProperties` returns null for a message stored without properties (e.g. a message written by a non-Java client, or through any path that stores a message on a `%RETRY%`-prefixed topic without user properties). The store's own code acknowledges this — `DefaultMessageStore#notifyMessageArrive4MultiQueue` explicitly guards `prop == null`, and `PullRequestHoldService#notifyMessageArriving` guards `properties != null` — but the pop retry branch does not.

Why this is severe: the listener is invoked from `DefaultMessageStore.ReputMessageService#doReput` **before** `reputFromOffset` is advanced, and `doReput` only catches `RocksDBException`. The resulting `NullPointerException` propagates to `ServiceThread.run`, which logs and loops — then re-reads the **same** commitlog record and throws again, forever. The broker stops dispatching *all* messages (consume queues stop advancing, no long-polling wakeups, consumers see a full outage) while flooding the log at ~1000 lines/s. A single poison message is enough; it stays poisoned across restarts because `reputFromOffset` is recovered from the consume queue state.

### Steps to Reproduce

1. On a broker with pop enabled, store a message with no properties on a topic named `%RETRY%` (any client able to write that topic; the properties map in the dispatch request is then null).
2. Watch the reput thread loop on `service has exception. NullPointerException` at `PopLongPollingService.notifyMessageArrivingFromRetry`, with `reputFromOffset` frozen.

In a unit test, calling `notifyMessageArrivingWithRetryTopic("%RETRY%g", -1, -1, -1L, 0L, null, null)` throws NPE on current develop.

### What Did You Expect to See?

The notification is skipped (there is no origin group to wake up), and dispatch continues.

### What Did You See Instead?

NPE in the reput thread → the same message is re-dispatched forever → broker-wide dispatch stall.

### Additional Context

Fix: return early when `properties == null` (a retry topic message without properties can't be mapped back to an origin group, so there is nothing to wake up), consistent with the existing guards in `PullRequestHoldService` and `notifyMessageArrive4MultiQueue`. I will submit a PR with a regression test.

Contributor guide

Open the contributing guide

Research direction

Start at PopLongPollingService#notifyMessageArrivingFromRetry and trace its caller from DefaultMessageStore.ReputMessageService#doReput. Compare the existing null guards in PullRequestHoldService#notifyMessageArriving and DefaultMessageStore#notifyMessageArrive4MultiQueue, then run the unit case invoking notifyMessageArrivingWithRetryTopic with null properties. Done means the notification is skipped without an exception and dispatch can continue.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.