Disable writes to navmeshdb on database is locked error

Simultaneously writing to sqlite3 database is not possible. Process exclusively
locks the database for this. Another process will fail to perform any request
when database is locked. Alternatively it can wait. Handling this situation
properly requires complexity that is not really needed. Users are not expected
to run multiple openmw processes simultaneously using the same navmeshdb.

Before this change running multiple openmw processes using the same navmeshdb
can lead to a crash when first transaction fails to start because there is
exception thrown and not catched.

Remove use of explicit transactions from DbWorker. Handling all possible
transaction states due to different errors brings unnecessary complexity.
Initially they were introduced to increase time between flushes to disk. This
makes sense for navmeshtool because of massive number of writes but for the
engine this is not an issue.

apps/openmw_test_suite/detournavigator/asyncnavmeshupdater.cpp

@@ -262,6 +262,20 @@ namespace
         updater.post(mAgentHalfExtents, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles);
         updater.wait(mListener, WaitConditionType::allJobsDone);
         updater.stop();
+        const std::set<TilePosition> present {
+            TilePosition(-2, 0),
+            TilePosition(-1, -1),
+            TilePosition(-1, 0),
+            TilePosition(-1, 1),
+            TilePosition(0, -2),
+            TilePosition(0, -1),
+            TilePosition(0, 0),
+            TilePosition(0, 1),
+            TilePosition(0, 2),
+            TilePosition(1, -1),
+            TilePosition(1, 0),
+            TilePosition(1, 1),
+        };
         for (int x = -5; x <= 5; ++x)
             for (int y = -5; y <= 5; ++y)
             {
@@ -272,8 +286,9 @@ namespace
                     [&] (const MeshSource& v) { return resolveMeshSource(*dbPtr, v); });
                 if (!objects.has_value())
                     continue;
-                EXPECT_FALSE(dbPtr->findTile(mWorldspace, tilePosition, serialize(mSettings.mRecast, *recastMesh, *objects)).has_value())
-                    << tilePosition.x() << " " << tilePosition.y();
+                EXPECT_EQ(dbPtr->findTile(mWorldspace, tilePosition, serialize(mSettings.mRecast, *recastMesh, *objects)).has_value(),
+                          present.find(tilePosition) != present.end())
+                    << tilePosition.x() << " " << tilePosition.y() << " present=" << (present.find(tilePosition) != present.end());
             }
     }
 }

components/detournavigator/asyncnavmeshupdater.cpp

@@ -678,10 +678,10 @@ namespace DetourNavigator
         mHasJob.notify_all();
     }
 
-    std::optional<JobIt> DbJobQueue::pop(std::chrono::steady_clock::duration timeout)
+    std::optional<JobIt> DbJobQueue::pop()
     {
         std::unique_lock lock(mMutex);
-        mHasJob.wait_for(lock, timeout, [&] { return mShouldStop || !mJobs.empty(); });
+        mHasJob.wait(lock, [&] { return mShouldStop || !mJobs.empty(); });
         if (mJobs.empty())
             return std::nullopt;
         const JobIt job = mJobs.front();
@@ -752,45 +752,18 @@ namespace DetourNavigator
 
     void DbWorker::run() noexcept
     {
-        constexpr std::chrono::seconds transactionInterval(1);
-        auto transaction = mDb->startTransaction(Sqlite3::TransactionMode::Immediate);
-        auto start = std::chrono::steady_clock::now();
         while (!mShouldStop)
         {
             try
             {
-                if (const auto job = mQueue.pop(transactionInterval))
+                if (const auto job = mQueue.pop())
                     processJob(*job);
-                const auto now = std::chrono::steady_clock::now();
-                if (mHasChanges && now - start > transactionInterval)
-                {
-                    mHasChanges = false;
-                    try
-                    {
-                        transaction.commit();
-                    }
-                    catch (const std::exception& e)
-                    {
-                        Log(Debug::Error) << "DbWorker exception on commit: " << e.what();
-                    }
-                    transaction = mDb->startTransaction(Sqlite3::TransactionMode::Immediate);
-                    start = now;
-                }
             }
             catch (const std::exception& e)
             {
                 Log(Debug::Error) << "DbWorker exception: " << e.what();
             }
         }
-        if (mHasChanges)
-            try
-            {
-                transaction.commit();
-            }
-            catch (const std::exception& e)
-            {
-                Log(Debug::Error) << "DbWorker exception on final commit: " << e.what();
-            }
     }
 
     void DbWorker::processJob(JobIt job)
@@ -804,10 +777,19 @@ namespace DetourNavigator
             catch (const std::exception& e)
             {
                 Log(Debug::Error) << "DbWorker exception while processing job " << job->mId << ": " << e.what();
-                if (std::string_view(e.what()).find("database or disk is full") != std::string_view::npos)
+                if (mWriteToDb)
                 {
-                    mWriteToDb = false;
-                    Log(Debug::Warning) << "Writes to navmeshdb are disabled because file size limit is reached or disk is full";
+                    const std::string_view message(e.what());
+                    if (message.find("database or disk is full") != std::string_view::npos)
+                    {
+                        mWriteToDb = false;
+                        Log(Debug::Warning) << "Writes to navmeshdb are disabled because file size limit is reached or disk is full";
+                    }
+                    else if (message.find("database is locked") != std::string_view::npos)
+                    {
+                        mWriteToDb = false;
+                        Log(Debug::Warning) << "Writes to navmeshdb are disabled to avoid concurrent writes from multiple processes";
+                    }
                 }
             }
         };
@@ -833,11 +815,8 @@ namespace DetourNavigator
             Log(Debug::Debug) << "Serializing input for job " << job->mId;
             if (mWriteToDb)
             {
-                const ShapeId shapeId = mNextShapeId;
                 const auto objects = makeDbRefGeometryObjects(job->mRecastMesh->getMeshSources(),
                     [&] (const MeshSource& v) { return resolveMeshSource(*mDb, v, mNextShapeId); });
-                if (shapeId != mNextShapeId)
-                    mHasChanges = true;
                 job->mInput = serialize(mRecastSettings, *job->mRecastMesh, objects);
             }
             else
@@ -877,7 +856,6 @@ namespace DetourNavigator
             Log(Debug::Debug) << "Update db tile by job " << job->mId;
             job->mGeneratedNavMeshData->mUserId = cachedTileData->mTileId;
             mDb->updateTile(cachedTileData->mTileId, mVersion, serialize(*job->mGeneratedNavMeshData));
-            mHasChanges = true;
             return;
         }
 
@@ -893,6 +871,5 @@ namespace DetourNavigator
         mDb->insertTile(mNextTileId, job->mWorldspace, job->mChangedTile,
                         mVersion, job->mInput, serialize(*job->mGeneratedNavMeshData));
         ++mNextTileId;
-        mHasChanges = true;
     }
 }

components/detournavigator/asyncnavmeshupdater.hpp

@@ -87,7 +87,7 @@ namespace DetourNavigator
     public:
         void push(JobIt job);
 
-        std::optional<JobIt> pop(std::chrono::steady_clock::duration timeout);
+        std::optional<JobIt> pop();
 
         void update(TilePosition playerTile, int maxTiles);
 
@@ -137,7 +137,6 @@ namespace DetourNavigator
         DbJobQueue mQueue;
         std::atomic_bool mShouldStop {false};
         std::atomic_size_t mGetTileCount {0};
-        bool mHasChanges = false;
         std::thread mThread;
 
         inline void run() noexcept;