From 0d6f0a8451c69e26abcc9c2cd2a61f11f332c1b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Thu, 12 Dec 2024 19:01:08 -0800 Subject: [PATCH] Fix deadlock in standalone mcrouter on invalid config (#455) Summary: After 6c2142acd8e69edd40eed70a93ea17ee2909287d, standalone mcrouter now deadlocks if the router configuration was incorrect. Spotted on https://github.com/facebook/mcrouter/pull/449, where `test_unknown_named_handles` started failing due to the mcrouter process being tested never exiting. GDB [indicates](https://gist.github.com/mszabo-wikia/47916c5655deffdb95332e972a52caf8) a deadlock between three threads: ``` (gdb) info threads Id Target Id Frame * 1 Thread 0x7f7b4cce1e00 (LWP 211878) "mcrouter" syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 2 Thread 0x7f7b43fff6c0 (LWP 211882) "mcr-cpuaux-0" syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 3 Thread 0x7f7b49e0a6c0 (LWP 211879) "IOThreadPool0" syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 ``` * Thread 1, the main thread, has triggered exit() and is waiting on the auxiliary thread pool to be destroyed. * Thread 2, an auxiliary thread pool thread, is in the process of destroying the CarbonRouterInstance due to 6c2142acd8e69edd40eed70a93ea17ee2909287d and is blocked on destroying the virtual EVBs by child proxies. These however are ultimately sourced from the IO thread pool which is also used by AsyncMcServer. * Thread 3, an IO thread pool thread, is blocked by AsyncMcServer which is waiting to be started by later initialization code that never runs due to the config error, preventing the IO thread pool itself from being destroyed. Fix it by initializing the AsyncMcServer only after the router has been initialized. Pull Request resolved: https://github.com/facebook/mcrouter/pull/455 Reviewed By: lenar-f Differential Revision: D67069250 Pulled By: disylh fbshipit-source-id: d6edff40b9f40ee7925e94c4ee9cf985c10a98de --- mcrouter/Server-inl.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/mcrouter/Server-inl.h b/mcrouter/Server-inl.h index 788a45420..26c90c54d 100644 --- a/mcrouter/Server-inl.h +++ b/mcrouter/Server-inl.h @@ -310,11 +310,6 @@ bool runServerDual( auto evbs = extractEvbs(*ioThreadPool); CHECK_EQ(evbs.size(), mcrouterOpts.num_proxies); - // Create AsyncMcServer instance - asyncMcServer = - std::make_shared(detail::createAsyncMcServerOptions( - mcrouterOpts, standaloneOpts, &evbs)); - // Create CarbonRouterInstance if (standaloneOpts.remote_thread) { router = @@ -328,6 +323,11 @@ bool runServerDual( return false; } + // Create AsyncMcServer instance + asyncMcServer = + std::make_shared(detail::createAsyncMcServerOptions( + mcrouterOpts, standaloneOpts, &evbs)); + setupRouter(mcrouterOpts, standaloneOpts, router, preRunCb); // Create CarbonRouterClients for each worker thread @@ -515,10 +515,6 @@ bool runServer( // Get EVB of main thread auto localEvb = ioThreadPool->getEventBaseManager()->getEventBase(); - asyncMcServer = - std::make_shared(detail::createAsyncMcServerOptions( - mcrouterOpts, standaloneOpts, &evbs)); - if (standaloneOpts.remote_thread) { router = CarbonRouterInstance::init("standalone", mcrouterOpts); @@ -531,6 +527,10 @@ bool runServer( return false; } + asyncMcServer = + std::make_shared(detail::createAsyncMcServerOptions( + mcrouterOpts, standaloneOpts, &evbs)); + setupRouter(mcrouterOpts, standaloneOpts, router, preRunCb); auto shutdownStarted = std::make_shared>(false);