Fix fpmsyncd crash during pfcwd/test_pfcwd_warm_reboot.py worm reboot issue (#3746)

Fix fpmsyncd crash during warm reboot issue

Why I did it
The SONiC management test case pfcwd/test_pfcwd_warm_reboot.py failed due to a crash in fpmsyncd during the warm reboot process.

The crash occurred when fpmsyncd received the warm reboot signal and invoked the WarmStartHelper::checkAndStart() method. At that moment, the m_syncTable inside WarmStartHelper pointed to an invalid route table, which triggered a segmentation fault (SIGSEGV).

Root Cause:
In the constructor of RouteSync, the initialization of m_warmStartHelper depends on m_routeTable being initialized first. However, in the class declaration, m_warmStartHelper is listed before m_routeTable.

In C++, member variables are initialized in the order they are declared in the class—not in the order specified in the constructor’s initializer list. As a result, m_warmStartHelper is initialized before m_routeTable, leading to m_warmStartHelper referencing an uninitialized route table.

Author: liuh-80

fpmsyncd/fpmsyncd.cpp

@@ -141,11 +141,11 @@ int main(int argc, char **argv)
             }
 
             /* If warm-restart feature is enabled, execute 'restoration' logic */
-            bool warmStartEnabled = sync.m_warmStartHelper.checkAndStart();
+            bool warmStartEnabled = sync.getWarmStartHelper().checkAndStart();
             if (warmStartEnabled)
             {
                 /* Obtain warm-restart timer defined for routing application */
-                time_t warmRestartIval = sync.m_warmStartHelper.getRestartTimer();
+                time_t warmRestartIval = sync.getWarmStartHelper().getRestartTimer();
                 if (!warmRestartIval)
                 {
                     warmStartTimer.setInterval(timespec{DEFAULT_ROUTING_RESTART_INTERVAL, 0});
@@ -156,7 +156,7 @@ int main(int argc, char **argv)
                 }
 
                 /* Execute restoration instruction and kick off warm-restart timer */
-                if (sync.m_warmStartHelper.runRestoration())
+                if (sync.getWarmStartHelper().runRestoration())
                 {
                     warmStartTimer.start();
                     s.addSelectable(&warmStartTimer);
@@ -171,7 +171,7 @@ int main(int argc, char **argv)
             }
             else
             {
-                sync.m_warmStartHelper.setState(WarmStart::WSDISABLED);
+                sync.getWarmStartHelper().setState(WarmStart::WSDISABLED);
             }
 
             gSelectTimeout = INFINITE;
@@ -209,7 +209,7 @@ int main(int argc, char **argv)
                 }
                 else if (temps == &eoiuCheckTimer)
                 {
-                    if (sync.m_warmStartHelper.inProgress())
+                    if (sync.getWarmStartHelper().inProgress())
                     {
                         if (eoiuFlagsSet(bgpStateTable))
                         {
@@ -308,7 +308,7 @@ int main(int argc, char **argv)
                         sync.onRouteResponse(key, fieldValues);
                     }
                 }
-                else if (!warmStartEnabled || sync.m_warmStartHelper.isReconciled())
+                else if (!warmStartEnabled || sync.getWarmStartHelper().isReconciled())
                 {
                     flushPipeline(pipeline);
                 }

fpmsyncd/routesync.h

@@ -81,7 +81,10 @@ class RouteSync : public NetMsg
         m_fpmInterface = nullptr;
     }
 
-    WarmStartHelper  m_warmStartHelper;
+    WarmStartHelper& getWarmStartHelper()
+    {
+        return m_warmStartHelper;
+    }
 
 private:
     /* ZMQ client */
@@ -94,6 +97,8 @@ class RouteSync : public NetMsg
     ProducerStateTable  m_vnet_routeTable;
     /* vnet vxlan tunnel table */  
     ProducerStateTable  m_vnet_tunnelTable;
+    /* Warm start helper */
+    WarmStartHelper m_warmStartHelper;
     /* srv6 mySid table */
     ProducerStateTable m_srv6MySidTable; 
     /* srv6 sid list table */

tests/mock_tests/fpmsyncd/test_routesync.cpp

@@ -1026,19 +1026,6 @@ TEST_F(FpmSyncdResponseTest, TestGetNextHopWt)
     EXPECT_EQ(m_mockRouteSync.getNextHopWt(test_route.get()), "1,1");
 }
 
-/**
- * Unit tests for warm restart route handling functionality
- *
- */
-
-class TestableRouteSync : public RouteSync
-{
-public:
-    TestableRouteSync(RedisPipeline *pipeline) : RouteSync(pipeline) {}
-
-    WarmStartHelper& getWarmStartHelper() { return m_warmStartHelper; }
-};
-
 class WarmRestartRouteSyncTest : public ::testing::Test
 {
 public:
@@ -1057,7 +1044,7 @@ class WarmRestartRouteSyncTest : public ::testing::Test
 
     shared_ptr<swss::DBConnector> m_db = make_shared<swss::DBConnector>("APPL_DB", 0);
     shared_ptr<RedisPipeline> m_pipeline = make_shared<RedisPipeline>(m_db.get());
-    TestableRouteSync m_testRouteSync{m_pipeline.get()};
+    RouteSync m_testRouteSync{m_pipeline.get()};
 };
 
 TEST_F(WarmRestartRouteSyncTest, TestRouteMessageHandlingWarmRestartNotInProgress)

tests/test_warm_reboot.py

@@ -2475,6 +2475,23 @@ def test_TunnelMgrdWarmRestart(self, dvs):
         assert nadd == 0
         assert ndel == 0
 
+    def test_FpmsyncdWarmRestart(self, dvs):
+        # This test aims to improve code coverage in fpmsyncd.
+        warm_restart_set(dvs, "system", "true")
+        warm_restart_set(dvs, "bgp", "true")
+
+        # set restore count
+        db = swsscommon.DBConnector(6, dvs.redis_sock, 0)
+        tbl = swsscommon.Table(db, "WARM_RESTART_ENABLE_TABLE")
+        fvs = swsscommon.FieldValuePairs([("restore_count", "0")])
+        tbl.set("bgp", fvs)
+
+        # Stop fpmsyncd
+        dvs.stop_fpmsyncd()
+
+        # Start fpmsyncd
+        dvs.start_fpmsyncd()
+
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying
 def test_nonflaky_dummy():