Utils: Avoid createInventoryAssoc for destroyed sensors
PSU sensors call `createInventoryAssoc` in their constructors to
initialize Association interfaces. This function involves an async call
to `GetSubTree` to get the chassis paths on D-Bus for its need.
Therefore, the Association interface of type
`std::shared_ptr<sdbusplus::asio::dbus_interface>` will only be
initialized after `GetSubTree` returns, not before the sensor's
constructor finishes.
`createInventoryAssoc` passes a shared pointer of type
`sdbusplus::asio::dbus_interface` to the lambda callback of GetSubTree.
Therefore, it will still retain the stored object of this shared pointer
until the callback finishes. This results in the initialization of the
Association interface to D-Bus even after the sensor was destroyed. This
interface will only be removed when this callback finishes.
Although the sensor's destructor calls
`objServer.remove_interface(association)`, this only removes the
interface from the object server's interface list [1], not the interface
itself, which was created with `std::make_shared` [2] by a
`objServer.add_interface(...)` call in the sensor's constructor.
This commit enlists std::weak_ptr in this process, so nothing will share
the ownership of this interface with the sensor class outside its
scope.
Tested:
Let a PSU sensor object be destroyed before the Association interface is
registered for that sensor path on D-Bus. After that, there shall be no
pair of interfacesAdded and interfacesRemoved signals of the Association
interface on the sensor path on D-Bus.
[1]: https://github.com/openbmc/sdbusplus/blob/f2cc743735e1cfea65e765619b1d8334b22932e6/include/sdbusplus/asio/object_server.hpp#L900
[2]: https://github.com/openbmc/sdbusplus/blob/f2cc743735e1cfea65e765619b1d8334b22932e6/include/sdbusplus/asio/object_server.hpp#L858
Signed-off-by: Chau Ly <chaul@amperecomputing.com>
Change-Id: I50bd04b2c6d61c39f6890cf63b471d6c2ba9aa59
diff --git a/src/Utils.cpp b/src/Utils.cpp
index 202e394..b25bef9 100644
--- a/src/Utils.cpp
+++ b/src/Utils.cpp
@@ -619,18 +619,21 @@
}
void setInventoryAssociation(
- const std::shared_ptr<sdbusplus::asio::dbus_interface>& association,
+ const std::weak_ptr<sdbusplus::asio::dbus_interface>& weakRef,
const std::string& inventoryPath, const std::string& chassisPath)
{
- if (association)
+ auto association = weakRef.lock();
+ if (!association)
{
- std::vector<Association> associations;
- associations.emplace_back("inventory", "sensors", inventoryPath);
- associations.emplace_back("chassis", "all_sensors", chassisPath);
-
- association->register_property("Associations", associations);
- association->initialize();
+ return;
}
+
+ std::vector<Association> associations;
+ associations.emplace_back("inventory", "sensors", inventoryPath);
+ associations.emplace_back("chassis", "all_sensors", chassisPath);
+
+ association->register_property("Associations", associations);
+ association->initialize();
}
std::optional<std::string> findContainingChassis(std::string_view configParent,
@@ -678,9 +681,10 @@
"xyz.openbmc_project.Inventory.Item.Chassis",
});
+ std::weak_ptr<sdbusplus::asio::dbus_interface> weakRef = association;
conn->async_method_call(
- [association, path](const boost::system::error_code ec,
- const GetSubTreeType& subtree) {
+ [weakRef, path](const boost::system::error_code ec,
+ const GetSubTreeType& subtree) {
// The parent of the config is always the inventory object, and may
// be the associated chassis. If the parent is not itself a chassis
// or board, the sensor is associated with the system chassis.
@@ -690,11 +694,11 @@
{
// In case of error, set the default associations and
// initialize the association Interface.
- setInventoryAssociation(association, parent, parent);
+ setInventoryAssociation(weakRef, parent, parent);
return;
}
setInventoryAssociation(
- association, parent,
+ weakRef, parent,
findContainingChassis(parent, subtree).value_or(parent));
},
mapper::busName, mapper::path, mapper::interface, "GetSubTree",