Fix Entity-Manager Crash issue
When using multiple dbus-probe types, we were seeing:
Program received signal SIGBUS, Bus error.
0x00475c6c in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() ()
(gdb) bt
#0 0x00475c6c in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() ()
#1 0x00477820 in std::vector<std::shared_ptr<PerformProbe>, std::allocator<std::shared_ptr<PerformProbe> > >::clear() ()
#2 0x0046d594 in ?? ()
#3 0x0046e14c in ?? ()
#4 0x76f60bd0 in ?? () from /lib/libsystemd.so.0
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
The logic in this was quite bad, by moving the storage of
PerformProbe shared_ptrs into the captures, we don't need
to worry about calling clear ever, so we won't run into this
problem. This was reordered to fix the issue.
Tested: On system that frequently saw the crash, it went
away, all sensors still available.
Change-Id: Icacb8861466816df64b24efe940e5a732102345a
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/src/EntityManager.cpp b/src/EntityManager.cpp
index b177119..784ef54 100644
--- a/src/EntityManager.cpp
+++ b/src/EntityManager.cpp
@@ -129,41 +129,19 @@
// calls the mapper to find all exposed objects of an interface type
// and creates a vector<flat_map> that contains all the key value pairs
// getManagedObjects
-void findDbusObjects(std::shared_ptr<PerformProbe> probe,
+void findDbusObjects(std::vector<std::shared_ptr<PerformProbe>>& probeVector,
std::shared_ptr<sdbusplus::asio::connection> connection,
- std::string& interface)
+ boost::container::flat_set<std::string>& interfaces)
{
- // store reference to pending callbacks so we don't overwhelm services
- static boost::container::flat_map<
- std::string, std::vector<std::shared_ptr<PerformProbe>>>
- pendingProbes;
-
- if (DBUS_PROBE_OBJECTS[interface].size())
- {
- return;
- }
-
- // add shared_ptr to vector of Probes waiting for callback from a specific
- // interface to keep alive while waiting for response
- std::array<const char*, 1> objects = {interface.c_str()};
- std::vector<std::shared_ptr<PerformProbe>>& pending =
- pendingProbes[interface];
- auto iter = pending.emplace(pending.end(), probe);
- // only allow first call to run to not overwhelm processes
- if (iter != pending.begin())
- {
- return;
- }
-
// find all connections in the mapper that expose a specific type
connection->async_method_call(
- [connection, interface, probe](boost::system::error_code& ec,
- const GetSubTreeType& interfaceSubtree) {
+ [connection, interfaces,
+ probeVector](boost::system::error_code& ec,
+ const GetSubTreeType& interfaceSubtree) {
boost::container::flat_set<std::string> interfaceConnections;
if (ec)
{
- pendingProbes[interface].clear();
if (ec.value() == ENOENT)
{
return; // wasn't found by mapper
@@ -173,59 +151,57 @@
// if we can't communicate to the mapper something is very wrong
std::exit(EXIT_FAILURE);
}
- else
+
+ for (auto& object : interfaceSubtree)
{
- for (auto& object : interfaceSubtree)
+ for (auto& connPair : object.second)
{
- for (auto& connPair : object.second)
- {
- interfaceConnections.insert(connPair.first);
- }
+ interfaceConnections.insert(connPair.first);
}
}
+
if (interfaceConnections.empty())
{
- pendingProbes[interface].clear();
return;
}
// get managed objects for all interfaces
for (const auto& conn : interfaceConnections)
{
connection->async_method_call(
- [conn,
- interface](boost::system::error_code& errc,
- const ManagedObjectType& managedInterface) {
+ [conn, interfaces,
+ probeVector](boost::system::error_code& errc,
+ const ManagedObjectType& managedInterface) {
if (errc)
{
std::cerr
<< "error getting managed object for device "
<< conn << "\n";
- pendingProbes[interface].clear();
return;
}
for (auto& interfaceManagedObj : managedInterface)
{
- auto ifaceObjFind =
- interfaceManagedObj.second.find(interface);
- if (ifaceObjFind !=
- interfaceManagedObj.second.end())
+ // we could match multiple interfaces with one owner
+ for (auto& [interface, object] :
+ interfaceManagedObj.second)
{
- std::vector<boost::container::flat_map<
- std::string, BasicVariantType>>&
- dbusObject = DBUS_PROBE_OBJECTS[interface];
- dbusObject.emplace_back(ifaceObjFind->second);
+ auto ifaceObjFind = interfaces.find(interface);
+
+ if (ifaceObjFind != interfaces.end())
+ {
+ DBUS_PROBE_OBJECTS[interface].emplace_back(
+ object);
+ }
}
}
- pendingProbes[interface].clear();
},
- conn.c_str(), "/", "org.freedesktop.DBus.ObjectManager",
+ conn, "/", "org.freedesktop.DBus.ObjectManager",
"GetManagedObjects");
}
},
"xyz.openbmc_project.ObjectMapper",
"/xyz/openbmc_project/object_mapper",
"xyz.openbmc_project.ObjectMapper", "GetSubTree", "/", MAX_MAPPER_DEPTH,
- objects);
+ interfaces);
}
// probes dbus interface dictionary for a key with a value that matches a regex
bool probeDbus(const std::string& interface,
@@ -489,35 +465,6 @@
_callback(foundDevs);
}
}
- void run()
- {
- // parse out dbus probes by discarding other probe types
-
- for (std::string& probe : _probeCommand)
- {
- bool found = false;
- boost::container::flat_map<const char*, probe_type_codes,
- cmp_str>::const_iterator probeType;
- for (probeType = PROBE_TYPES.begin();
- probeType != PROBE_TYPES.end(); ++probeType)
- {
- if (probe.find(probeType->first) != std::string::npos)
- {
- found = true;
- break;
- }
- }
- if (found)
- {
- continue;
- }
- // syntax requires probe before first open brace
- auto findStart = probe.find("(");
- std::string interface = probe.substr(0, findStart);
-
- findDbusObjects(shared_from_this(), SYSTEM_BUS, interface);
- }
- }
std::vector<std::string> _probeCommand;
std::function<void(FoundDeviceT&)> _callback;
};
@@ -1228,6 +1175,9 @@
}
void run()
{
+ boost::container::flat_set<std::string> dbusProbeInterfaces;
+ std::vector<std::shared_ptr<PerformProbe>> dbusProbePointers;
+
for (auto it = _configurations.begin(); it != _configurations.end();)
{
auto findProbe = it->find("Probe");
@@ -1272,7 +1222,7 @@
// store reference to this to children to makes sure we don't get
// destroyed too early
auto thisRef = shared_from_this();
- auto p = std::make_shared<
+ auto probePointer = std::make_shared<
PerformProbe>(probeCommand, [&, recordPtr, probeName, thisRef](
FoundDeviceT& foundDevices) {
_passed = true;
@@ -1421,9 +1371,39 @@
foundDeviceIdx++;
}
});
- p->run();
+
+ // parse out dbus probes by discarding other probe types, store in a
+ // map
+ for (const std::string& probe : probeCommand)
+ {
+ bool found = false;
+ boost::container::flat_map<const char*, probe_type_codes,
+ cmp_str>::const_iterator probeType;
+ for (probeType = PROBE_TYPES.begin();
+ probeType != PROBE_TYPES.end(); ++probeType)
+ {
+ if (probe.find(probeType->first) != std::string::npos)
+ {
+ found = true;
+ break;
+ }
+ }
+ if (found)
+ {
+ continue;
+ }
+ // syntax requires probe before first open brace
+ auto findStart = probe.find("(");
+ std::string interface = probe.substr(0, findStart);
+ dbusProbeInterfaces.emplace(interface);
+ dbusProbePointers.emplace_back(probePointer);
+ }
it++;
}
+
+ // probe vector stores a shared_ptr to each PerformProbe that cares
+ // about a dbus interface
+ findDbusObjects(dbusProbePointers, SYSTEM_BUS, dbusProbeInterfaces);
}
~PerformScan()
@@ -1444,7 +1424,6 @@
nlohmann::json& _missingConfigurations;
std::list<nlohmann::json> _configurations;
std::function<void(void)> _callback;
- std::vector<std::shared_ptr<PerformProbe>> _probes;
bool _passed = false;
bool powerWasOn = isPowerOn();
};
@@ -1655,8 +1634,8 @@
for (const auto& objectMap : DBUS_PROBE_OBJECTS)
{
- auto findObject = watchedObjects.find(objectMap.first);
- if (findObject != watchedObjects.end())
+ auto [_, inserted] = watchedObjects.insert(objectMap.first);
+ if (!inserted)
{
continue;
}