#89 - Make Manager::shutdown() idempotent to prevent post-finalize log crash (#90)

* #89 - Make Manager::shutdown() idempotent to prevent post-finalize log crash

Every Manager backend's shutdown() begins with a DAQIRI_LOG_INFO call,
and is invoked twice during the typical bench lifecycle:
1. Explicitly from main() via daqiri::shutdown().
2. Again from the manager's destructor (or, for SocketMgr in RoCE mode,
   a destructor cascade into RdmaMgr::shutdown()) during C++
   __cxa_finalize.

By the time the destructor cascade fires, spdlog's default logger -- a
function-local static created lazily on the first DAQIRI_LOG_INFO -- has
already been destroyed. The DAQIRI_LOG_INFO at the top of the second
shutdown() call then crashes inside spdlog::sink_it_.

Repro on DGX Spark: daqiri_bench_rdma --mode both against
examples/daqiri_bench_rdma_tx_rx_spark.yaml segfaults immediately after
the legitimate shutdown completes. Backtrace shows
__cxa_finalize -> ~SocketMgr -> SocketMgr::shutdown ->
RdmaMgr::shutdown -> daqiri::log_formatted_message ->
spdlog::logger::log_ -> spdlog::logger::sink_it_ -> SIGSEGV.

Fix: short-circuit shutdown() on subsequent calls by returning early
when initialized_ is false. Applied symmetrically to RdmaMgr, DpdkMgr,
and SocketMgr -- the log-first body-second pattern is identical in all
three. DpdkMgr's existing num_init reference-counted body is preserved;
the guard only activates after the body has cleared initialized_ in
the final shutdown.

Verified by repeated daqiri_bench_rdma --mode both runs from a
bash-parent shell. Pre-fix: SIGSEGV 100% reproducible. Post-fix: clean
exit 0, all destructor markers run in order.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: rgurunathan <rgurunathan@nvidia.com>

* #89 - Tighten SocketMgr::shutdown() guard to preserve init-failure cleanup

Greptile review of the original idempotency commit flagged the secondary
`if (!initialized_ && !running_.load()) { return; }` in SocketMgr::shutdown()
as having a dead `!initialized_` clause, since the new top-of-function guard
`if (!initialized_) { return; }` already covers that state. Investigation
surfaced the deeper concern: the top guard was too aggressive.
SocketMgr::initialize() sets initialized_=false and running_=true before
running setup, then sets initialized_=true on success. If setup_tcp_endpoint
or setup_udp_endpoint throws after spawning an accept_thread or io_thread,
the catch-block shutdown() call entered with initialized_=false and
running_=true. Under the original top guard the cleanup body was skipped,
leaving the worker threads joinable on the EndpointState — the destructor
cascade would then std::terminate on an unjoined std::thread.

Tighten the top guard to require both flags cleared. The post-shutdown
re-entry from __cxa_finalize still fires (both flags cleared at the end of
the body) while the init-failure cleanup path (running_=true) falls through
and joins its threads. The pre-existing secondary check is now fully
redundant and removed.

DpdkMgr and RdmaMgr keep the simpler `if (!initialized_) { return; }` —
neither has an init-failure shutdown() caller, so the asymmetry is
intentional and isolated to the manager whose initialize() partially
spawns threads before setting initialized_=true.

Verified manually with both the existing DPDK / socket-udp / socket-tcp
normal-shutdown smokes and a new 2-endpoint UDP init-failure repro
(malformed remote IP on endpoint 2 → parse_ipv4_addr throws after
endpoint 1's io_thread is spawned): rc=1, no SIGSEGV / SIGABRT /
"terminate called" in stderr.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: rgurunathan <rgurunathan@nvidia.com>

---------

Signed-off-by: rgurunathan <rgurunathan@nvidia.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: RamyaGuru

src/managers/dpdk/daqiri_dpdk_mgr.cpp

@@ -4403,6 +4403,11 @@ Status DpdkMgr::send_tx_burst(BurstParams* burst) {
 }
 
 void DpdkMgr::shutdown() {
+  // Idempotency guard: shutdown() may be invoked a second time via ~DpdkMgr
+  // during C++ __cxa_finalize, by which point the spdlog default logger has
+  // already been destroyed and any DAQIRI_LOG_INFO here crashes inside
+  // spdlog::sink_it_. Skip the body (and the log calls) if already torn down.
+  if (!initialized_) { return; }
   DAQIRI_LOG_INFO("daqiri DPDK manager shutdown called {}", num_init);
 
   if (--num_init == 0) {

src/managers/rdma/daqiri_rdma_mgr.cpp

@@ -1512,6 +1512,12 @@ void RdmaMgr::initialize() {
 }
 
 void RdmaMgr::shutdown() {
+  // Idempotency guard: shutdown() runs explicitly from the caller AND again
+  // from ~RdmaMgr / ~SocketMgr during C++ __cxa_finalize. By the second call
+  // the spdlog default logger (a function-local static created lazily on the
+  // first DAQIRI_LOG_INFO) has already been destroyed, so any logging here
+  // crashes inside spdlog::sink_it_. Skip the whole body on subsequent calls.
+  if (!initialized_) { return; }
   DAQIRI_LOG_INFO("RDMA manager shutting down");
   rdma_force_quit.store(true);
 

src/managers/socket/daqiri_socket_mgr.cpp

@@ -466,6 +466,21 @@ void SocketMgr::clear_rx_queues() {
 }
 
 void SocketMgr::shutdown() {
+  // Idempotency guard: shutdown() may be invoked a second time via
+  // ~SocketMgr during C++ __cxa_finalize, after the spdlog default logger
+  // has been destroyed. Any DAQIRI_LOG_INFO from the cascade (here, the
+  // RoCE branch, or the manager method this delegates to) would then crash
+  // inside spdlog::sink_it_. Skip the whole body on subsequent calls.
+  //
+  // The guard checks BOTH flags because initialize() sets initialized_=false
+  // and running_=true before running setup, then sets initialized_=true on
+  // success. If setup throws, the catch block calls shutdown() with
+  // initialized_=false and running_=true to clean up any threads/sockets
+  // that were spawned partway. Guarding on initialized_ alone would skip
+  // that cleanup. Both flags are only cleared together after a successful
+  // shutdown() body, so the post-shutdown re-entry from __cxa_finalize is
+  // the only case where the guard fires.
+  if (!initialized_ && !running_.load()) { return; }
   if (is_roce_protocol()) {
 #if DAQIRI_MGR_RDMA
     if (roce_mgr_ != nullptr) {
@@ -476,8 +491,6 @@ void SocketMgr::shutdown() {
     return;
   }
 
-  if (!initialized_ && !running_.load()) { return; }
-
   running_.store(false);
 
   for (auto& ep : endpoints_) {