NavCache: don’t crash if rebuild is abandoned

Add a flag, so we can cleanly exit/join the rebuild thread, if we
are asked to delete the NavCache during a rebuild.

src/Main/bootstrap.cxx

@@ -58,15 +58,16 @@
 #include <osg/Version>
 #include <osgText/Font>
 
-#include <Viewer/fgviewer.hxx>
 #include "main.hxx"
-#include <flightgearBuildId.h>
-#include <Main/globals.hxx>
-#include <Main/fg_init.hxx>
-#include <Main/options.hxx>
-#include <Main/fg_props.hxx>
-#include <Main/sentryIntegration.hxx>
 #include <GUI/MessageBox.hxx>
+#include <Main/fg_init.hxx>
+#include <Main/fg_props.hxx>
+#include <Main/globals.hxx>
+#include <Main/options.hxx>
+#include <Main/sentryIntegration.hxx>
+#include <Navaids/NavDataCache.hxx>
+#include <Viewer/fgviewer.hxx>
+#include <flightgearBuildId.h>
 
 #include "fg_os.hxx"
 
@@ -366,6 +367,8 @@ void fgExitCleanup() {
         fgOSCloseWindow();
     }
 
+    flightgear::NavDataCache::shutdown();
+
     // you might imagine we'd call shutdownQtApp here, but it's not safe to do
     // so in an atexit handler, and crashes on Mac. Thiago states this explicitly:
     // https://bugreports.qt.io/browse/QTBUG-48709

src/Navaids/NavDataCache.cxx

@@ -154,6 +154,14 @@ static string cleanRunwayNo(const string& aRwyNo)
   return result;
 }
 
+class AbandonCacheException : public sg_exception
+{
+public:
+    AbandonCacheException() : sg_exception("Cache build cancelled", {}, {}, false)
+    {
+    }
+};
+
 } // anonymous namespace
 
 namespace flightgear
@@ -178,6 +186,11 @@ public:
 
   }
 
+  ~RebuildThread()
+  {
+      join();
+  }
+
   bool isFinished() const
   {
     std::lock_guard<std::mutex> g(_lock);
@@ -468,6 +481,9 @@ public:
 
   bool stepSelect(sqlite3_stmt_ptr stmt)
   {
+      if (abandonCache)
+          throw AbandonCacheException{};
+
       int retries = 0;
       int result;
       int retryMSec = 10;
@@ -801,6 +817,9 @@ public:
                                 const string& name, const SGGeod& pos, PositionedID apt,
                                 bool spatialIndex)
   {
+      if (abandonCache)
+          throw AbandonCacheException{};
+
       SGVec3d cartPos(SGVec3d::fromGeod(pos));
 
       sqlite3_bind_int(insertPositionedQuery, 1, ty);
@@ -912,6 +931,11 @@ public:
   SGPath path;
     bool readOnly;
 
+    // flag set during shutdown: allows us to abandon queries, etc
+    // if exit is requested during a rebuild.
+    bool abandonCache = false;
+
+
     /// the actual cache of ID -> instances. This holds an owning reference,
     /// so once items are in the cache they will never be deleted until
     /// the cache drops its reference
@@ -942,7 +966,7 @@ public:
     sqlite3_stmt_ptr removePOIQuery;
 
     sqlite3_stmt_ptr findClosestWithIdent;
-// octree (spatial index) related queries
+    // octree (spatial index) related queries
     sqlite3_stmt_ptr getOctreeChildren, insertOctree, updateOctreeChildren,
         getOctreeLeafChildren;
 
@@ -955,15 +979,15 @@ public:
 
     sqlite3_stmt_ptr runwayLengthFtQuery;
 
-// airways
+    // airways
     sqlite3_stmt_ptr findAirway, findAirwayNet, insertAirwayEdge,
         isPosInAirway, airwayEdgesFrom, airwayEdgesTo,
         insertAirway, airwayEdges;
     sqlite3_stmt_ptr loadAirway;
 
-// since there's many permutations of ident/name queries, we create
+    // since there's many permutations of ident/name queries, we create
-// them programtically, but cache the exact query by its raw SQL once
+    // them programtically, but cache the exact query by its raw SQL once
-// used.
+    // used.
     std::map<string, sqlite3_stmt_ptr> findByStringDict;
 
     typedef std::vector<sqlite3_stmt_ptr> StmtVec;
@@ -981,16 +1005,18 @@ public:
 FGPositioned* NavDataCache::NavDataCachePrivate::loadById(sqlite3_int64 rowid,
                                                           sqlite3_int64& aptId)
 {
+    if (abandonCache)
+        throw AbandonCacheException{};
 
     sqlite3_bind_int64(loadPositioned, 1, rowid);
     execSelect1(loadPositioned);
 
     assert(rowid == sqlite3_column_int64(loadPositioned, 0));
-  FGPositioned::Type ty = (FGPositioned::Type) sqlite3_column_int(loadPositioned, 1);
+    FGPositioned::Type ty = (FGPositioned::Type)sqlite3_column_int(loadPositioned, 1);
 
     PositionedID prowid = static_cast<PositionedID>(rowid);
-  string ident = (char*) sqlite3_column_text(loadPositioned, 2);
+    string ident = (char*)sqlite3_column_text(loadPositioned, 2);
-  string name = (char*) sqlite3_column_text(loadPositioned, 3);
+    string name = (char*)sqlite3_column_text(loadPositioned, 3);
     aptId = sqlite3_column_int64(loadPositioned, 4);
     double lon = sqlite3_column_double(loadPositioned, 5);
     double lat = sqlite3_column_double(loadPositioned, 6);
@@ -1315,12 +1341,24 @@ NavDataCache::NavDataCache()
 NavDataCache::~NavDataCache()
 {
   assert(static_instance == this);
-  static_instance = nullptr;
+
-  d.reset();
+  if (d->rebuilder) {
+      addSentryBreadcrumb("shutting down cache with rebuild active", "info");
+      // setting thsi will cause DB operations to throw the special
+      // AbandonCache exception, and hence cause the rebuild thread to
+      // exit pretty quickly, so the join() won't take too long.
+      d->abandonCache = true;
+      d->rebuilder.reset(); // will the destructor which does a join()
+      addSentryBreadcrumb("abandoned rebuild scuessfully", "info");
+  }
+
 
 // ensure we wip the airports cache too, or we'll get out
 // of sync during tests
   FGAirport::clearAirportsCache();
+
+  static_instance = nullptr;
+  d.reset();
 }
 
 NavDataCache* NavDataCache::createInstance()
@@ -1335,6 +1373,13 @@ NavDataCache* NavDataCache::instance()
   return static_instance;
 }
 
+void NavDataCache::shutdown()
+{
+    if (static_instance) {
+        delete static_instance;
+    }
+}
+
 // Update the lists of dat files used for NavCache freshness checking and
 // rebuilding.
 void NavDataCache::updateListsOfDatFiles() {
@@ -2752,7 +2797,7 @@ NavDataCache::Transaction::Transaction(NavDataCache* cache) :
 
 NavDataCache::Transaction::~Transaction()
 {
-    if (_instance->isReadOnly()) {
+    if (_instance->d->abandonCache || _instance->isReadOnly()) {
         return;
     }
 

src/Navaids/NavDataCache.hxx

@@ -67,6 +67,8 @@ public:
 // static creator
     static NavDataCache* createInstance();
 
+    static void shutdown();
+
     SGPath path() const;
 
     enum DatFileType {