[fixit] Ensure ordering between iomgr and event engine shutdown (#30764)

* [fixit] Terminate event engine before iomgr

* guard against self-joins

* build

* fix api_fuzzer

* Automated change: Fix sanity tests

* add test

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
pull/30767/head
Craig Tiller 2 years ago committed by GitHub
parent 7dfca52ed6
commit 5a5adfb1b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      BUILD
  2. 6
      src/core/lib/event_engine/posix_engine/timer_manager.cc
  3. 2
      src/core/lib/event_engine/posix_engine/timer_manager.h
  4. 5
      src/core/lib/surface/init.cc
  5. 3
      test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.cc
  6. 14
      test/core/surface/init_test.cc

@ -428,6 +428,7 @@ grpc_cc_library(
"grpc_trace",
"http_connect_handshaker",
"iomgr_timer",
"posix_event_engine_timer_manager",
"slice",
"tcp_connect_handshaker",
],
@ -493,6 +494,7 @@ grpc_cc_library(
"grpc_trace",
"http_connect_handshaker",
"iomgr_timer",
"posix_event_engine_timer_manager",
"slice",
"tcp_connect_handshaker",
],
@ -2360,6 +2362,7 @@ grpc_cc_library(
"forkable",
"gpr",
"gpr_codegen",
"gpr_tls",
"posix_event_engine_timer",
"time",
],

@ -32,8 +32,11 @@
#include <grpc/support/log.h>
#include <grpc/support/time.h>
#include "src/core/lib/gpr/tls.h"
#include "src/core/lib/gprpp/thd.h"
static GPR_THREAD_LOCAL(bool) g_timer_thread = false;
namespace grpc_event_engine {
namespace posix_engine {
@ -196,6 +199,7 @@ void TimerManager::MainLoop() {
}
void TimerManager::RunThread(void* arg) {
g_timer_thread = true;
std::unique_ptr<RunThreadArgs> thread(static_cast<RunThreadArgs*>(arg));
thread->self->Run(std::move(thread->thread));
}
@ -208,6 +212,8 @@ void TimerManager::Run(grpc_core::Thread thread) {
if (thread_count_ == 0) cv_threadcount_.Signal();
}
bool TimerManager::IsTimerManagerThread() { return g_timer_thread; }
TimerManager::TimerManager() : host_(this) {
timer_list_ = absl::make_unique<TimerList>(&host_);
grpc_core::MutexLock lock(&mu_);

@ -60,6 +60,8 @@ class TimerManager final : public grpc_event_engine::experimental::Forkable {
void PostforkParent() override;
void PostforkChild() override;
static bool IsTimerManagerThread();
private:
struct RunThreadArgs {
TimerManager* self;

@ -37,7 +37,9 @@
#include "src/core/lib/channel/channel_stack_builder.h"
#include "src/core/lib/config/core_configuration.h"
#include "src/core/lib/debug/trace.h"
#include "src/core/lib/event_engine/default_event_engine.h"
#include "src/core/lib/event_engine/forkable.h"
#include "src/core/lib/event_engine/posix_engine/timer_manager.h"
#include "src/core/lib/gprpp/fork.h"
#include "src/core/lib/gprpp/sync.h"
#include "src/core/lib/gprpp/thd.h"
@ -179,6 +181,7 @@ void grpc_shutdown_internal_locked(void)
}
}
}
grpc_event_engine::experimental::ResetDefaultEventEngine();
grpc_iomgr_shutdown();
gpr_timers_global_destroy();
grpc_tracer_shutdown();
@ -208,6 +211,8 @@ void grpc_shutdown(void) {
grpc_core::ApplicationCallbackExecCtx* acec =
grpc_core::ApplicationCallbackExecCtx::Get();
if (!grpc_iomgr_is_any_background_poller_thread() &&
!grpc_event_engine::posix_engine::TimerManager::
IsTimerManagerThread() &&
(acec == nullptr ||
(acec->Flags() & GRPC_APP_CALLBACK_EXEC_CTX_FLAG_IS_INTERNAL_THREAD) ==
0)) {

@ -99,6 +99,9 @@ gpr_timespec FuzzingEventEngine::NowAsTimespec(gpr_clock_type clock_type) {
}
gpr_timespec FuzzingEventEngine::GlobalNowImpl(gpr_clock_type clock_type) {
if (g_fuzzing_event_engine == nullptr) {
return gpr_inf_future(clock_type);
}
GPR_ASSERT(g_fuzzing_event_engine != nullptr);
grpc_core::MutexLock lock(&g_fuzzing_event_engine->mu_);
return g_fuzzing_event_engine->NowAsTimespec(clock_type);

@ -20,10 +20,13 @@
#include <gtest/gtest.h>
#include "absl/synchronization/notification.h"
#include <grpc/grpc.h>
#include <grpc/support/log.h>
#include <grpc/support/time.h>
#include "src/core/lib/event_engine/default_event_engine.h"
#include "src/core/lib/iomgr/exec_ctx.h"
#include "test/core/util/test_config.h"
@ -137,6 +140,17 @@ TEST(Init, repeatedly_blocking) {
EXPECT_FALSE(grpc_is_initialized());
}
TEST(Init, TimerManagerHoldsLastInit) {
grpc_init();
absl::Notification n;
grpc_event_engine::experimental::GetDefaultEventEngine()->RunAfter(
std::chrono::seconds(1), [&n] {
grpc_shutdown();
n.Notify();
});
n.WaitForNotification();
}
int main(int argc, char** argv) {
grpc::testing::TestEnvironment env(&argc, argv);
::testing::InitGoogleTest(&argc, argv);

Loading…
Cancel
Save