From 5d1835f87f96342300931689cf437ac760110d4f Mon Sep 17 00:00:00 2001 From: Jose Ulises Nino Rivera Date: Mon, 21 Oct 2019 10:54:09 -0700 Subject: [PATCH] core: fix shutdown libevent crash (#527) Description: #498 did not fully solve #492. The reset cleanly destructed all objects. However, because destruction was posted in to the event dispatcher, the event dispatcher was left with bad accesses. This PR fixes the issue by issuing shutdown on the dispatcher, and only destructing once the event loop has exited and control has returned to the Engine's run function. Risk Level: med - fixing crash on shutdown Testing: local Fixes #492 Signed-off-by: Jose Nino Signed-off-by: JP Simard --- mobile/library/common/engine.cc | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/mobile/library/common/engine.cc b/mobile/library/common/engine.cc index fe5b39d51ed6..97c88e86095f 100644 --- a/mobile/library/common/engine.cc +++ b/mobile/library/common/engine.cc @@ -70,7 +70,14 @@ envoy_status_t Engine::run(std::string config, std::string log_level) { } // mutex_ // The main run loop must run without holding the mutex, so that the destructor can acquire it. - return TS_UNCHECKED_READ(main_common_)->run() ? ENVOY_SUCCESS : ENVOY_FAILURE; + bool run_success = TS_UNCHECKED_READ(main_common_)->run(); + // After the event loop has exited clean up any state that has to be cleaned from the context of + // main_thread_. + // It is important to destroy here, because otherwise the destructors will run from the context of + // the application's main thread, not the Engine's main_thread_. + postinit_callback_handler_.reset(); + TS_UNCHECKED_READ(main_common_).reset(); + return run_success ? ENVOY_SUCCESS : ENVOY_FAILURE; } Engine::~Engine() { @@ -87,12 +94,11 @@ Engine::~Engine() { cv_.wait(mutex_); } ASSERT(main_common_); - // Gracefully shutdown the running envoy instance by resetting the main_common_ unique_ptr. - // Destroying MainCommon's member variables shutsdown things in the correct order and - // gracefully. event_dispatcher_->post([this]() -> void { callbacks_.on_exit(); - TS_UNCHECKED_READ(main_common_).reset(); + // This call will gracefully shutdown the Server::Instance and exit the event loop, + // returning main_thread_'s execution to Engine::run + TS_UNCHECKED_READ(main_common_)->server()->shutdown(); }); } // _mutex