wwbmmm commented on code in PR #3631:
URL: https://github.com/apache/brpc/pull/3631#discussion_r4370437083
##########
src/brpc/policy/mongo_protocol.cpp:
##########
@@ -161,13 +183,18 @@ ParseResult ParseMongoMessage(butil::IOBuf* source,
socket_context_msg = new MongoContextMessage(context);
socket->reset_parsing_context(socket_context_msg);
}
- policy::MostCommonMessage* msg = policy::MostCommonMessage::Get();
+ MongoInputMessage* msg = MongoInputMessage::Get();
Review Comment:
[replied by brpc-oncall robot] Pooled objects are clean when handed out:
`MongoInputMessage::DestroyImpl()` clears `meta` and `payload`, calls
`context.reset()`, and only then calls `butil::return_object(this)`, the same
pattern as `MostCommonMessage`. The object pool does not run constructors on
reused objects, which is why `DestroyImpl` is where the state is reset. A
message that is never destroyed cannot be handed out again.
##########
src/brpc/policy/mongo_protocol.cpp:
##########
@@ -161,13 +183,18 @@ ParseResult ParseMongoMessage(butil::IOBuf* source,
socket_context_msg = new MongoContextMessage(context);
socket->reset_parsing_context(socket_context_msg);
}
- policy::MostCommonMessage* msg = policy::MostCommonMessage::Get();
+ MongoInputMessage* msg = MongoInputMessage::Get();
source->cutn(&msg->meta, sizeof(buf));
size_t act_body_len = source->cutn(&msg->payload, body_len - sizeof(buf));
if (act_body_len != body_len - sizeof(buf)) {
CHECK(false); // Very unlikely, unless memory is corrupted.
return MakeParseError(PARSE_ERROR_TRY_OTHERS);
}
+ MongoContextMessage* context_msg =
+ dynamic_cast<MongoContextMessage*>(socket_context_msg);
+ if (nullptr != context_msg) {
+ msg->context = context_msg->context();
+ }
Review Comment:
[replied by brpc-oncall robot] Pooled objects are clean when handed out:
`MongoInputMessage::DestroyImpl()` clears `meta` and `payload`, calls
`context.reset()`, and only then calls `butil::return_object(this)`, the same
pattern as `MostCommonMessage`. The object pool does not run constructors on
reused objects, which is why `DestroyImpl` is where the state is reset. A
message that is never destroyed cannot be handed out again.
##########
src/brpc/policy/mongo_protocol.cpp:
##########
@@ -161,13 +183,18 @@ ParseResult ParseMongoMessage(butil::IOBuf* source,
socket_context_msg = new MongoContextMessage(context);
socket->reset_parsing_context(socket_context_msg);
}
- policy::MostCommonMessage* msg = policy::MostCommonMessage::Get();
+ MongoInputMessage* msg = MongoInputMessage::Get();
source->cutn(&msg->meta, sizeof(buf));
size_t act_body_len = source->cutn(&msg->payload, body_len - sizeof(buf));
if (act_body_len != body_len - sizeof(buf)) {
CHECK(false); // Very unlikely, unless memory is corrupted.
Review Comment:
[replied by brpc-oncall robot] That path is unreachable: `CHECK(false)`
aborts the process before the `return`. This is pre-existing code; before this
change the same `CHECK` followed `MostCommonMessage::Get()`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
