commit b5d7a7fb010f753782a07e18379a954e6915d5ee
author Andrew Jeffery <andrew@aj.id.au> Mon May 02 11:57:03 2022 +0930
committer Andrew Jeffery <andrew@aj.id.au> Thu May 05 00:16:08 2022 +0000
tree c7a5530a39825df3e0d3ed9eda4d2caa975a0b57
parent 0b1ae9ff7ff61f8e562307b72db220d91e830117

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

src/NVMeBasicContext.cpp

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)