You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "lizhimins (via GitHub)" <gi...@apache.org> on 2026/06/08 07:43:18 UTC

[PR] [C++] Fix critical correctness, thread safety, and memory management bugs [rocketmq-clients]

lizhimins opened a new pull request, #1259:
URL: https://github.com/apache/rocketmq-clients/pull/1259

   ## Summary
   
   Fix 8 bugs in the C++ client SDK (4 correctness, 3 thread safety, 1 memory management):
   
   - **onVerifyMessage switch fall-through** — SUCCESS case fell through to FAILURE, all verifications reported as failed
   - **onVerifyMessage callback never invoked** — verification result silently discarded, server-side verification mechanism broken
   - **schedule() empty body** — FIFO consumption retry completely non-functional, failed tasks silently dropped
   - **std::remove_if without erase** — SimpleConsumer queue assignments only grew, never shrank, causing duplicate consumption
   - **Static task_id data race** — non-atomic static shared across SchedulerImpl instances
   - **state_ read without lock** — TelemetryBidiReactor read state_ under wrong mutex
   - **Use-after-free in asyncCallback** — missing return after delete led to dangling pointer capture and null deref
   - **const_cast UB in sync send** — SendCallback signature changed from `const SendReceipt&` to `SendReceipt&` to eliminate undefined behavior
   
   ## Changed files (11 source + 1 doc)
   
   | File | Change |
   |------|--------|
   | `PushConsumerImpl.cpp` | Add `break;` to switch cases, add `cb(cmd);` call |
   | `ConsumeMessageServiceImpl.cpp` | Implement `schedule()` with delayed retry via SchedulerImpl |
   | `SimpleConsumerImpl.cpp` | Complete erase-remove idiom |
   | `SchedulerImpl.cpp` | `static uint32_t` → `static atomic<uint32_t>` |
   | `TelemetryBidiReactor.cpp` | Acquire `state_mtx_` before reading `state_`, remove redundant checks |
   | `RpcClientImpl.cpp` | Add `return;` after `delete invocation_context` |
   | `SendCallback.h` | `const SendReceipt&` → `SendReceipt&` |
   | `ProducerImpl.cpp` | Remove `const_cast`, move directly from `receipt` |
   | `FifoProducerPartition.cpp/.h` | Update `onComplete` and lambda signatures |
   | `SendContextTest.cpp` | Update test lambda signature |
   | `bugfix-correctness-threadsafety-memory.md` | Detailed write-up of all fixes |
   
   ## Test plan
   
   - [x] CMake build: all targets (library, tests, examples) compiled successfully
   - [x] CMake tests: 24/24 passed
   - [x] Bazel build: all library and test targets compiled (3387 actions)
   - [x] Bazel tests: 25/25 passed
   - [x] Runtime: Producer (sync + async), PushConsumer, SimpleConsumer validated against live RocketMQ 5.x instance
   
   Closes #1258


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [C++] Fix critical correctness, thread safety, and memory management bugs [rocketmq-clients]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #1259:
URL: https://github.com/apache/rocketmq-clients/pull/1259


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org