adcsensor: Check if the object is still valid in the async callback functions
Sometimes adcsesnor will crash during stress test.
The root cause is that the object is destroyed already
when the async callback functions are executed.
The timer cancel() cannot cancel the expired callback
handlers. When the callback handler is executed,
the object is deleted already.
The buffer used by async_read_util must guarantee
that they remain valid until the handler is called.
Using the weak_ptr to check if the object is still valid
before using these member variables/functions.
Tested:
No adcsensor crash with stress test,
Change the threshold of ADC sensor 0x51 P3VBAT:
while [ true ]; do; \
ipmitool raw 4 0x26 0x51 0x1b 0x20 0x1b 0 0x2a 0x2a 0;\
ipmitool raw 4 0x26 0x51 0x1b 0x20 0x1b 0 0x2a 0x2b 0;\
sleep 1; done
Signed-off-by: Yong Li <yong.b.li@linux.intel.com>
Change-Id: I2ddb934199e2d0f52a1a6df25c46eadfe06cfda3
diff --git a/include/ADCSensor.hpp b/include/ADCSensor.hpp
index e1f1330..f49b3db 100644
--- a/include/ADCSensor.hpp
+++ b/include/ADCSensor.hpp
@@ -57,7 +57,7 @@
gpiod::line line;
};
-class ADCSensor : public Sensor
+class ADCSensor : public Sensor, public std::enable_shared_from_this<ADCSensor>
{
public:
ADCSensor(const std::string& path,
@@ -69,19 +69,19 @@
const std::string& sensorConfiguration,
std::optional<BridgeGpio>&& bridgeGpio);
~ADCSensor();
+ void setupRead(void);
private:
sdbusplus::asio::object_server& objServer;
boost::asio::posix::stream_descriptor inputDev;
boost::asio::deadline_timer waitTimer;
- boost::asio::streambuf readBuf;
+ std::shared_ptr<boost::asio::streambuf> readBuf;
std::string path;
size_t errCount;
double scaleFactor;
std::optional<BridgeGpio> bridgeGpio;
PowerState readState;
thresholds::ThresholdTimer thresholdTimer;
- void setupRead(void);
void handleResponse(const boost::system::error_code& err);
void checkThresholds(void) override;
};
diff --git a/src/ADCSensor.cpp b/src/ADCSensor.cpp
index 2ab1a4f..d22afd8 100644
--- a/src/ADCSensor.cpp
+++ b/src/ADCSensor.cpp
@@ -54,10 +54,10 @@
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(_thresholds), sensorConfiguration,
"xyz.openbmc_project.Configuration.ADC", maxReading, minReading),
- objServer(objectServer), inputDev(io, open(path.c_str(), O_RDONLY)),
- waitTimer(io), path(path), errCount(0), scaleFactor(scaleFactor),
- bridgeGpio(std::move(bridgeGpio)), readState(std::move(readState)),
- thresholdTimer(io, this)
+ std::enable_shared_from_this<ADCSensor>(), objServer(objectServer),
+ inputDev(io, open(path.c_str(), O_RDONLY)), waitTimer(io), path(path),
+ errCount(0), scaleFactor(scaleFactor), bridgeGpio(std::move(bridgeGpio)),
+ readState(std::move(readState)), thresholdTimer(io, this)
{
sensorInterface = objectServer.add_interface(
"/xyz/openbmc_project/sensors/voltage/" + name,
@@ -77,7 +77,6 @@
association = objectServer.add_interface(
"/xyz/openbmc_project/sensors/voltage/" + name, association::interface);
setInitialProperties(conn);
- setupRead();
// setup match
setupPowerMatch(conn);
@@ -96,6 +95,11 @@
void ADCSensor::setupRead(void)
{
+ std::shared_ptr<boost::asio::streambuf> buffer =
+ std::make_shared<boost::asio::streambuf>();
+
+ std::weak_ptr<ADCSensor> weakRef = weak_from_this();
+
if (bridgeGpio.has_value())
{
(*bridgeGpio).set(1);
@@ -105,33 +109,55 @@
// could be instability.
waitTimer.expires_from_now(
boost::posix_time::milliseconds(gpioBridgeEnableMs));
- waitTimer.async_wait([&](const boost::system::error_code& ec) {
- if (ec == boost::asio::error::operation_aborted)
- {
- return; // we're being canceled
- }
- boost::asio::async_read_until(
- inputDev, readBuf, '\n',
- [&](const boost::system::error_code& ec,
- std::size_t /*bytes_transfered*/) { handleResponse(ec); });
- });
+ waitTimer.async_wait(
+ [weakRef, buffer](const boost::system::error_code& ec) {
+ std::shared_ptr<ADCSensor> self = weakRef.lock();
+ if (ec == boost::asio::error::operation_aborted)
+ {
+ return; // we're being canceled
+ }
+
+ if (self)
+ {
+ boost::asio::async_read_until(
+ self->inputDev, *buffer, '\n',
+ [weakRef, buffer](const boost::system::error_code& ec,
+ std::size_t /*bytes_transfered*/) {
+ std::shared_ptr<ADCSensor> self = weakRef.lock();
+ if (self)
+ {
+ self->readBuf = buffer;
+ self->handleResponse(ec);
+ }
+ });
+ }
+ });
}
else
{
boost::asio::async_read_until(
- inputDev, readBuf, '\n',
- [&](const boost::system::error_code& ec,
- std::size_t /*bytes_transfered*/) { handleResponse(ec); });
+ inputDev, *buffer, '\n',
+ [weakRef, buffer](const boost::system::error_code& ec,
+ std::size_t /*bytes_transfered*/) {
+ std::shared_ptr<ADCSensor> self = weakRef.lock();
+ if (self)
+ {
+ self->readBuf = buffer;
+ self->handleResponse(ec);
+ }
+ });
}
}
void ADCSensor::handleResponse(const boost::system::error_code& err)
{
+ std::weak_ptr<ADCSensor> weakRef = weak_from_this();
+
if (err == boost::system::errc::bad_file_descriptor)
{
return; // we're being destroyed
}
- std::istream responseStream(&readBuf);
+ std::istream responseStream(readBuf.get());
if (!err)
{
@@ -188,12 +214,17 @@
}
inputDev.assign(fd);
waitTimer.expires_from_now(boost::posix_time::milliseconds(sensorPollMs));
- waitTimer.async_wait([&](const boost::system::error_code& ec) {
+ waitTimer.async_wait([weakRef](const boost::system::error_code& ec) {
+ std::shared_ptr<ADCSensor> self = weakRef.lock();
if (ec == boost::asio::error::operation_aborted)
{
return; // we're being canceled
}
- setupRead();
+
+ if (self)
+ {
+ self->setupRead();
+ }
});
}
diff --git a/src/ADCSensorMain.cpp b/src/ADCSensorMain.cpp
index 1b56d93..df35722 100644
--- a/src/ADCSensorMain.cpp
+++ b/src/ADCSensorMain.cpp
@@ -65,7 +65,7 @@
void createSensors(
boost::asio::io_service& io, sdbusplus::asio::object_server& objectServer,
- boost::container::flat_map<std::string, std::unique_ptr<ADCSensor>>&
+ boost::container::flat_map<std::string, std::shared_ptr<ADCSensor>>&
sensors,
std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
const std::unique_ptr<boost::container::flat_set<std::string>>&
@@ -269,10 +269,11 @@
}
}
- sensor = std::make_unique<ADCSensor>(
+ sensor = std::make_shared<ADCSensor>(
path.string(), objectServer, dbusConnection, io, sensorName,
std::move(sensorThresholds), scaleFactor, readState,
*interfacePath, std::move(bridgeGpio));
+ sensor->setupRead();
}
}));
@@ -286,7 +287,7 @@
auto systemBus = std::make_shared<sdbusplus::asio::connection>(io);
systemBus->request_name("xyz.openbmc_project.ADCSensor");
sdbusplus::asio::object_server objectServer(systemBus);
- boost::container::flat_map<std::string, std::unique_ptr<ADCSensor>> sensors;
+ boost::container::flat_map<std::string, std::shared_ptr<ADCSensor>> sensors;
std::vector<std::unique_ptr<sdbusplus::bus::match::match>> matches;
std::unique_ptr<boost::container::flat_set<std::string>> sensorsChanged =
std::make_unique<boost::container::flat_set<std::string>>();