Eliminate swampd core dump after D-Bus updates sensors
The swamp daemon intializes a list of sensors and uses those to
periodically scan the state associated devices. Reading the sensors is
done with an async timer, that runs code to re-arm an async timer.
There is also a D-Bus update cycle that is independent of the async
timer reading the sensors. When the D-Bus updates the number of
sensors in the system a new list must be created. In order to create
the new list the timers using the old list must be stopped. Only after
those timers have stopped may a new list be generated, and a new set of
timers started.
The two processes are unware of each other. To safely perform the
change the pointers to the list of zones and timers must be kept alive
until all timer actions complete. Only after all references to the
pointers have been release may the new state be built, and new timers
started.
Prior to this change swampd would throw a SYSSEGV fault due to an
attempt to use a pointer that was no longer active.
Tested:
Issued a "reset -w" (Warm Reset command) from the EFI shell.
Waited for the system to reboot, and enter EFI
Checked for a core file in /var/lib/systemd/coredump
Repeated step 1 if coredump file was not present.
Completed 2900+ passes successfully when ealier code failed at less
than 800 passes.
Change-Id: Iff4607510db579c36dc34d6f76e6eb2f0250a03a
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
diff --git a/main.cpp b/main.cpp
index 712d2cf..ec9b576 100644
--- a/main.cpp
+++ b/main.cpp
@@ -79,10 +79,15 @@
void restartControlLoops()
{
static SensorManager mgmr;
- static std::unordered_map<int64_t, std::unique_ptr<ZoneInterface>> zones;
- static std::list<boost::asio::steady_timer> timers;
+ static std::unordered_map<int64_t, std::shared_ptr<ZoneInterface>> zones;
+ static std::vector<std::shared_ptr<boost::asio::steady_timer>> timers;
+ for (const auto timer : timers)
+ {
+ timer->cancel();
+ }
timers.clear();
+ zones.clear();
const std::string& path =
(configPath.length() > 0) ? configPath : jsonConfigurationPath;
@@ -126,9 +131,10 @@
for (const auto& i : zones)
{
- auto& timer = timers.emplace_back(io);
+ std::shared_ptr<boost::asio::steady_timer> timer = timers.emplace_back(
+ std::make_shared<boost::asio::steady_timer>(io));
std::cerr << "pushing zone " << i.first << "\n";
- pidControlLoop(i.second.get(), timer);
+ pidControlLoop(i.second, timer);
}
}
diff --git a/pid/builder.cpp b/pid/builder.cpp
index 9f27ed4..706585a 100644
--- a/pid/builder.cpp
+++ b/pid/builder.cpp
@@ -44,12 +44,12 @@
return std::string(objectPath) + std::to_string(zone);
}
-std::unordered_map<int64_t, std::unique_ptr<ZoneInterface>>
+std::unordered_map<int64_t, std::shared_ptr<ZoneInterface>>
buildZones(const std::map<int64_t, conf::PIDConf>& zonePids,
std::map<int64_t, struct conf::ZoneConfig>& zoneConfigs,
SensorManager& mgr, sdbusplus::bus::bus& modeControlBus)
{
- std::unordered_map<int64_t, std::unique_ptr<ZoneInterface>> zones;
+ std::unordered_map<int64_t, std::shared_ptr<ZoneInterface>> zones;
for (const auto& [zoneId, pidConfig] : zonePids)
{
@@ -67,7 +67,7 @@
throw std::runtime_error(err);
}
- auto zone = std::make_unique<DbusPidZone>(
+ auto zone = std::make_shared<DbusPidZone>(
zoneId, zoneConf->second.minThermalOutput,
zoneConf->second.failsafePercent, mgr, modeControlBus,
getControlPath(zoneId).c_str(), deferSignals);
diff --git a/pid/builder.hpp b/pid/builder.hpp
index ff2f682..c07de74 100644
--- a/pid/builder.hpp
+++ b/pid/builder.hpp
@@ -12,7 +12,7 @@
namespace pid_control
{
-std::unordered_map<int64_t, std::unique_ptr<ZoneInterface>>
+std::unordered_map<int64_t, std::shared_ptr<ZoneInterface>>
buildZones(const std::map<int64_t, conf::PIDConf>& zonePids,
std::map<int64_t, struct conf::ZoneConfig>& zoneConfigs,
SensorManager& mgr, sdbusplus::bus::bus& modeControlBus);
diff --git a/pid/pidloop.cpp b/pid/pidloop.cpp
index 1cdf019..711ba2f 100644
--- a/pid/pidloop.cpp
+++ b/pid/pidloop.cpp
@@ -33,7 +33,7 @@
namespace pid_control
{
-static void processThermals(ZoneInterface* zone)
+static void processThermals(std::shared_ptr<ZoneInterface> zone)
{
// Get the latest margins.
zone->updateSensors();
@@ -46,7 +46,8 @@
zone->determineMaxSetPointRequest();
}
-void pidControlLoop(ZoneInterface* zone, boost::asio::steady_timer& timer,
+void pidControlLoop(std::shared_ptr<ZoneInterface> zone,
+ std::shared_ptr<boost::asio::steady_timer> timer,
bool first, int ms100cnt)
{
if (first)
@@ -60,9 +61,9 @@
processThermals(zone);
}
- timer.expires_after(std::chrono::milliseconds(100));
- timer.async_wait(
- [zone, &timer, ms100cnt](const boost::system::error_code& ec) mutable {
+ timer->expires_after(std::chrono::milliseconds(100));
+ timer->async_wait(
+ [zone, timer, ms100cnt](const boost::system::error_code& ec) mutable {
if (ec == boost::asio::error::operation_aborted)
{
return; // timer being canceled, stop loop
diff --git a/pid/pidloop.hpp b/pid/pidloop.hpp
index 0b8690b..b7d1b34 100644
--- a/pid/pidloop.hpp
+++ b/pid/pidloop.hpp
@@ -17,7 +17,8 @@
* @param[in] first - boolean to denote if initialization needs to be run.
* @param[in] ms100cnt - loop timer counter.
*/
-void pidControlLoop(ZoneInterface* zone, boost::asio::steady_timer& timer,
+void pidControlLoop(std::shared_ptr<ZoneInterface> zone,
+ std::shared_ptr<boost::asio::steady_timer> timer,
bool first = true, int ms100cnt = 0);
} // namespace pid_control