RockteMQ-AI commented on PR #10675: URL: https://github.com/apache/rocketmq/pull/10675#issuecomment-5113225203
## Review by github-manager-bot ### Summary Fixes three `(int)(longA - longB)` integer overflow bugs in comparators by replacing them with `Long.compare()`. These overflows corrupt ordering invariants in `ConcurrentSkipListSet` (pop long-polling) and sort lists (pop revival), potentially causing request misordering or starvation. ### Findings - **[Critical → Fixed]** `PopRequest.java:95,98` — The `op` counter starts at `Long.MIN_VALUE`, making overflow essentially guaranteed in production. The `ConcurrentSkipListSet` backed by this comparator would silently corrupt its ordering. `Long.compare` is the correct fix. - **[Critical → Fixed]** `PopCheckPoint.java:215` — `startOffset` is a `long` queue offset; long-running brokers will eventually exceed `Integer.MAX_VALUE` delta. Fix is correct. - **[Critical → Fixed]** `PopReviveService.java:728` — Same pattern, same fix. Consistent with the prior #10579 fix in `DefaultElectPolicy`. - **[Info]** Tests — Excellent regression coverage. Using reflection to set `op` to extreme values and verify ordering is the right approach. 6 tests across 2 classes cover all three comparators. ### Suggestions None — this is a well-documented, thoroughly tested fix for a real data-corruption bug. The PR description clearly explains the impact of each comparator. --- *Automated review by github-manager-bot* -- 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]
