diff --git a/apps/openmw/mwphysics/mtphysics.cpp b/apps/openmw/mwphysics/mtphysics.cpp index 5c49dee297..2f7e3b5995 100644 --- a/apps/openmw/mwphysics/mtphysics.cpp +++ b/apps/openmw/mwphysics/mtphysics.cpp @@ -267,7 +267,7 @@ namespace MWPhysics , mNumJobs(0) , mRemainingSteps(0) , mLOSCacheExpiry(Settings::Manager::getInt("lineofsight keep inactive cache", "Physics")) - , mNewFrame(false) + , mFrameCounter(0) , mAdvanceSimulation(false) , mQuit(false) , mNextJob(0) @@ -302,13 +302,14 @@ namespace MWPhysics PhysicsTaskScheduler::~PhysicsTaskScheduler() { + waitForWorkers(); { MaybeExclusiveLock lock(mSimulationMutex, mNumThreads); mQuit = true; mNumJobs = 0; mRemainingSteps = 0; + mHasJob.notify_all(); } - mHasJob.notify_all(); for (auto& thread : mThreads) thread.join(); } @@ -361,6 +362,8 @@ namespace MWPhysics void PhysicsTaskScheduler::applyQueuedMovements(float & timeAccum, std::vector&& simulations, 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. @@ -393,7 +396,7 @@ namespace MWPhysics mPhysicsDt = newDelta; mSimulations = std::move(simulations); mAdvanceSimulation = (mRemainingSteps != 0); - mNewFrame = true; + ++mFrameCounter; mNumJobs = mSimulations.size(); mNextLOS.store(0, std::memory_order_relaxed); mNextJob.store(0, std::memory_order_release); @@ -421,6 +424,7 @@ namespace MWPhysics void PhysicsTaskScheduler::resetSimulation(const ActorMap& actors) { + waitForWorkers(); MaybeExclusiveLock lock(mSimulationMutex, mNumThreads); mBudget.reset(mDefaultPhysicsDt); mAsyncBudget.reset(0.0f); @@ -577,11 +581,15 @@ 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 (lastFrame == mFrameCounter) + { + mHasJob.wait(lock, [&] { return mQuit || lastFrame != mFrameCounter; }); + lastFrame = mFrameCounter; + } doSimulation(); } @@ -663,6 +671,7 @@ namespace MWPhysics void PhysicsTaskScheduler::releaseSharedStates() { + waitForWorkers(); std::scoped_lock lock(mSimulationMutex, mUpdateAabbMutex); mSimulations.clear(); mUpdateAabb.clear(); @@ -693,7 +702,6 @@ namespace MWPhysics void PhysicsTaskScheduler::afterPostSim() { - mNewFrame = false; { MaybeExclusiveLock lock(mLOSCacheMutex, mNumThreads); mLOSCache.erase( @@ -702,6 +710,10 @@ namespace MWPhysics mLOSCache.end()); } mTimeEnd = mTimer->tick(); + + std::unique_lock lock(mWorkersDoneMutex); + ++mWorkersFrameCounter; + mWorkersDone.notify_all(); } void PhysicsTaskScheduler::syncWithMainThread() @@ -710,4 +722,19 @@ namespace MWPhysics for (auto& sim : mSimulations) std::visit(vis, sim); } + + // 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 44330b2cc6..9ded1262d6 100644 --- a/apps/openmw/mwphysics/mtphysics.hpp +++ b/apps/openmw/mwphysics/mtphysics.hpp @@ -74,6 +74,7 @@ namespace MWPhysics void afterPostStep(); void afterPostSim(); void syncWithMainThread(); + void waitForWorkers(); std::unique_ptr mWorldFrameData; std::vector mSimulations; @@ -95,13 +96,17 @@ namespace MWPhysics int mNumJobs; int mRemainingSteps; int mLOSCacheExpiry; - bool mNewFrame; + std::size_t mFrameCounter; bool mAdvanceSimulation; bool mQuit; std::atomic mNextJob; 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;