diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc index 762632e159010..59640c8668933 100644 --- a/test/exe/main_common_test.cc +++ b/test/exe/main_common_test.cc @@ -26,7 +26,10 @@ #include "exe/signal_action.h" #endif +#include "absl/synchronization/notification.h" + using testing::HasSubstr; +using testing::IsEmpty; namespace Envoy { @@ -40,14 +43,13 @@ namespace Envoy { * https://github.com/envoyproxy/envoy/issues/2649 */ class MainCommonTest : public testing::Test { -public: +protected: MainCommonTest() : config_file_(Envoy::TestEnvironment::getCheckedEnvVar("TEST_RUNDIR") + "/test/config/integration/google_com_proxy_port_0.v2.yaml"), random_string_(fmt::format("{}", computeBaseId())), argv_({"envoy-static", "--base-id", random_string_.c_str(), "-c", config_file_.c_str(), - nullptr}), - envoy_return_(false), envoy_started_(false), envoy_finished_(false) {} + nullptr}) {} /** * Computes a numeric ID to incorporate into the names of @@ -87,87 +89,9 @@ class MainCommonTest : public testing::Test { addArg("init_only"); } - // Runs a an admin request specified in path, blocking until completion, and - // returning the response body. - std::string adminRequest(absl::string_view path, absl::string_view method) { - Thread::MutexBasicLockable mutex; - Thread::CondVar condvar; - bool done = false; - std::string out; - main_common_->adminRequest( - path, method, - [&mutex, &condvar, &done, &out](const Http::HeaderMap& /*response_headers*/, - absl::string_view body) { - Thread::LockGuard lock(mutex); - out = std::string(body); - done = true; - condvar.notifyOne(); - }); - { - Thread::LockGuard lock(mutex); - while (!done) { - condvar.wait(mutex); - } - } - return out; - } - - // Initiates Envoy running in its own thread. - void startEnvoy() { - envoy_thread_ = std::make_unique([this]() { - // Note: main_common_ is accesesed in the testing thread, but - // is race-free, as MainCommon::run() does not return until - // triggered with an adminRequest POST to /quitquitquit, which - // is done in the testing thread. - main_common_ = std::make_unique(argc(), argv()); - { - Thread::LockGuard lock(mutex_); - envoy_started_ = true; - started_condvar_.notifyOne(); - } - bool status = main_common_->run(); - main_common_.reset(); - { - Thread::LockGuard lock(mutex_); - envoy_finished_ = true; - envoy_return_ = status; - finished_condvar_.notifyOne(); - } - }); - } - - // Having started Envoy in its own thread, this waits for Envoy to be - // responsive by sending it a simple stats request. - void waitForEnvoyToStart() { - Thread::LockGuard lock(mutex_); - while (!envoy_started_) { - started_condvar_.wait(mutex_); - } - } - - // Having sent a /quitquitquit to Envoy, this blocks until Envoy exits. - bool waitForEnvoyToExit() { - { - Thread::LockGuard lock(mutex_); - while (!envoy_finished_) { - finished_condvar_.wait(mutex_); - } - } - envoy_thread_->join(); - return envoy_return_; - } - std::string config_file_; std::string random_string_; std::vector argv_; - std::unique_ptr envoy_thread_; - std::unique_ptr main_common_; - Thread::CondVar started_condvar_; - Thread::CondVar finished_condvar_; - Thread::MutexBasicLockable mutex_; - bool envoy_return_; - bool envoy_started_ GUARDED_BY(mutex_); - bool envoy_finished_ GUARDED_BY(mutex_); }; // Exercise the codepath to instantiate MainCommon and destruct it, with hot restart. @@ -226,13 +150,85 @@ TEST_F(MainCommonTest, LegacyMain) { EXPECT_EQ(EXIT_SUCCESS, return_code); } -TEST_F(MainCommonTest, AdminRequestGetStatsAndQuit) { +class AdminRequestTest : public MainCommonTest { +protected: + AdminRequestTest() + : envoy_return_(false), envoy_started_(false), envoy_finished_(false), + pause_before_run_(false), pause_after_run_(false) { + addArg("--disable-hot-restart"); + } + + // Runs an admin request specified in path, blocking until completion, and + // returning the response body. + std::string adminRequest(absl::string_view path, absl::string_view method) { + absl::Notification done; + std::string out; + main_common_->adminRequest( + path, method, + [&done, &out](const Http::HeaderMap& /*response_headers*/, absl::string_view body) { + out = std::string(body); + done.Notify(); + }); + done.WaitForNotification(); + return out; + } + + // Initiates Envoy running in its own thread. + void startEnvoy() { + envoy_thread_ = std::make_unique([this]() { + // Note: main_common_ is accesesed in the testing thread, but + // is race-free, as MainCommon::run() does not return until + // triggered with an adminRequest POST to /quitquitquit, which + // is done in the testing thread. + main_common_ = std::make_unique(argc(), argv()); + envoy_started_ = true; + started_.Notify(); + pauseResumeInterlock(pause_before_run_); + bool status = main_common_->run(); + pauseResumeInterlock(pause_after_run_); + main_common_.reset(); + envoy_finished_ = true; + envoy_return_ = status; + finished_.Notify(); + }); + } + + // Conditionally pauses at a critical point in the Envoy thread, waiting for + // the test thread to trigger something at that exact line. The test thread + // can then call resume_.Notify() to allow the Envoy thread to resume. + void pauseResumeInterlock(bool enable) { + if (enable) { + pause_point_.Notify(); + resume_.WaitForNotification(); + } + } + + // Having triggered Envoy to quit (via signal or /quitquitquit), this blocks until Envoy exits. + bool waitForEnvoyToExit() { + finished_.WaitForNotification(); + envoy_thread_->join(); + return envoy_return_; + } + + std::unique_ptr envoy_thread_; + std::unique_ptr main_common_; + absl::Notification started_; + absl::Notification finished_; + absl::Notification resume_; + absl::Notification pause_point_; + bool envoy_return_; + bool envoy_started_; + bool envoy_finished_; + bool pause_before_run_; + bool pause_after_run_; +}; + +TEST_F(AdminRequestTest, AdminRequestGetStatsAndQuit) { if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) { return; } - addArg("--disable-hot-restart"); startEnvoy(); - waitForEnvoyToStart(); + started_.WaitForNotification(); EXPECT_THAT(adminRequest("/stats", "GET"), HasSubstr("filesystem.reopen_failed")); adminRequest("/quitquitquit", "POST"); EXPECT_TRUE(waitForEnvoyToExit()); @@ -240,18 +236,106 @@ TEST_F(MainCommonTest, AdminRequestGetStatsAndQuit) { // This test is identical to the above one, except that instead of using an admin /quitquitquit, // we send ourselves a SIGTERM, which should have the same effect. -TEST_F(MainCommonTest, AdminRequestGetStatsAndKill) { +TEST_F(AdminRequestTest, AdminRequestGetStatsAndKill) { if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) { return; } - addArg("--disable-hot-restart"); startEnvoy(); - waitForEnvoyToStart(); + started_.WaitForNotification(); EXPECT_THAT(adminRequest("/stats", "GET"), HasSubstr("filesystem.reopen_failed")); kill(getpid(), SIGTERM); EXPECT_TRUE(waitForEnvoyToExit()); } +TEST_F(AdminRequestTest, AdminRequestBeforeRun) { + if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) { + return; + } + // Induce the situation where the Envoy thread is active, and main_common_ is constructed, + // but run() hasn't been issued yet. AdminRequests will not finish immediately, but will + // do so at some point after run() is allowed to start. + pause_before_run_ = true; + startEnvoy(); + pause_point_.WaitForNotification(); + + bool admin_handler_was_called = false; + std::string out; + main_common_->adminRequest( + "/stats", "GET", + [&admin_handler_was_called, &out](const Http::HeaderMap& /*response_headers*/, + absl::string_view body) { + admin_handler_was_called = true; + out = std::string(body); + }); + + // The admin handler can't be called until after we let run() go. + EXPECT_FALSE(admin_handler_was_called); + EXPECT_THAT(out, IsEmpty()); + + // Now unblock the envoy thread so it can wake up and process outstanding posts. + resume_.Notify(); + + // We don't get a notification when run(), so it's not safe to check whether the + // admin handler is called until after we quit. + adminRequest("/quitquitquit", "POST"); + EXPECT_TRUE(waitForEnvoyToExit()); + EXPECT_TRUE(admin_handler_was_called); + + // This just checks that some stat output was reported. We could pick any stat. + EXPECT_THAT(out, HasSubstr("filesystem.reopen_failed")); +} + +// Class to track whether an object has been destroyed, which it does by bumping an atomic. +class DestroyCounter { +public: + // Note: destroy_count is captured by reference, so the variable must last longer than + // the DestroyCounter. + explicit DestroyCounter(std::atomic& destroy_count) : destroy_count_(destroy_count) {} + ~DestroyCounter() { ++destroy_count_; } + +private: + std::atomic& destroy_count_; +}; + +TEST_F(AdminRequestTest, AdminRequestAfterRun) { + if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) { + return; + } + startEnvoy(); + started_.WaitForNotification(); + // Induce the situation where Envoy is no longer in run(), but hasn't been + // destroyed yet. AdminRequests will never finish, but they won't crash. + pause_after_run_ = true; + adminRequest("/quitquitquit", "POST"); + pause_point_.WaitForNotification(); // run() finished, but main_common_ still exists. + + // Admin requests will not work, but will never complete. The lambda itself will be + // destroyed on thread exit, which we'll track with an object that counts destructor calls. + + std::atomic lambda_destroy_count(0); + bool admin_handler_was_called = false; + { + // Ownership of the tracker will be passed to the lambda. + auto tracker = std::make_shared(lambda_destroy_count); + main_common_->adminRequest( + "/stats", "GET", + [&admin_handler_was_called, tracker](const Http::HeaderMap& /*response_headers*/, + absl::string_view /*body*/) { + admin_handler_was_called = true; + UNREFERENCED_PARAMETER(tracker); + }); + } + EXPECT_EQ(0, lambda_destroy_count); // The lambda won't be destroyed till envoy thread exit. + + // Now unblock the envoy thread so it can destroy the object, along with our unfinished + // admin request. + resume_.Notify(); + + EXPECT_TRUE(waitForEnvoyToExit()); + EXPECT_FALSE(admin_handler_was_called); + EXPECT_EQ(1, lambda_destroy_count); +} + } // namespace Envoy #endif // ENVOY_CONFIG_COVERAGE