commit ad11f7df3ead2b42d1688fe2cd5b2531f2912c73
author Vikash Chandola <vikash.chandola@intel.com> Fri Apr 28 17:23:52 2023 +0530
committer Vikash Chandola <vikash.chandola@intel.com> Wed May 03 15:16:23 2023 +0530
tree e7946a0dacc3df8644c4d22764eaeb195fd40091
parent a244198f9526c2c0853619cdb0d3ebc072e14bff

remove dbus interfaces on Backplane destruction

activationIface(dbus_interface) attribute of Backplane(struct) was not
getting removed from dbus on destruction of Backplane object. Later as
Backplane object gets reconstructed for same Backplane add_interface API
throws exception since interface is still there.
Remove activation dbus_interface on destruction of Backplane object.

Led associated with a Backplane were not getting destructed on
destruction of Backplane object. This was because Led is created as
shared_ptr and Led object itself keeps one reference causing use_count
to never go to zero.
Use weak reference in Led object. This causes Led object destructor to
get called on destruction of Backplane.

Change-Id: I3499b3617810ca39c99ecd1a8de7f56c93a2a586
Signed-off-by: Vikash Chandola <vikash.chandola@intel.com>

hsbp-manager/src/hsbp_manager.cpp

diff --git a/hsbp-manager/src/hsbp_manager.cpp b/hsbp-manager/src/hsbp_manager.cpp
index 51f733d..b3b1ff5 100644
--- a/hsbp-manager/src/hsbp_manager.cpp
+++ b/hsbp-manager/src/hsbp_manager.cpp
@@ -618,10 +618,14 @@
     // work
     void createInterface(void)
     {
-        std::shared_ptr<Led> self = shared_from_this();
-
         ledInterface->register_property(
-            ledGroup::asserted, false, [self](const bool req, bool& val) {
+            ledGroup::asserted, false,
+            [weakRef{weak_from_this()}](const bool req, bool& val) {
+                auto self = weakRef.lock();
+                if (!self)
+                {
+                    return 0;
+                }
                 if (req == val)
                 {
                     return 1;
@@ -848,7 +852,7 @@
     void populateAsset(const std::string& rootPath, const std::string& busname)
     {
         conn->async_method_call(
-            [assetIface{assetInterface}, hsbpIface{hsbpItemIface}](
+            [assetIface{assetInterface}](
                 const boost::system::error_code ec,
                 const boost::container::flat_map<
                     std::string, std::variant<std::string>>& values) mutable {
@@ -859,8 +863,6 @@
 
                     return;
                 }
-                assetIface = objServer.add_interface(
-                    hsbpIface->get_object_path(), assetTag);
                 for (const auto& [key, value] : values)
                 {
                     const std::string* ptr = std::get_if<std::string>(&value);
@@ -923,6 +925,9 @@
             "xyz.openbmc_project.Inventory.Item.StorageController");
         storageInterface->initialize();
 
+        assetInterface =
+            objServer.add_interface(hsbpItemIface->get_object_path(), assetTag);
+
         versionIface =
             objServer.add_interface("/xyz/openbmc_project/software/" + dbusName,
                                     "xyz.openbmc_project.Software.Version");
@@ -935,10 +940,9 @@
                 "xyz.openbmc_project.Software.Version.VersionPurpose.HSBP"));
         versionIface->initialize();
 
-        auto activationIface =
+        activationIface =
             objServer.add_interface("/xyz/openbmc_project/software/" + dbusName,
                                     "xyz.openbmc_project.Software.Activation");
-
         activationIface->register_property(
             "Activation",
             std::string(
@@ -947,7 +951,6 @@
             "RequestedActivation",
             std::string("xyz.openbmc_project.Software.Activation."
                         "RequestedActivations.None"));
-
         activationIface->initialize();
 
         getPresence(presence);
@@ -1251,6 +1254,7 @@
         objServer.remove_interface(versionIface);
         objServer.remove_interface(storageInterface);
         objServer.remove_interface(assetInterface);
+        objServer.remove_interface(activationIface);
         if (file >= 0)
         {
             close(file);
@@ -1282,7 +1286,7 @@
     std::shared_ptr<sdbusplus::asio::dbus_interface> versionIface;
     std::shared_ptr<sdbusplus::asio::dbus_interface> storageInterface;
     std::shared_ptr<sdbusplus::asio::dbus_interface> assetInterface;
-
+    std::shared_ptr<sdbusplus::asio::dbus_interface> activationIface;
     std::list<Drive> drives;
     std::vector<std::shared_ptr<Led>> leds;
     std::shared_ptr<boost::container::flat_set<Mux>> muxes;