wwbmmm commented on code in PR #3631:
URL: https://github.com/apache/brpc/pull/3631#discussion_r4348920028
##########
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;
+}
+
+TEST_F(SocketTest, debug_socket_keeps_parsing_context_alive) {
+ brpc::SocketId id;
+ brpc::SocketOptions options;
+ ASSERT_EQ(0, brpc::Socket::Create(options, &id));
+ brpc::SocketUniquePtr s;
+ ASSERT_EQ(0, brpc::Socket::Address(id, &s));
+
+ bthread::CountdownEvent describing(1);
+ butil::atomic<int> ndestroyed(0);
+ s->reset_parsing_context(new SlowDescribedContext(&describing,
&ndestroyed));
+
+ DebugSocketArg arg = { id, &describing, "" };
+ bthread_t describer;
+ ASSERT_EQ(0, bthread_start_background(&describer, nullptr, DescribeSocket,
&arg));
+ // Reset in a pthread: signaling a bthread in Describe() would switch to
+ // it on the worker of the describer, which holds a pthread mutex.
+ pthread_t resetter;
+ ASSERT_EQ(0, pthread_create(&resetter, nullptr,
+ ResetParsingContextWhileDescribing, &arg));
+ ASSERT_EQ(0, bthread_join(describer, nullptr));
+ ASSERT_EQ(0, pthread_join(resetter, nullptr));
+
+ ASSERT_NE(std::string::npos, arg.description.find(
+ "parsing_context=SlowDescribedContext{SlowDescribedContext}"))
+ << arg.description;
Review Comment:
[replied by brpc-oncall robot] Fixed: the assertion now only checks that
`parsing_context=` and `SlowDescribedContext` appear in the output, together
with the destruction count and the reset context.
--
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]
