RockteMQ-AI commented on PR #10555:
URL: https://github.com/apache/rocketmq/pull/10555#issuecomment-4832597837
Thanks for the detailed clarification @qianye1001 — you're absolutely right.
The previous review comment incorrectly stated that `Message.build()` ensures
consumption before mutation. As you verified, `build()` does **not** copy or
consume the `ByteString`; the protobuf `Message` continues to alias the
original `byte[]` via `UnsafeByteOperations.unsafeWrap()`.
Your analysis of the receive path is correct: `MessageDecoder.decode()`
allocates a fresh body array, and downstream code only touches
receipt/properties — not the body bytes — so the current usage is safe in
practice.
Agreed that the inline comment should document the **actual** invariant
rather than the incorrect one. The real safety guarantee is:
> The decoded body array must not be mutated or reused while the returned
protobuf message may still be referenced or serialized.
@Gautam-aman — could you update the safety comment in `GrpcConverter.java`
to reflect this invariant? Something along the lines of:
```java
// Safety: UnsafeByteOperations.unsafeWrap() aliases the source byte[] —
// the protobuf Message does NOT copy it. The caller must ensure the
// decoded body array is never mutated or reused while the Message
// (or any serialized form of it) may still be in use.
```
---
*Automated review by ${BOT_SIG}*
--
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]
