wwbmmm commented on code in PR #3631:
URL: https://github.com/apache/brpc/pull/3631#discussion_r4348918068
##########
src/brpc/policy/mongo_protocol.cpp:
##########
@@ -212,15 +239,13 @@ void ProcessMongoRequest(InputMessageBase* msg_base) {
const Server::MethodProperty *mp =
server_accessor.FindMethodPropertyByFullName(
srv_des->method(0)->full_name());
- MongoContextMessage* context_msg =
- dynamic_cast<MongoContextMessage*>(socket->parsing_context());
- if (nullptr == context_msg) {
+ if (nullptr == msg->context) {
LOG(WARNING) << "socket context wasn't set correctly";
return;
}
SendMongoResponse* mongo_done = new SendMongoResponse(server);
- mongo_done->cntl.set_mongo_session_data(context_msg->context());
+ mongo_done->cntl.set_mongo_session_data(msg->context.get());
Review Comment:
[replied by brpc-oncall robot] No dangling pointer here:
`Controller::_mongo_session_data` is a `butil::intrusive_ptr<MongoContext>`
(controller.h:898), and `set_mongo_session_data()` assigns the raw pointer to
it (controller.cpp:1885). `intrusive_ptr::operator=(T*)` takes its own strong
reference, so the controller keeps the context alive after `msg` is destroyed.
Added a comment at the call site to make this explicit.
##########
test/brpc_socket_unittest.cpp:
##########
@@ -2130,6 +2131,88 @@ TEST_F(SocketTest, shutdown_write) {
}
}
+// A parsing context taking a while to be described, during which the bthread
+// parsing messages of the socket may reset the context.
+class SlowDescribedContext : public brpc::Destroyable, public
brpc::Describable {
+public:
+ SlowDescribedContext(bthread::CountdownEvent* describing,
+ butil::atomic<int>* ndestroyed)
+ : _describing(describing)
+ , _ndestroyed(ndestroyed)
+ , _name("SlowDescribedContext") {}
+
+ void Describe(std::ostream& os, const brpc::DescribeOptions&) const
override {
+ _describing->signal();
+ // Not bthread_usleep(): DebugSocket() describes under a pthread mutex.
+ usleep(100000);
+ os << _name;
+ }
+
+ void Destroy() override {
+ _ndestroyed->fetch_add(1);
+ delete this;
+ }
+
+private:
+ bthread::CountdownEvent* _describing;
+ butil::atomic<int>* _ndestroyed;
+ std::string _name;
+};
+
+struct DebugSocketArg {
+ brpc::SocketId id;
+ bthread::CountdownEvent* describing;
+ std::string description;
+};
+
+void* DescribeSocket(void* void_arg) {
+ DebugSocketArg* arg = static_cast<DebugSocketArg*>(void_arg);
+ std::ostringstream os;
+ brpc::Socket::DebugSocket(os, arg->id);
+ arg->description = os.str();
+ return nullptr;
+}
+
+void* ResetParsingContextWhileDescribing(void* void_arg) {
+ DebugSocketArg* arg = static_cast<DebugSocketArg*>(void_arg);
+ EXPECT_EQ(0, arg->describing->wait());
+ brpc::SocketUniquePtr s;
+ EXPECT_EQ(0, brpc::Socket::Address(arg->id, &s));
+ // What InputMessenger does before trying other protocols.
+ s->reset_parsing_context(nullptr);
+ return nullptr;
+}
Review Comment:
[replied by brpc-oncall robot] Fixed: the resetting pthread now stores an
error code in the shared argument and the main thread asserts on it after
`pthread_join`, so failures are reported deterministically.
##########
test/brpc_socket_unittest.cpp:
##########
@@ -2130,6 +2131,88 @@ TEST_F(SocketTest, shutdown_write) {
}
}
+// A parsing context taking a while to be described, during which the bthread
+// parsing messages of the socket may reset the context.
+class SlowDescribedContext : public brpc::Destroyable, public
brpc::Describable {
+public:
+ SlowDescribedContext(bthread::CountdownEvent* describing,
+ butil::atomic<int>* ndestroyed)
+ : _describing(describing)
+ , _ndestroyed(ndestroyed)
+ , _name("SlowDescribedContext") {}
+
+ void Describe(std::ostream& os, const brpc::DescribeOptions&) const
override {
+ _describing->signal();
+ // Not bthread_usleep(): DebugSocket() describes under a pthread mutex.
+ usleep(100000);
Review Comment:
[replied by brpc-oncall robot] Fixed: added an explicit `#include
<unistd.h>`.
--
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]
