NVMeContext: Rework sensor removal concurrent to polling
Concurrent removal of a sensor's configuration while the sensor list is
being iterated for polling can lead to undefined behaviour via access
through a deleted iterator:
Program terminated with signal SIGSEGV, Segmentation fault.
#0 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count (__r=..., this=<optimised out>, this=<optimised out>, __r=...)
at /usr/include/c++/11.2.0/bits/stl_list.h:224
224 /usr/include/c++/11.2.0/bits/stl_list.h: No such file or directory.
[Current thread is 1 (LWP 6649)]
#0 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count (__r=..., this=<optimised out>, this=<optimised out>, __r=...)
at /usr/include/c++/11.2.0/bits/stl_list.h:224
#1 std::__shared_ptr<NVMeSensor, (__gnu_cxx::_Lock_policy)2>::__shared_ptr (this=<optimised out>, this=<optimised out>)
at /usr/include/c++/11.2.0/bits/shared_ptr_base.h:1152
#2 std::shared_ptr<NVMeSensor>::shared_ptr (this=<optimised out>, this=<optimised out>) at /usr/include/c++/11.2.0/bits/shared_ptr.h:150
#3 NVMeBasicContext::readAndProcessNVMeSensor (this=0x1ac8a90, iter=non-dereferenceable iterator for std::list) at ../git/src/NVMeBasicContext.cpp:299
#4 0x004dd8b8 in NVMeBasicContext::readAndProcessNVMeSensor (this=0x1ac8a90, iter=non-dereferenceable iterator for std::list) at ../git/src/NVMeBasicContext.cpp:312
#5 0x004dd8b8 in NVMeBasicContext::readAndProcessNVMeSensor (this=0x1ac8a90, iter=std::shared_ptr<NVMeSensor> (use count 26, weak count 28153871) = {get() = 0x1ad8db0})
at ../git/src/NVMeBasicContext.cpp:312
Rework polling and sensor removal to uphold the requirement that the
iterator remains valid.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I69b005fe3dad7ddf21d1762731f9cdfd2408cae1
diff --git a/include/NVMeBasicContext.hpp b/include/NVMeBasicContext.hpp
index 6953dd7..ec7a0b1 100644
--- a/include/NVMeBasicContext.hpp
+++ b/include/NVMeBasicContext.hpp
@@ -11,8 +11,7 @@
NVMeBasicContext(boost::asio::io_service& io, int rootBus);
~NVMeBasicContext() override = default;
void pollNVMeDevices() override;
- void readAndProcessNVMeSensor(
- std::list<std::shared_ptr<NVMeSensor>>::iterator iter) override;
+ void readAndProcessNVMeSensor() override;
void processResponse(std::shared_ptr<NVMeSensor>& sensor, void* msg,
size_t len) override;
diff --git a/include/NVMeContext.hpp b/include/NVMeContext.hpp
index 2489815..230ca74 100644
--- a/include/NVMeContext.hpp
+++ b/include/NVMeContext.hpp
@@ -12,7 +12,7 @@
{
public:
NVMeContext(boost::asio::io_service& io, int rootBus) :
- scanTimer(io), rootBus(rootBus)
+ scanTimer(io), rootBus(rootBus), pollCursor(sensors.end())
{
if (rootBus < 0)
{
@@ -45,9 +45,44 @@
return std::nullopt;
}
+ // Post-condition: The sensor list does not contain the provided sensor
+ // Post-condition: pollCursor is a valid iterator for the sensor list
void removeSensor(const std::shared_ptr<NVMeSensor>& sensor)
{
- sensors.remove(sensor);
+ // Locate the sensor that we're removing in the sensor list
+ auto found = std::find(sensors.begin(), sensors.end(), sensor);
+
+ // If we failed to find the sensor in the list the post-condition is
+ // already satisfied
+ if (found == sensors.end())
+ {
+ return;
+ }
+
+ // We've found the sensor in the list
+
+ // If we're not actively polling the sensor list, then remove the sensor
+ if (pollCursor == sensors.end())
+ {
+ sensors.erase(found);
+ return;
+ }
+
+ // We're actively polling the sensor list
+
+ // If we're not polling the specific sensor that has been removed, then
+ // remove the sensor
+ if (*pollCursor != *found)
+ {
+ sensors.erase(found);
+ return;
+ }
+
+ // We're polling the sensor that is being removed
+
+ // Remove the sensor and update the poll cursor so the cursor remains
+ // valid
+ pollCursor = sensors.erase(found);
}
virtual void close()
@@ -57,16 +92,16 @@
virtual void pollNVMeDevices() = 0;
- virtual void readAndProcessNVMeSensor(
- std::list<std::shared_ptr<NVMeSensor>>::iterator iter) = 0;
+ virtual void readAndProcessNVMeSensor() = 0;
virtual void processResponse(std::shared_ptr<NVMeSensor>& sensor, void* msg,
size_t len) = 0;
protected:
boost::asio::deadline_timer scanTimer;
- int rootBus; // Root bus for this drive
- std::list<std::shared_ptr<NVMeSensor>> sensors; // used as a poll queue
+ int rootBus; // Root bus for this drive
+ std::list<std::shared_ptr<NVMeSensor>> sensors;
+ std::list<std::shared_ptr<NVMeSensor>>::iterator pollCursor;
};
using NVMEMap = boost::container::flat_map<int, std::shared_ptr<NVMeContext>>;
diff --git a/src/NVMeBasicContext.cpp b/src/NVMeBasicContext.cpp
index 5708ad8..d0ece2c 100644
--- a/src/NVMeBasicContext.cpp
+++ b/src/NVMeBasicContext.cpp
@@ -246,29 +246,28 @@
thread.detach();
}
-void NVMeBasicContext::readAndProcessNVMeSensor(
- std::list<std::shared_ptr<NVMeSensor>>::iterator iter)
+void NVMeBasicContext::readAndProcessNVMeSensor()
{
- if (iter == sensors.end())
+ if (pollCursor == sensors.end())
{
this->pollNVMeDevices();
return;
}
- std::shared_ptr<NVMeSensor> sensor = *iter++;
+ std::shared_ptr<NVMeSensor> sensor = *pollCursor++;
if (!sensor->readingStateGood())
{
sensor->markAvailable(false);
sensor->updateValue(std::numeric_limits<double>::quiet_NaN());
- readAndProcessNVMeSensor(iter);
+ readAndProcessNVMeSensor();
return;
}
/* Potentially defer sampling the sensor sensor if it is in error */
if (!sensor->sample())
{
- readAndProcessNVMeSensor(iter);
+ readAndProcessNVMeSensor();
return;
}
@@ -326,7 +325,7 @@
response->prepare(len);
return len;
},
- [self{shared_from_this()}, iter, sensor, response](
+ [self{shared_from_this()}, sensor, response](
const boost::system::error_code& ec, std::size_t length) mutable {
if (ec)
{
@@ -350,30 +349,30 @@
self->processResponse(sensor, data.data(), data.size());
/* Enqueue processing of the next sensor */
- self->readAndProcessNVMeSensor(iter);
+ self->readAndProcessNVMeSensor();
});
}
void NVMeBasicContext::pollNVMeDevices()
{
- auto scan = sensors.begin();
+ pollCursor = sensors.begin();
scanTimer.expires_from_now(boost::posix_time::seconds(1));
- scanTimer.async_wait([self{shared_from_this()},
- scan](const boost::system::error_code errorCode) {
- if (errorCode == boost::asio::error::operation_aborted)
- {
- return;
- }
+ scanTimer.async_wait(
+ [self{shared_from_this()}](const boost::system::error_code errorCode) {
+ if (errorCode == boost::asio::error::operation_aborted)
+ {
+ return;
+ }
- if (errorCode)
- {
- std::cerr << errorCode.message() << "\n";
- return;
- }
+ if (errorCode)
+ {
+ std::cerr << errorCode.message() << "\n";
+ return;
+ }
- self->readAndProcessNVMeSensor(scan);
- });
+ self->readAndProcessNVMeSensor();
+ });
}
static double getTemperatureReading(int8_t reading)