Merge branch 'disable_navmeshdb_write_on_lock' into 'master'

Disable writes to navmeshdb on database is locked error

See merge request OpenMW/openmw!1837

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;