From b7caf1f36e8638760605401c0feae56f2b79d7fd Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Tue, 2 Nov 2021 00:09:47 +0000 Subject: [PATCH 1/3] Merge branch 'rtd' into 'master' Fix readthedocs config See merge request OpenMW/openmw!1340 (cherry picked from commit ade3c2c61b29665abfa7562e96ee157c83a89def) 665d756f Fix readthedocs config --- .readthedocs.yaml | 10 ++++++++++ docs/requirements.txt | 2 +- readthedocs.yml | 2 -- 3 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 .readthedocs.yaml delete mode 100644 readthedocs.yml diff --git a/.readthedocs.yaml b/.readthedocs.yaml new file mode 100644 index 000000000..e0b39ec49 --- /dev/null +++ b/.readthedocs.yaml @@ -0,0 +1,10 @@ +version: 2 + +sphinx: + configuration: docs/source/conf.py + +python: + version: 3.8 + install: + - requirements: docs/requirements.txt + diff --git a/docs/requirements.txt b/docs/requirements.txt index 288d462d0..14c09d53e 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,2 +1,2 @@ parse_cmake -sphinx>=1.7.0 +sphinx==1.8.5 diff --git a/readthedocs.yml b/readthedocs.yml deleted file mode 100644 index e53e54b78..000000000 --- a/readthedocs.yml +++ /dev/null @@ -1,2 +0,0 @@ -# Don't build any extra formats -formats: [] \ No newline at end of file From fe8922bdc3e037863f575fd67a1c182133ed3a93 Mon Sep 17 00:00:00 2001 From: psi29a Date: Tue, 2 Nov 2021 10:13:34 +0000 Subject: [PATCH 2/3] Merge branch 'rtd' into 'master' Fix readthedocs config, second attempt. See merge request OpenMW/openmw!1342 (cherry picked from commit 26ee2d284ecbad20247385805f8baeea2e3bfeb6) 20f851b3 Fix readthedocs config, second attempt. --- docs/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/requirements.txt b/docs/requirements.txt index 14c09d53e..ac82149f5 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,2 +1,3 @@ parse_cmake sphinx==1.8.5 +docutils==0.17.1 From 7f1e5b368e5bb2f7b500076ca0dccc29267f0075 Mon Sep 17 00:00:00 2001 From: elsid Date: Wed, 24 Nov 2021 19:10:02 +0100 Subject: [PATCH 3/3] Fix deadlock in physics system * Reorder unlock and notify_all calls to avoid notifying when not all worker threads are waiting. * Make sure main thread does not attempt to exclusively lock mSimulationMutex while not all workers are done with previous frame. * Replace mNewFrame flag by counter to avoid modification from multiple threads. --- apps/openmw/mwphysics/mtphysics.cpp | 38 +++++++++++++++++++++++------ apps/openmw/mwphysics/mtphysics.hpp | 7 +++++- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/apps/openmw/mwphysics/mtphysics.cpp b/apps/openmw/mwphysics/mtphysics.cpp index 29d1e0b7c..5a7f5f044 100644 --- a/apps/openmw/mwphysics/mtphysics.cpp +++ b/apps/openmw/mwphysics/mtphysics.cpp @@ -138,7 +138,7 @@ namespace MWPhysics , mRemainingSteps(0) , mLOSCacheExpiry(Settings::Manager::getInt("lineofsight keep inactive cache", "Physics")) , mDeferAabbUpdate(Settings::Manager::getBool("defer aabb update", "Physics")) - , mNewFrame(false) + , mFrameCounter(0) , mAdvanceSimulation(false) , mQuit(false) , mNextJob(0) @@ -176,12 +176,13 @@ namespace MWPhysics PhysicsTaskScheduler::~PhysicsTaskScheduler() { + waitForWorkers(); std::unique_lock lock(mSimulationMutex); mQuit = true; mNumJobs = 0; mRemainingSteps = 0; - lock.unlock(); mHasJob.notify_all(); + lock.unlock(); for (auto& thread : mThreads) thread.join(); } @@ -234,9 +235,10 @@ namespace MWPhysics const std::vector& PhysicsTaskScheduler::moveActors(float & timeAccum, std::vector&& actorsData, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats) { + waitForWorkers(); + // This function run in the main thread. // While the mSimulationMutex is held, background physics threads can't run. - std::unique_lock lock(mSimulationMutex); double timeStart = mTimer->tick(); @@ -282,7 +284,7 @@ namespace MWPhysics mPhysicsDt = newDelta; mActorsFrameData = std::move(actorsData); mAdvanceSimulation = (mRemainingSteps != 0); - mNewFrame = true; + ++mFrameCounter; mNumJobs = mActorsFrameData.size(); mNextLOS.store(0, std::memory_order_relaxed); mNextJob.store(0, std::memory_order_release); @@ -302,8 +304,8 @@ namespace MWPhysics } mAsyncStartTime = mTimer->tick(); - lock.unlock(); mHasJob.notify_all(); + lock.unlock(); if (mAdvanceSimulation) mBudget.update(mTimer->delta_s(timeStart, mTimer->tick()), 1, mBudgetCursor); return mMovedActors; @@ -311,6 +313,7 @@ namespace MWPhysics const std::vector& PhysicsTaskScheduler::resetSimulation(const ActorMap& actors) { + waitForWorkers(); std::unique_lock lock(mSimulationMutex); mBudget.reset(mDefaultPhysicsDt); mAsyncBudget.reset(0.0f); @@ -477,11 +480,13 @@ namespace MWPhysics void PhysicsTaskScheduler::worker() { + std::size_t lastFrame = 0; std::shared_lock lock(mSimulationMutex); while (!mQuit) { - if (!mNewFrame) - mHasJob.wait(lock, [&]() { return mQuit || mNewFrame; }); + if (mRemainingSteps == 0 && lastFrame == mFrameCounter) + mHasJob.wait(lock, [&] { return mQuit || lastFrame != mFrameCounter; }); + lastFrame = mFrameCounter; mPreStepBarrier->wait([this] { afterPreStep(); }); @@ -618,7 +623,6 @@ namespace MWPhysics void PhysicsTaskScheduler::afterPostSim() { - mNewFrame = false; if (mLOSCacheExpiry >= 0) { std::unique_lock lock(mLOSCacheMutex); @@ -628,5 +632,23 @@ namespace MWPhysics mLOSCache.end()); } mTimeEnd = mTimer->tick(); + std::unique_lock lock(mWorkersDoneMutex); + ++mWorkersFrameCounter; + mWorkersDone.notify_all(); + } + + // Attempt to acquire unique lock on mSimulationMutex while not all worker + // threads are holding shared lock but will have to may lead to a deadlock because + // C++ standard does not guarantee priority for exclusive and shared locks + // for std::shared_mutex. For example microsoft STL implementation points out + // for the absence of such priority: + // https://docs.microsoft.com/en-us/windows/win32/sync/slim-reader-writer--srw--locks + void PhysicsTaskScheduler::waitForWorkers() + { + if (mNumThreads == 0) + return; + std::unique_lock lock(mWorkersDoneMutex); + if (mFrameCounter != mWorkersFrameCounter) + mWorkersDone.wait(lock); } } diff --git a/apps/openmw/mwphysics/mtphysics.hpp b/apps/openmw/mwphysics/mtphysics.hpp index 3fd4d5a69..0c4402579 100644 --- a/apps/openmw/mwphysics/mtphysics.hpp +++ b/apps/openmw/mwphysics/mtphysics.hpp @@ -69,6 +69,7 @@ namespace MWPhysics void afterPreStep(); void afterPostStep(); void afterPostSim(); + void waitForWorkers(); std::unique_ptr mWorldFrameData; std::vector mActorsFrameData; @@ -91,7 +92,7 @@ namespace MWPhysics int mRemainingSteps; int mLOSCacheExpiry; bool mDeferAabbUpdate; - bool mNewFrame; + std::size_t mFrameCounter; bool mAdvanceSimulation; bool mThreadSafeBullet; bool mQuit; @@ -99,6 +100,10 @@ namespace MWPhysics std::atomic mNextLOS; std::vector mThreads; + std::size_t mWorkersFrameCounter = 0; + std::condition_variable mWorkersDone; + std::mutex mWorkersDoneMutex; + mutable std::shared_mutex mSimulationMutex; mutable std::shared_mutex mCollisionWorldMutex; mutable std::shared_mutex mLOSCacheMutex;