cpuSensor:Check object validity in async callback
Sometimes the cpusensor crash was observed. The coredump was pointing
to segmentation fault while executing callback function inside
setupRead().
Similar issue was observed in adcsensor.
Reference to the fix: 1afda6bbb6db0e266795af3229b962c32775b928
The timer cancel() in destructor cannot cancel the expired callback
handlers. In such cases if the callback handler is executed, the
object is deleted already.
Use weak_ptr to check if the object is still valid before using these
member variables/functions.
The buffer used by async_read_util must guarantee that they remain
valid until the handler is called.
Tested: Stress tested by restarting the service. Service restart will
force re-creating the CPUSensor objects.
`count=0; \
while true; \
do systemctl restart xyz.openbmc_project.cpusensor.service; \
count=$((count + 1)); \
echo $count; \
sleep 40; \
done `
Signed-off-by: gokulsanker <gokul.sanker.v.g@intel.com>
Signed-off-by: Arun P. Mohanan <arun.p.m@linux.intel.com>
Change-Id: I7410feb0e17c5f6d555cd042f5e5b327de1910c5
diff --git a/src/CPUSensor.cpp b/src/CPUSensor.cpp
index e955fef..cd1038f 100644
--- a/src/CPUSensor.cpp
+++ b/src/CPUSensor.cpp
@@ -42,7 +42,8 @@
bool show, double dtsOffset) :
Sensor(escapeName(sensorName), std::move(thresholdsIn), sensorConfiguration,
objectType, false, false, 0, 0, conn, PowerState::on),
- objServer(objectServer), inputDev(io), waitTimer(io), path(path),
+ std::enable_shared_from_this<CPUSensor>(), objServer(objectServer),
+ inputDev(io), waitTimer(io), path(path),
privTcontrol(std::numeric_limits<double>::quiet_NaN()),
dtsOffset(dtsOffset), show(show), pollTime(CPUSensor::sensorPollMs),
minMaxReadCounter(0)
@@ -95,7 +96,6 @@
// call setup always as not all sensors call setInitialProperties
setupPowerMatch(conn);
- setupRead();
}
CPUSensor::~CPUSensor()
@@ -116,6 +116,8 @@
void CPUSensor::setupRead(void)
{
+ std::weak_ptr<CPUSensor> weakRef = weak_from_this();
+
if (readingStateGood())
{
inputDev.close();
@@ -126,8 +128,15 @@
boost::asio::async_read_until(
inputDev, readBuf, '\n',
- [&](const boost::system::error_code& ec,
- std::size_t /*bytes_transfered*/) { handleResponse(ec); });
+ [weakRef](const boost::system::error_code& ec,
+ std::size_t /*bytes_transfered*/) {
+ std::shared_ptr<CPUSensor> self = weakRef.lock();
+ if (!self)
+ {
+ return;
+ }
+ self->handleResponse(ec);
+ });
}
else
{
@@ -141,12 +150,17 @@
markAvailable(false);
}
waitTimer.expires_from_now(boost::posix_time::milliseconds(pollTime));
- waitTimer.async_wait([&](const boost::system::error_code& ec) {
+ waitTimer.async_wait([weakRef](const boost::system::error_code& ec) {
if (ec == boost::asio::error::operation_aborted)
{
return; // we're being canceled
}
- setupRead();
+ std::shared_ptr<CPUSensor> self = weakRef.lock();
+ if (!self)
+ {
+ return;
+ }
+ self->setupRead();
});
}