[action] [PR:4699] [orchagent]: _exit() on graceful shutdown to avoid destructor-chain memory corruption

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"

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;
 }

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.

orchagent/orchdaemon.h

@@ -1,6 +1,8 @@
 #ifndef SWSS_ORCHDAEMON_H
 #define SWSS_ORCHDAEMON_H
 
+#include <unistd.h>
+
 #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 */

tests/mock_tests/orchdaemon_ut.cpp

@@ -8,9 +8,13 @@
 #include "mock_sai_switch.h"
 #include "saihelper.h"
 
+#include <csignal>
+
 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());
+    }
+
 }