ai-yang opened a new issue, #10754: URL: https://github.com/apache/rocketmq/issues/10754
## Affected branch `develop` at `a06836dd564e5e43115493f775626cf98d51d10e`. ## Problem `LmqPullRequestHoldService.checkHoldRequest()` removes an LMQ `ManyPullRequest` bucket after observing it as empty. The empty check and `pullRequestTable.remove(key)` are not atomic with `PullRequestHoldService.suspendPullRequest()` adding a request to the same bucket. The following interleaving can occur: 1. the periodic check observes the current bucket as empty; 2. a concurrent request obtains that bucket from the map and appends a new suspended pull request; 3. the periodic check unconditionally removes the bucket based on its earlier empty observation. The new request remains in the detached `ManyPullRequest` object, but the hold table no longer contains that object. ## Deterministic reproduction A unit test pre-populates an empty bucket and uses a `ConcurrentHashMap` test double whose `remove(key)` is paused by two latches. This pauses the periodic check only after its empty condition has already selected the removal path. The test then calls the real `suspendPullRequest()` method, verifies that the existing bucket now contains the request, lets removal continue, and requires the same bucket to remain reachable from the table. The unmodified branch failed identically in 5/5 isolated JDK 8 Maven processes: ```text Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 java.lang.AssertionError: concurrently suspended request should remain reachable expected same:<...> was not:<null> ``` The reproduction has no sleep, network, timer, or random scheduling dependency. All latch and future waits are bounded only to turn a broken test into a prompt failure instead of a hang. ## Impact A concurrently suspended LMQ pull request can disappear from `pullRequestTable`. Later message-arrival and timeout scans cannot discover or wake it, so the broker-side long-poll request remains stranded until external connection or client timeout handling intervenes. ## Expected behavior An LMQ bucket must not be removed once a concurrent suspend or replay has made it non-empty. Every request that remains suspended must be reachable through `pullRequestTable` for later message-arrival and timeout processing. ## Suggested direction Make bucket insertion/replay and empty cleanup atomic per map key: - add newly suspended requests through `ConcurrentMap.compute`, creating or reusing the mapped bucket and appending inside the remapping function; - remove an LMQ bucket through `computeIfPresent`, using synchronized `ManyPullRequest.isEmpty()` as the authoritative check; - reinsert `notifyMessageArriving()` replay requests through the same map-aware helper, rather than appending them to a bucket that may have been detached while requests were evaluated. Changing only the final remove to a conditional remove is insufficient: a concurrent writer or replay can still retain and append to an object that cleanup has just detached from the map. ## Related work checked Searches covered open and closed issues and pull requests using `LmqPullRequestHoldService`, `ManyPullRequest`, `pullRequestTable`, `suspendPullRequest`, empty-bucket cleanup, and concurrent removal terms. No equivalent report, implementation, assignee, or maintainer handoff was found. #8341 and its unmerged PR #8342 only proposed replacing the redundant `getPullRequestList() == null` check with `isEmpty()`; they did not make the check/removal atomic or address concurrent suspend/replay. -- 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]
