diff --git a/configure.ac b/configure.ac index 63379cc0711..fe1e0a9e038 100644 --- a/configure.ac +++ b/configure.ac @@ -119,7 +119,7 @@ if test "x$enable_gcov" = "xyes"; then AC_MSG_ERROR(not compiling with gcc, which is required for gcov testing) fi - CFLAGS_COMMON+=" -fprofile-arcs -ftest-coverage" + CFLAGS_COMMON+=" -fprofile-arcs -ftest-coverage -DGCOV_ENABLED" AC_SUBST(CFLAGS_COMMON) LDFLAGS+=" -fprofile-arcs -lgcov" diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 6e80c33449a..0f12694289c 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -1041,5 +1041,21 @@ int main(int argc, char **argv) orchDaemon->start(heartBeatInterval); + /* + * On SIGTERM/SIGINT the signal handler sets gOrchShutdownRequested and + * start() returns. Do not fall through to `return 0;`: running ~OrchDaemon + * and its member destructors is unsafe here. FlexCounterManager destruction + * issues SAI calls (stopFlexCounterPolling -> set_switch_attribute) that + * round-trip through sairedis's ZMQ channel and park the main thread in + * zmq_poll while libzmq I/O threads are still alive; orchs torn down earlier + * in the reverse-order loop have already freed buffers those threads still + * reference, corrupting the heap. + * + * Instead, drain the async swss recorder so pending records flush, then + * _exit() to let the kernel reclaim the rest of the process without the + * destructor chain. + */ + exit_if_graceful_shutdown_requested(); + return 0; } diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index d9ab8f11870..f5af45aca86 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -1098,6 +1098,39 @@ void OrchDaemon::start(long heartBeatInterval) } } +#ifdef GCOV_ENABLED +extern "C" void __gcov_dump(void); +extern "C" void __gcov_reset(void); +#endif + +void exit_if_graceful_shutdown_requested(void (*exit_fn)(int)) +{ + if (gOrchShutdownRequested == 0) + { + return; + } + + SWSS_LOG_NOTICE("Exiting on graceful shutdown request (signal %d) without running destructors", gOrchShutdownRequested); + + /* + * Drain the async swss recorder before exiting: setAsync(false) stops the + * recorder worker only after any queued records have been written out. + */ + Recorder::Instance().swss.setAsync(false); + +#ifdef GCOV_ENABLED + /* + * _exit() skips libgcov's exit hook, so persist coverage data explicitly. + * Reset the counters afterwards so a unit-test caller passing a + * non-exiting exit_fn still dumps the remainder of its run at exit. + */ + __gcov_dump(); + __gcov_reset(); // LCOV_EXCL_LINE (resets its own arc counter, so it can never self-report) +#endif + + exit_fn(0); +} + /* * Try to perform orchagent state restore and dynamic states sync up if * warm start request is detected. diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 6fbee416a0f..9d3028e621b 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -1,6 +1,8 @@ #ifndef SWSS_ORCHDAEMON_H #define SWSS_ORCHDAEMON_H +#include + #include "dbconnector.h" #include "producerstatetable.h" #include "consumertable.h" @@ -156,4 +158,15 @@ class DpuOrchDaemon : public OrchDaemon DBConnector *m_dpu_appDb; DBConnector *m_dpu_appstateDb; }; + +/* + * If a graceful shutdown was requested (SIGTERM/SIGINT set + * gOrchShutdownRequested and OrchDaemon::start() returned), drain the async + * swss recorder so pending records are flushed and terminate the process via + * exit_fn without running the OrchDaemon destructor chain. No-op when no + * shutdown was requested. exit_fn is injectable for unit tests; production + * callers use the default _exit. + */ +void exit_if_graceful_shutdown_requested(void (*exit_fn)(int) = _exit); + #endif /* SWSS_ORCHDAEMON_H */ diff --git a/tests/mock_tests/orchdaemon_ut.cpp b/tests/mock_tests/orchdaemon_ut.cpp index faa86ab2799..12cc1dc6516 100644 --- a/tests/mock_tests/orchdaemon_ut.cpp +++ b/tests/mock_tests/orchdaemon_ut.cpp @@ -8,9 +8,13 @@ #include "mock_sai_switch.h" #include "saihelper.h" +#include + extern sai_switch_api_t* sai_switch_api; sai_switch_api_t test_sai_switch; +extern volatile sig_atomic_t gOrchShutdownRequested; + namespace orchdaemon_test { @@ -248,4 +252,53 @@ namespace orchdaemon_test orchd->disableRingBuffer(); } + static int gMockExitCallCount; + static int gMockExitStatus; + + static void mock_exit_fn(int status) + { + gMockExitCallCount++; + gMockExitStatus = status; + } + + class GracefulShutdownExitTest : public ::testing::Test + { + protected: + void SetUp() override + { + gMockExitCallCount = 0; + gMockExitStatus = -1; + gOrchShutdownRequested = 0; + } + + void TearDown() override + { + gOrchShutdownRequested = 0; + Recorder::Instance().swss.setAsync(false); + } + }; + + TEST_F(GracefulShutdownExitTest, NoShutdownRequestedDoesNotExit) + { + Recorder::Instance().swss.setAsync(true); + + exit_if_graceful_shutdown_requested(mock_exit_fn); + + EXPECT_EQ(gMockExitCallCount, 0); + // The recorder must be left untouched when no shutdown was requested. + EXPECT_TRUE(Recorder::Instance().swss.isAsyncEnabled()); + } + + TEST_F(GracefulShutdownExitTest, ShutdownRequestDrainsRecorderAndExits) + { + Recorder::Instance().swss.setAsync(true); + gOrchShutdownRequested = SIGTERM; + + exit_if_graceful_shutdown_requested(mock_exit_fn); + + EXPECT_EQ(gMockExitCallCount, 1); + EXPECT_EQ(gMockExitStatus, 0); + EXPECT_FALSE(Recorder::Instance().swss.isAsyncEnabled()); + } + }