commit afac94d1ffa271c9efb611a40f73a3e238164a9e
author George Liu <liuxiwei@ieisystem.com> Thu Oct 24 11:03:07 2024 +0800
committer George Liu <liuxiwei@ieisystem.com> Thu Oct 24 16:52:58 2024 +0800
tree 87f0cf2bfce4e462078a22e564b3342a30e44e98
parent 8425624a9046f5a853e8596cc74441e622028494

Fix the coredump problem caused by lock acquisition failure

As net-ipmid depends on ipmid, stress testing found that frequent
start/stop of net-ipmid will cause coredump due to lock acquisition
failure in ipmid's inithannelPersistData.
Since net-ipmid depends on ipmid, stress testing found that if
net-ipmid is started/stopped frequently, it will exit abnormally due
to failure to obtain the lock in the inithannelPersistData method of
ipmid.
```
netipmid-eth1[893]: terminate called after throwing an instance of ‘boost::interprocess::lock_exception’
netipmid-eth1[893]:     what():     boost::interprocess::lock_exception
```

Because in the main method of net-ipmid, the ipmi::ipmiChannelInit
method is called first, and then the startEventLoop method is called
to process the signal. This will cause a bug:
1. After net-ipmid@eth0 starts, ipmi::ipmiChannelInit will be
   executed to initialize and construct ChannelConfig.
2. The initChannelPersistData method will be executed in the
   construction method to obtain the lock and load the configuration.
3. If the stop method of net-ipmid is executed before the
   construction method is completed, the held lock will not be
   released correctly
4. If start is executed immediately at this time, the ipmid process
   will coredump because the current lock is undefined.

This submission moves the signal processing to before the
ipmi::ipmiChannelInit method, which will ensure that if net-ipmid
starts and captures abnormal operations, it will stop io first, so
that the held lock will be released correctly.

Tested:
Using this patch, no ipmid coredump occurs in the stress test.

Signed-off-by: George Liu <liuxiwei@ieisystem.com>
Change-Id: I29694405679330c083fa35d04443fb2e31bfca3b

main.cpp

diff --git a/main.cpp b/main.cpp
index 4702d62..a1427ca 100644
--- a/main.cpp
+++ b/main.cpp
@@ -91,6 +91,9 @@
     }
     sdbusp = std::make_shared<sdbusplus::asio::connection>(*io, bus);
 
+    auto& loop = eventloop::EventLoop::get();
+    loop.setupSignal();
+
     ipmi::ipmiChannelInit();
     if (channel.size())
     {
@@ -110,7 +113,6 @@
     // Register the phosphor-net-ipmid SOL commands
     sol::command::registerCommands();
 
-    auto& loop = eventloop::EventLoop::get();
     if (loop.setupSocket(sdbusp, channel))
     {
         return EXIT_FAILURE;

sd_event_loop.cpp

diff --git a/sd_event_loop.cpp b/sd_event_loop.cpp
index c7283e0..88fdf2e 100644
--- a/sd_event_loop.cpp
+++ b/sd_event_loop.cpp
@@ -239,7 +239,15 @@
 
 int EventLoop::startEventLoop()
 {
-    // set up boost::asio signal handling
+    startRmcpReceive();
+
+    io->run();
+
+    return EXIT_SUCCESS;
+}
+
+void EventLoop::setupSignal()
+{
     boost::asio::signal_set signals(*io, SIGINT, SIGTERM);
     signals.async_wait([this](const boost::system::error_code& /* error */,
                               int /* signalNumber */) {
@@ -247,12 +255,6 @@
         udpSocket->close();
         io->stop();
     });
-
-    startRmcpReceive();
-
-    io->run();
-
-    return EXIT_SUCCESS;
 }
 
 } // namespace eventloop

sd_event_loop.hpp

diff --git a/sd_event_loop.hpp b/sd_event_loop.hpp
index f580b32..b40782c 100644
--- a/sd_event_loop.hpp
+++ b/sd_event_loop.hpp
@@ -87,6 +87,9 @@
                     std::string iface,
                     uint16_t reqPort = ipmi::rmcpp::defaultPort);
 
+    /** @brief set up boost::asio signal handling */
+    void setupSignal();
+
   private:
     /** @brief async handler for incoming udp packets */
     void handleRmcpPacket();