[Bug] DefaultReceiptHandleManagerTest#testClientOffline is flaky during asynchronous cleanup

[unbridled-41]

Before Creating the Bug Report

• I found a test reliability/CI flaky issue, not a question.
• I searched existing GitHub Issues and found no obvious duplicate.
• The issue belongs to this repository.

Runtime platform environment

GitHub Actions Coverage workflow on a Linux runner.

RocketMQ version

Branch: develop. The failure was observed in the Coverage workflow.

JDK Version

JDK 8 (the Coverage workflow uses Java 8).

Describe the Bug

DefaultReceiptHandleManagerTest#testClientOffline is timing-sensitive during asynchronous cleanup.

After CLIENT_UNREGISTER calls clearGroup(), the group is removed from receiptHandleGroupMap synchronously. returnHandleGroup() then runs asynchronously on ReturnHandleGroupWorkerThread.

The test's StateEventListener calls messagingProcessor.changeInvisibleTime(...).whenComplete(...), but testClientOffline does not stub changeInvisibleTime() for this cleanup path. Mockito can therefore return null, causing a NullPointerException when the async worker calls whenComplete().

After that exception, the handle is not removed. When returnHandleGroup() sees that the group is still non-empty, it calls putIfAbsent() and can put the group back into receiptHandleGroupMap.

Meanwhile, the test immediately asserts receiptHandleManager.receiptHandleGroupMap.isEmpty() after triggering CLIENT_UNREGISTER. The final assertion therefore depends on async worker scheduling and can fail intermittently in Coverage CI.

This is a test reliability/flaky CI issue, not a production behavior report.

Steps to Reproduce

The failure was observed in the Coverage CI workflow. Run:

DefaultReceiptHandleManagerTest#testClientOffline

The problem is timing-sensitive, so a single local run does not guarantee reproduction. It was not claimed to reproduce consistently on an unmodified local checkout.

What Did You Expect to See?

testClientOffline should deterministically verify the asynchronous cleanup initiated by CLIENT_UNREGISTER, without failures caused by an unstubbed Mockito call or thread scheduling.

What Did You See Instead?

Coverage CI logged a NullPointerException in ReturnHandleGroupWorkerThread. The group could then be reinserted into the map, and the final isEmpty() assertion failed.

Additional Context

A test-only fix is prepared:

• Stub the cleanup-path changeInvisibleTime() call with a completed CompletableFuture.
• Use Awaitility to wait for async cleanup completion.
• Do not use Thread.sleep.
• Do not change production code.

Validation after the fix:

• JDK 8
• testClientOffline: 50/50 passes
• DefaultReceiptHandleManagerTest: 12/12 passes
• Proxy module: 308 tests run, 0 failures, 0 errors, 3 skipped; BUILD SUCCESS
• git diff --check passes

[RockteMQ-AI]

Issue Evaluation

Category: type/bug | Status: Confirmed

The reported flaky test issue has been verified against the current codebase on develop branch.

Root Cause Analysis:

The race condition is in DefaultReceiptHandleManagerTest#testClientOffline:

1. CLIENT_UNREGISTER triggers clearGroup() which removes the group from receiptHandleGroupMap synchronously, then submits returnHandleGroup() to the async worker thread.
2. returnHandleGroup() iterates handles and calls eventListener.fireEvent() which calls messagingProcessor.changeInvisibleTime(...).whenComplete(...).
3. In testClientOffline(), changeInvisibleTime() is not stubbed for this cleanup path. Mockito returns null, causing NullPointerException on .whenComplete().
4. The NPE prevents the handle from being removed from the group. Since the group remains non-empty, returnHandleGroup() calls putIfAbsent() which re-inserts the group into receiptHandleGroupMap.
5. The test immediately asserts receiptHandleGroupMap.isEmpty() after triggering the event, making the assertion dependent on async thread scheduling.

Impact: Test reliability in CI (Coverage workflow). Intermittent failures block CI pipelines.

Severity: Medium - test-only issue, no production behavior impact.

Proposed Fix Assessment: The author's proposed test-only fix is appropriate:

• Stub changeInvisibleTime() with a completed CompletableFuture for the cleanup path
• Use Awaitility to wait for async cleanup completion instead of immediate assertion
• No production code changes needed

An automated fix proposal will be generated. Reply /approve to proceed with PR generation.

---

Automated evaluation by github-manager-bot