nemo.cpp: deadlock when calling stop during synchronization removed

src/MeasurementComplexItem/MeasurementComplexItem.cc

 #include "MeasurementComplexItem.h"
 
+#include "AreaData.h"
 #include "CircularGenerator.h"
 #include "LinearGenerator.h"
 #include "NemoInterface.h"

src/MeasurementComplexItem/MeasurementComplexItem.h

@@ -9,9 +9,9 @@
 #include "QGCQGeoCoordinate.h"
 #include "SettingsFact.h"
 
-#include "AreaData.h"
 #include "geometry/ProgressArray.h"
 
+class AreaData;
 class RoutingThread;
 class RoutingResult;
 

src/MeasurementComplexItem/NemoInterface.cpp

@@ -181,7 +181,7 @@ private:
   // Worker functions.
   static bool
   _callAddTiles(const QVector<std::shared_ptr<const Tile>> &tileArray,
-                Dispatcher &d, Rosbridge &rb);
+                Dispatcher &disp, Rosbridge &rb);
   static bool _callClearTiles(Dispatcher &d, Rosbridge &rb);
   static ProgressArray _callGetProgress(const IDArray &pIdArray, Dispatcher &d,
                                         Rosbridge &rb);
@@ -880,9 +880,11 @@ void NemoInterface::Impl::_synchronize() {
       if (!synced) {
         auto success = this->_clearTilesImpl();
         if (!success) {
-          Lock lk(this->_m);
-          _setStateNotSafe(STATE::SYNC_ERROR);
-          _doActionNotSafe();
+          Lock lk1(this->_m);
+          if (this->_state == STATE::SYS_SYNC) {
+            _setStateNotSafe(STATE::SYNC_ERROR);
+            _doActionNotSafe();
+          }
           return QVariant(false);
         }
 
@@ -894,9 +896,11 @@ void NemoInterface::Impl::_synchronize() {
         success = this->_addTilesImpl(pTileArray, pIdArray);
 
         if (!success) {
-          Lock lk(this->_m);
-          _setStateNotSafe(STATE::SYNC_ERROR);
-          _doActionNotSafe();
+          Lock lk2(this->_m);
+          if (this->_state == STATE::SYS_SYNC) {
+            _setStateNotSafe(STATE::SYNC_ERROR);
+            _doActionNotSafe();
+          }
           return QVariant(false);
         }
       } else {
@@ -914,12 +918,14 @@ void NemoInterface::Impl::_synchronize() {
 
         if (!ret) {
           Lock lk(this->_m);
-          this->_setStateNotSafe(STATE::SYNC_ERROR);
-          this->_doActionNotSafe();
-          lk.unlock();
-          this->_setWarningString("Getting progress failed.");
-          qCDebug(NemoInterfaceLog)
-              << "_addTilesImpl(): _updateProgress failed: unknown id.";
+          if (this->_state == STATE::SYS_SYNC) {
+            this->_setStateNotSafe(STATE::SYNC_ERROR);
+            this->_doActionNotSafe();
+            lk.unlock();
+            this->_setWarningString("Getting progress failed.");
+            qCDebug(NemoInterfaceLog)
+                << "_addTilesImpl(): _updateProgress failed: unknown id.";
+          }
           return QVariant(false);
         }
       }
@@ -976,7 +982,6 @@ void NemoInterface::Impl::_doActionNotSafe() {
   static bool resetDone = false;
   switch (this->_state) {
   case STATE::STOPPED: {
-    std::promise<void> p;
     INVM(this->_parent, ([this]() mutable {
            this->_timeoutTimer.stop();
            this->_restartTimer.stop();
@@ -991,7 +996,6 @@ void NemoInterface::Impl::_doActionNotSafe() {
 
     _setInfoStringNotSafe("");
     _setWarningStringNotSafe("");
-
   } break;
 
   case STATE::START_BRIDGE: {
@@ -999,14 +1003,20 @@ void NemoInterface::Impl::_doActionNotSafe() {
          ([this]() mutable { this->_restartTimer.start(RESTART_INTERVAl); }))
 
     this->_pRosbridge->start();
+    _setInfoStringNotSafe("");
+    _setWarningStringNotSafe("");
   } break;
   case STATE::WEBSOCKET_DETECTED:
     resetDone = false;
     this->_setStateNotSafe(STATE::TRY_SETUP);
     this->_doActionNotSafe();
+    _setInfoStringNotSafe("");
+    _setWarningStringNotSafe("");
     break;
   case STATE::TRY_SETUP:
     this->_doSetup();
+    _setInfoStringNotSafe("");
+    _setWarningStringNotSafe("");
     break;
   case STATE::READY: {
     INVM(this->_parent,
@@ -1023,10 +1033,13 @@ void NemoInterface::Impl::_doActionNotSafe() {
   case STATE::USER_SYNC:
   case STATE::SYS_SYNC:
   case STATE::SYNC_ERROR:
+    _setInfoStringNotSafe("");
+    _setWarningStringNotSafe("");
     break;
   case STATE::HEARTBEAT_TIMEOUT: {
     INVM(this->_parent, ([this]() mutable { this->_syncTimer.stop(); }))
-
+    _setInfoStringNotSafe("");
+    _setWarningStringNotSafe("");
   } break;
   case STATE::WEBSOCKET_TIMEOUT: {
     INVM(this->_parent, ([this]() mutable {
@@ -1049,7 +1062,7 @@ void NemoInterface::Impl::_doActionNotSafe() {
 }
 
 bool NemoInterface::Impl::_callAddTiles(
-    const QVector<std::shared_ptr<const Tile>> &tileArray, Dispatcher &d,
+    const QVector<std::shared_ptr<const Tile>> &tileArray, Dispatcher &disp,
     Rosbridge &rb) {
 
   // qDebug() << "_callAddTiles called";
@@ -1091,14 +1104,14 @@ bool NemoInterface::Impl::_callAddTiles(
   // wait for response.
   auto tStart = hrc::now();
   bool abort = true;
-  do {
-    auto status = future_response.wait_for(std::chrono::milliseconds(100));
+  while (hrc::now() - tStart < maxResponseTime &&
+         !disp.isInterruptionRequested()) {
+    auto status = future_response.wait_for(std::chrono::milliseconds(10));
     if (status == std::future_status::ready) {
       abort = false;
       break;
     }
-  } while (hrc::now() - tStart < maxResponseTime ||
-           d.isInterruptionRequested());
+  }
 
   if (abort) {
     qCWarning(NemoInterfaceLog)
@@ -1149,14 +1162,14 @@ bool NemoInterface::Impl::_callRemoveTiles(const IDArray &pIdArray,
   // wait for response.
   auto tStart = hrc::now();
   bool abort = true;
-  do {
-    auto status = future_response.wait_for(std::chrono::milliseconds(100));
+  while (hrc::now() - tStart < maxResponseTime &&
+         !disp.isInterruptionRequested()) {
+    auto status = future_response.wait_for(std::chrono::milliseconds(10));
     if (status == std::future_status::ready) {
       abort = false;
       break;
     }
-  } while (hrc::now() - tStart < maxResponseTime ||
-           disp.isInterruptionRequested());
+  }
 
   if (abort) {
     qCWarning(NemoInterfaceLog)
@@ -1188,14 +1201,14 @@ bool NemoInterface::Impl::_callClearTiles(Dispatcher &disp, Rosbridge &rb) {
   // wait for response.
   auto tStart = hrc::now();
   bool abort = true;
-  do {
-    auto status = future_response.wait_for(std::chrono::milliseconds(100));
+  while (hrc::now() - tStart < maxResponseTime &&
+         !disp.isInterruptionRequested()) {
+    auto status = future_response.wait_for(std::chrono::milliseconds(10));
     if (status == std::future_status::ready) {
       abort = false;
       break;
     }
-  } while (hrc::now() - tStart < maxResponseTime ||
-           disp.isInterruptionRequested());
+  }
 
   if (abort) {
     qCWarning(NemoInterfaceLog) << "Websocket not responding to request.";
@@ -1251,14 +1264,14 @@ ProgressArray NemoInterface::Impl::_callGetProgress(const IDArray &pIdArray,
   // wait for response.
   auto tStart = hrc::now();
   bool abort = true;
-  do {
-    auto status = future_response.wait_for(std::chrono::milliseconds(100));
+  while (hrc::now() - tStart < maxResponseTime &&
+         !disp.isInterruptionRequested()) {
+    auto status = future_response.wait_for(std::chrono::milliseconds(10));
     if (status == std::future_status::ready) {
       abort = false;
       break;
     }
-  } while (hrc::now() - tStart < maxResponseTime ||
-           disp.isInterruptionRequested());
+  }
 
   if (abort) {
     qCWarning(NemoInterfaceLog)
@@ -1302,14 +1315,14 @@ ProgressArray NemoInterface::Impl::_callGetAllProgress(Dispatcher &disp,
   // wait for response.
   auto tStart = hrc::now();
   bool abort = true;
-  do {
-    auto status = future_response.wait_for(std::chrono::milliseconds(100));
+  while (hrc::now() - tStart < maxResponseTime &&
+         !disp.isInterruptionRequested()) {
+    auto status = future_response.wait_for(std::chrono::milliseconds(10));
     if (status == std::future_status::ready) {
       abort = false;
       break;
     }
-  } while (hrc::now() - tStart < maxResponseTime ||
-           disp.isInterruptionRequested());
+  }
 
   if (abort) {
     qCWarning(NemoInterfaceLog)
@@ -1368,14 +1381,14 @@ NemoInterface::Impl::_callGetAllTiles(Dispatcher &disp, Rosbridge &rb) {
   // wait for response.
   auto tStart = hrc::now();
   bool abort = true;
-  do {
-    auto status = future_response.wait_for(std::chrono::milliseconds(100));
+  while (hrc::now() - tStart < maxResponseTime &&
+         !disp.isInterruptionRequested()) {
+    auto status = future_response.wait_for(std::chrono::milliseconds(10));
     if (status == std::future_status::ready) {
       abort = false;
       break;
     }
-  } while (hrc::now() - tStart < maxResponseTime ||
-           disp.isInterruptionRequested());
+  }
 
   if (abort) {
     qCWarning(NemoInterfaceLog)
@@ -1386,7 +1399,7 @@ NemoInterface::Impl::_callGetAllTiles(Dispatcher &disp, Rosbridge &rb) {
   return *future_response.get();
 }
 
-QString NemoInterface::Impl::_callGetVersion(Dispatcher &d, Rosbridge &rb) {
+QString NemoInterface::Impl::_callGetVersion(Dispatcher &disp, Rosbridge &rb) {
 
   // create response handler.
   typedef QString ResponseType;
@@ -1412,14 +1425,14 @@ QString NemoInterface::Impl::_callGetVersion(Dispatcher &d, Rosbridge &rb) {
   // wait for response.
   auto tStart = hrc::now();
   bool abort = true;
-  do {
-    auto status = future_response.wait_for(std::chrono::milliseconds(100));
+  while (hrc::now() - tStart < maxResponseTime &&
+         !disp.isInterruptionRequested()) {
+    auto status = future_response.wait_for(std::chrono::milliseconds(10));
     if (status == std::future_status::ready) {
       abort = false;
       break;
     }
-  } while (hrc::now() - tStart < maxResponseTime ||
-           d.isInterruptionRequested());
+  }
 
   if (abort) {
     qCWarning(NemoInterfaceLog)
@@ -1653,36 +1666,42 @@ bool NemoInterface::Impl::_addTilesImpl(
   auto ret = this->_addTilesRemote(*pTileArray);
   if (!ret) {
     Lock lk(this->_m);
-    this->_setStateNotSafe(STATE::SYNC_ERROR);
-    this->_doActionNotSafe();
-    lk.unlock();
-    this->_setWarningString("Adding tiles failed.");
-    qCDebug(NemoInterfaceLog) << "_addTilesImpl(): _addTilesRemote return "
-                                 "false: different tiles with same id.";
+    if (this->_state == STATE::SYS_SYNC || this->_state == STATE::USER_SYNC) {
+      this->_setStateNotSafe(STATE::SYNC_ERROR);
+      this->_doActionNotSafe();
+      lk.unlock();
+      this->_setWarningString("Adding tiles failed.");
+      qCDebug(NemoInterfaceLog) << "_addTilesImpl(): _addTilesRemote return "
+                                   "false: different tiles with same id.";
+    }
     return false;
   }
 
   // fetch progress
   auto array = this->_callGetProgress(*pIdArray, disp, *rb);
   if (array.size() != pIdArray->size()) {
-    Lock lk(this->_m);
-    this->_setStateNotSafe(STATE::SYNC_ERROR);
-    this->_doActionNotSafe();
-    lk.unlock();
-    this->_setWarningString("Getting progress failed. This might "
-                            "indicate a poor connection.");
+    Lock lk1(this->_m);
+    if (this->_state == STATE::SYS_SYNC || this->_state == STATE::USER_SYNC) {
+      this->_setStateNotSafe(STATE::SYNC_ERROR);
+      this->_doActionNotSafe();
+      lk1.unlock();
+      this->_setWarningString("Getting progress failed. This might "
+                              "indicate a poor connection.");
+    }
     return false;
   }
   ret = this->_updateProgress(array);
 
   if (!ret) {
-    Lock lk(this->_m);
-    this->_setStateNotSafe(STATE::SYNC_ERROR);
-    this->_doActionNotSafe();
-    lk.unlock();
-    this->_setWarningString("Getting progress failed.");
-    qCDebug(NemoInterfaceLog)
-        << "_addTilesImpl(): _updateProgress failed: unknown id.";
+    Lock lk2(this->_m);
+    if (this->_state == STATE::SYS_SYNC || this->_state == STATE::USER_SYNC) {
+      this->_setStateNotSafe(STATE::SYNC_ERROR);
+      this->_doActionNotSafe();
+      lk2.unlock();
+      this->_setWarningString("Getting progress failed.");
+      qCDebug(NemoInterfaceLog)
+          << "_addTilesImpl(): _updateProgress failed: unknown id.";
+    }
     return false;
   }
 
@@ -1697,13 +1716,16 @@ bool NemoInterface::Impl::_removeTilesImpl(
   auto success = this->_callRemoveTiles(*pIdArray, disp, *rb);
   if (!success) {
     Lock lk(this->_m);
-    this->_setStateNotSafe(STATE::SYNC_ERROR);
-    this->_doActionNotSafe();
-    lk.unlock();
-    this->_setWarningString("Removing tiles failed. This might "
-                            "indicate a poor connection.");
+    if (this->_state == STATE::SYS_SYNC || this->_state == STATE::USER_SYNC) {
+      this->_setStateNotSafe(STATE::SYNC_ERROR);
+      this->_doActionNotSafe();
+      lk.unlock();
+      this->_setWarningString("Removing tiles failed. This might "
+                              "indicate a poor connection.");
+    }
     return false;
   }
+
   this->_removeTilesRemote(*pIdArray);
 
   return true;
@@ -1716,11 +1738,13 @@ bool NemoInterface::Impl::_clearTilesImpl() {
   auto success = this->_callClearTiles(disp, *rb);
   if (!success) {
     Lock lk(this->_m);
-    this->_setStateNotSafe(STATE::SYNC_ERROR);
-    this->_doActionNotSafe();
-    lk.unlock();
-    this->_setWarningString("Clear tiles failed. This might "
-                            "indicate a poor connection.");
+    if (this->_state == STATE::SYS_SYNC || this->_state == STATE::USER_SYNC) {
+      this->_setStateNotSafe(STATE::SYNC_ERROR);
+      this->_doActionNotSafe();
+      lk.unlock();
+      this->_setWarningString("Clear tiles failed. This might "
+                              "indicate a poor connection.");
+    }
     return false;
   }
   this->_clearTilesRemote();

src/MeasurementComplexItem/NemoInterface.h

@@ -13,8 +13,6 @@
 #include "TilePtrArray.h"
 #include "geometry/ProgressArray.h"
 
-// Singelton class used to interface measurement devices implementing the nemo
-// interface.
 class NemoInterface : public QObject {
   Q_OBJECT
   class Impl;

src/MeasurementComplexItem/geometry/MeasurementArea.cc

@@ -15,6 +15,8 @@
 #include "QGCLoggingCategory.h"
 #include "QmlObjectListHelper.h"
 
+#include "MeasurementComplexItem/nemo_interface/MeasurementTile.h"
+
 #ifndef MAX_TILES
 #define MAX_TILES 1000
 #endif
@@ -602,27 +604,6 @@ void MeasurementArea::setState(MeasurementArea::STATE s) {
   }
 }
 
-void MeasurementArea::updateIds(const QList<TileDiff> &array) {
-  for (const auto &diff : array) {
-
-    auto it = _indexMap.find(diff.oldTile.id());
-
-    if (it != _indexMap.end()) {
-      int tileIndex = it->second;
-      auto *tile = _tiles->value<MeasurementTile *>(tileIndex);
-      if (diff.oldTile.coordinateList() == tile->coordinateList()) {
-        // Change id and update _tileMap.
-        const auto newId = diff.newTile.id();
-        tile->setId(newId);
-        _indexMap.erase(it);
-        auto ret = _indexMap.insert(std::make_pair(newId, tileIndex));
-        Q_ASSERT(ret.second == true /*insert success?*/);
-        Q_UNUSED(ret);
-      }
-    }
-  }
-}
-
 bool getTiles(const FPolygon &area, Length tileHeight, Length tileWidth,
               Area minTileArea, std::vector<FPolygon> &tiles,
               BoundingBox &bbox) {

src/MeasurementComplexItem/geometry/MeasurementArea.h

@@ -6,10 +6,8 @@
 #include <QTimer>
 
 #include "GeoArea.h"
-#include "MeasurementComplexItem/nemo_interface/MeasurementTile.h"
-#include "ProgressArray.h"
-#include "TileDiff.h"
 
+#include "ProgressArray.h"
 #include "SettingsFact.h"
 
 class MeasurementArea : public GeoArea {
@@ -61,7 +59,6 @@ signals:
   void progressChanged();
 
 public slots:
-  void updateIds(const QList<TileDiff> &array);
   void updateProgress(const ProgressArray &array);
   Q_INVOKABLE void randomProgress();
   Q_INVOKABLE void resetProgress();

src/MeasurementComplexItem/geometry/ProgressArray.h

@@ -2,7 +2,6 @@
 #define PROGRESSARRAY_H
 
 #include <QVector>
-#include <tuple>
 
 #include "comm/ros_bridge/include/messages/nemo_msgs/labeled_progress.h"
 

src/MeasurementComplexItem/nemo_interface/TaskDispatcher.cpp

@@ -52,11 +52,7 @@ void TaskDispatcher::start() {
   ULock lk1(this->_mutex);
   if (!_running) {
     this->_running = true;
-    auto p = std::make_shared<std::promise<void>>();
-    _threadFuture = p->get_future();
-    QtConcurrent::run([this, p = std::move(p)]() mutable {
-      return this->run(std::move(*p));
-    });
+    QtConcurrent::run([this]() mutable { return this->run(); });
     lk1.unlock();
   }
 }
@@ -65,7 +61,6 @@ void TaskDispatcher::stop() {
   ULock lk1(this->_mutex);
   if (_running) {
     this->_running = false;
-    _threadFuture.wait();
     lk1.unlock();
   }
 }
@@ -92,7 +87,7 @@ std::size_t TaskDispatcher::pendingTasks() {
 
 bool TaskDispatcher::idle() { return this->pendingTasks() == 0; }
 
-void TaskDispatcher::run(std::promise<void> p) {
+void TaskDispatcher::run() {
   while (true) {
     ULock lk1(this->_mutex);
 
@@ -116,7 +111,6 @@ void TaskDispatcher::run(std::promise<void> p) {
       break;
     }
   }
-  p.set_value();
 }
 
 } // namespace nemo_interface

src/MeasurementComplexItem/nemo_interface/TaskDispatcher.h

@@ -56,12 +56,11 @@ public:
   bool idle();
 
 protected:
-  void run(std::promise<void> p);
+  void run();
 
 private:
   std::deque<std::unique_ptr<Task>> _taskQueue;
   std::deque<std::promise<QVariant>> _promiseQueue;
-  std::future<void> _threadFuture;
   bool _running;
   std::mutex _mutex;
 };