dong0713 opened a new issue, #10674:
URL: https://github.com/apache/rocketmq/issues/10674
### Before Creating the Bug Report
- [x] I found a bug, not just asking a question
- [x] I have searched the GitHub Issues and believe this is not a duplicate
- [x] I have confirmed that this bug belongs to the current repository
### Runtime platform environment
Any (logic bug, environment-independent)
### RocketMQ version
branch: develop (5.5.0)
### JDK Version
JDK 8
### Describe the Bug
PR #10579 recently fixed an integer overflow in `DefaultElectPolicy`'s
comparator, where a `long` offset delta was cast to `int` via `(int)
(o2.getMaxOffset() - o1.getMaxOffset())`. When the offset difference exceeds
`Integer.MAX_VALUE`, the cast overflows and the comparator returns the wrong
ordering.
After scanning the codebase, I found **three more comparators** with the
same `long`-subtraction-cast-to-`int` overflow pattern. All three involve
`long` fields (offsets / timestamps / counters) and all three are in production
hot paths.
#### 1. `PopRequest.COMPARATOR` (broker, long-polling)
`broker/src/main/java/org/apache/rocketmq/broker/longpolling/PopRequest.java:95,100`
```java
public static final Comparator<PopRequest> COMPARATOR = (o1, o2) -> {
int ret = (int) (o1.getExpired() - o2.getExpired()); // line 95: long
timestamp delta
if (ret != 0) {
return ret;
}
ret = (int) (o1.op - o2.op); // line 100:
long counter delta
if (ret != 0) {
return ret;
}
return -1;
};
```
- `expired` is a `long` timestamp (ms). Two requests far apart in time can
overflow `int`.
- `op` is a `long` initialized from `COUNTER.getAndIncrement()` starting at
`Long.MIN_VALUE` (line 29, 34). **Overflow here is essentially guaranteed** for
any non-trivial number of PopRequests, since the counter walks from
`Long.MIN_VALUE` upward and deltas between early and late requests easily
exceed `Integer.MAX_VALUE`.
- This comparator backs a `ConcurrentSkipListSet` for pop long-polling — a
broken comparator corrupts the ordered set's invariants, causing subtle request
ordering/delivery bugs.
#### 2. `PopCheckPoint.compareTo` (store, pop checkpoint)
`store/src/main/java/org/apache/rocketmq/store/pop/PopCheckPoint.java:215`
```java
@Override
public int compareTo(PopCheckPoint o) {
return (int) (this.getStartOffset() - o.getStartOffset());
}
```
- `startOffset` is a `long` queue offset. A long-running broker can produce
queue offsets whose delta exceeds `Integer.MAX_VALUE`. The `Comparable`
contract is relied upon by sorted collections; a broken `compareTo` violates
sort invariants.
#### 3. `PopReviveService.genSortList` (broker, pop revival)
`broker/src/main/java/org/apache/rocketmq/broker/processor/PopReviveService.java:728`
```java
sortList.sort((o1, o2) -> (int) (o1.getReviveOffset() -
o2.getReviveOffset()));
```
- `reviveOffset` is a `long` commit-log offset. Same overflow class as
#10579.
### Impact
- `PopRequest.COMPARATOR` is the most severe: the `op` counter starts at
`Long.MIN_VALUE`, so the overflow is not theoretical — it triggers for real
workloads, corrupting the `ConcurrentSkipListSet` that orders pop long-polling
requests. A broken comparator in a `SortedSet` can cause requests to be
misplaced, starved, or delivered out of order.
- `PopCheckPoint.compareTo` and `PopReviveService` sort affect pop
retry/checkpoint ordering correctness under large offsets.
All three are the **exact same bug class** as the already-fixed #10579.
### Steps to Reproduce
For `PopRequest.COMPARATOR` (the guaranteed-overflow case):
```java
// op counter starts at Long.MIN_VALUE; after many requests the delta
overflows int
PopRequest a = ...; // op = Long.MIN_VALUE
PopRequest b = ...; // op = Long.MIN_VALUE + (long)Integer.MAX_VALUE + 2
int cmp = PopRequest.COMPARATOR.compare(a, b);
// (int)(a.op - b.op) overflows -> wrong sign -> wrong ordering
```
### What Did You Expect to See?
Comparators over `long` fields should use `Long.compare(a, b)` (and
`Integer.compare` for the `int` tiebreaker) instead of
subtraction-cast-to-`int`, exactly as #10579 did for `DefaultElectPolicy`.
### What Did You See Instead?
Subtraction-cast-to-`int`, which overflows when the `long` delta exceeds
`Integer.MAX_VALUE`.
### Additional Context
This is a systematic scan of the same overflow class fixed by #10579. Happy
to submit a PR mirroring the #10579 fix (`Long.compare` / `Integer.compare`) if
the maintainers agree these are real.
--
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]
