hwmontempsensor: Fix crash on eventHandler signal callback
Previously the device-management logic didn't handle the eventHandler()
case quite right, leading to crashes like the following when
eventHandler()'s call to createSensors() ends up spuriously trying to
re-create sensor devices:
hwmontempsensor[455]: terminate called after throwing an instance of 'boost::wrapexcept<boost::system::system_error>'
hwmontempsensor[455]: what(): open: No such file or directory [system:2 at /usr/include/boost/asio/detail/impl/io_uring_file_service.ipp:61:5 in function 'boost::system::error_code boost::asio::detail::io_uring_file_service::open(implementation_type&, const char*, boost::asio::file_base::flags, boost::system::error_code&)']
To fix this we have instantiateDevices() augment the map it returns to
include all sensor devices (not just newly created ones) and adding a
bool to indicate whether each one was newly instantiated or already
existed. This allows createSensors() to reuse existing I2CDevices when
called by eventHandler() instead of needlessly trying to destroy &
re-create them.
Tested: On romed8hm3 ran hwmontempsensor by hand (to keep systemd from
restarting it) and restarted E-M to trigger an eventHandler() call,
verified that hwmontempsensor didn't crash and kept all sensors
instantiated. Also tested powering the host on and off to verify that
host-power-domain sensors were still torn down and re-created as
appropriate.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Change-Id: I576ff6238f45d8e2885790a624fb5c838648c897
diff --git a/include/HwmonTempSensor.hpp b/include/HwmonTempSensor.hpp
index 276f37d..40be3bb 100644
--- a/include/HwmonTempSensor.hpp
+++ b/include/HwmonTempSensor.hpp
@@ -40,6 +40,11 @@
void deactivate(void);
bool isActive(void);
+ std::shared_ptr<I2CDevice> getI2CDevice() const
+ {
+ return i2cDevice;
+ }
+
private:
// Ordering is important here; readBuf is first so that it's not destroyed
// while async operations from other member fields might still be using it.
diff --git a/src/HwmonTempMain.cpp b/src/HwmonTempMain.cpp
index e1b0d62..27b28d5 100644
--- a/src/HwmonTempMain.cpp
+++ b/src/HwmonTempMain.cpp
@@ -240,14 +240,18 @@
return configMap;
}
-boost::container::flat_map<std::string, std::shared_ptr<I2CDevice>>
+// returns a {path: <I2CDevice, is_new>} map. is_new indicates if the I2CDevice
+// is newly instantiated by this call (true) or was already there (false).
+boost::container::flat_map<std::string,
+ std::pair<std::shared_ptr<I2CDevice>, bool>>
instantiateDevices(
const ManagedObjectType& sensorConfigs,
const boost::container::flat_map<
std::string, std::shared_ptr<HwmonTempSensor>>& sensors)
{
- boost::container::flat_map<std::string, std::shared_ptr<I2CDevice>>
- newSensors;
+ boost::container::flat_map<std::string,
+ std::pair<std::shared_ptr<I2CDevice>, bool>>
+ devices;
for (const auto& [path, sensor] : sensorConfigs)
{
for (const auto& [name, cfg] : sensor)
@@ -270,6 +274,9 @@
if (findSensor != sensors.end() && findSensor->second != nullptr &&
findSensor->second->isActive())
{
+ devices.emplace(
+ path.str,
+ std::make_pair(findSensor->second->getI2CDevice(), false));
continue;
}
@@ -294,8 +301,10 @@
try
{
- newSensors.emplace(path.str,
- std::make_shared<I2CDevice>(*params));
+ devices.emplace(
+ path.str,
+ std::make_pair(std::make_shared<I2CDevice>(*params),
+ true));
}
catch (std::runtime_error&)
{
@@ -306,7 +315,7 @@
}
}
}
- return newSensors;
+ return devices;
}
void createSensors(
@@ -326,8 +335,7 @@
SensorConfigMap configMap = buildSensorConfigMap(sensorConfigurations);
- boost::container::flat_map<std::string, std::shared_ptr<I2CDevice>>
- newSensors = instantiateDevices(sensorConfigurations, sensors);
+ auto devices = instantiateDevices(sensorConfigurations, sensors);
// IIO _raw devices look like this on sysfs:
// /sys/bus/iio/devices/iio:device0/in_temp_raw
@@ -397,16 +405,23 @@
}
const std::string& interfacePath = findSensorCfg->second.sensorPath;
- auto findI2CDev = newSensors.find(interfacePath);
- if (activateOnly && findI2CDev == newSensors.end())
+ auto findI2CDev = devices.find(interfacePath);
+
+ // If we're only looking to activate newly-instantiated i2c
+ // devices and this sensor's underlying device either (a) wasn't
+ // found (e.g. because it's a static device and not a dynamic one
+ // whose lifetime we're managing) or (b) was already there before
+ // this call, there's nothing more to do here.
+ if (activateOnly &&
+ (findI2CDev == devices.end() || !findI2CDev->second.second))
{
continue;
}
std::shared_ptr<I2CDevice> i2cDev;
- if (findI2CDev != newSensors.end())
+ if (findI2CDev != devices.end())
{
- i2cDev = findI2CDev->second;
+ i2cDev = findI2CDev->second.first;
}
const SensorData& sensorData = findSensorCfg->second.sensorData;