mirror/OpenMW
1
0
Fork 0
mirror of https://gitlab.com/OpenMW/openmw.git synced 2025-01-26 09:35:28 +00:00
Code Issues Projects Releases Wiki Activity

Merge branch 'fix_simulation_crash' into 'master'

Browse Source 
Fix use after free and possible deadlock on exit (#6577)

Closes #6577

See merge request OpenMW/openmw!1601
This commit is contained in:
psi29a 2022-02-02 21:06:18 +00:00
commit bb6b031afd
2 changed files with 84 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

99
apps/openmw/mwphysics/mtphysics.cpp
View File

@ -1,3 +1,5 @@
#include <functional>
#include <BulletCollision/BroadphaseCollision/btDbvtBroadphase.h> #include <BulletCollision/BroadphaseCollision/btDbvtBroadphase.h>
#include <BulletCollision/CollisionShapes/btCollisionShape.h> #include <BulletCollision/CollisionShapes/btCollisionShape.h>
@ -111,17 +113,49 @@ namespace
return ptr.getPosition() * interpolationFactor + ptr.getPreviousPosition() * (1.f - interpolationFactor); return ptr.getPosition() * interpolationFactor + ptr.getPreviousPosition() * (1.f - interpolationFactor);
} }
using LockedActorSimulation = std::pair<
std::shared_ptr<MWPhysics::Actor>,
std::reference_wrapper<MWPhysics::ActorFrameData>
>;
using LockedProjectileSimulation = std::pair<
std::shared_ptr<MWPhysics::Projectile>,
std::reference_wrapper<MWPhysics::ProjectileFrameData>
>;
namespace Visitors namespace Visitors
{ {
template <class Impl, template <class> class Lock>
struct WithLockedPtr
{
const Impl& mImpl;
std::shared_mutex& mCollisionWorldMutex;
const int mNumJobs;
template <class Ptr, class FrameData>
void operator()(MWPhysics::SimulationImpl<Ptr, FrameData>& sim) const
{
auto locked = sim.lock();
if (!locked.has_value())
return;
auto&& [ptr, frameData] = *std::move(locked);
// Locked shared_ptr has to be destructed after releasing mCollisionWorldMutex to avoid
// possible deadlock. Ptr destructor also acquires mCollisionWorldMutex.
const std::pair arg(std::move(ptr), frameData);
const Lock<std::shared_mutex> lock(mCollisionWorldMutex, mNumJobs);
mImpl(arg);
}
};
struct InitPosition struct InitPosition
{ {
const btCollisionWorld* mCollisionWorld; const btCollisionWorld* mCollisionWorld;
void operator()(MWPhysics::ActorSimulation& sim) const void operator()(MWPhysics::ActorSimulation& sim) const
{ {
auto& [actorPtr, frameData] = sim; auto locked = sim.lock();
const auto actor = actorPtr.lock(); if (!locked.has_value())
if (actor == nullptr)
return; return;
auto& [actor, frameDataRef] = *locked;
auto& frameData = frameDataRef.get();
actor->applyOffsetChange(); actor->applyOffsetChange();
frameData.mPosition = actor->getPosition(); frameData.mPosition = actor->getPosition();
if (frameData.mWaterCollision && frameData.mPosition.z() < frameData.mWaterlevel && actor->canMoveToWaterSurface(frameData.mWaterlevel, mCollisionWorld)) if (frameData.mWaterCollision && frameData.mPosition.z() < frameData.mWaterlevel && actor->canMoveToWaterSurface(frameData.mWaterlevel, mCollisionWorld))
@ -138,7 +172,7 @@ namespace
frameData.mStuckFrames = actor->getStuckFrames(); frameData.mStuckFrames = actor->getStuckFrames();
frameData.mLastStuckPosition = actor->getLastStuckPosition(); frameData.mLastStuckPosition = actor->getLastStuckPosition();
} }
void operator()(MWPhysics::ProjectileSimulation& sim) const void operator()(MWPhysics::ProjectileSimulation& /*sim*/) const
{ {
} }
}; };
@ -146,11 +180,11 @@ namespace
struct PreStep struct PreStep
{ {
btCollisionWorld* mCollisionWorld; btCollisionWorld* mCollisionWorld;
void operator()(MWPhysics::ActorSimulation& sim) const void operator()(const LockedActorSimulation& sim) const
{ {
MWPhysics::MovementSolver::unstuck(sim.second, mCollisionWorld); MWPhysics::MovementSolver::unstuck(sim.second, mCollisionWorld);
} }
void operator()(MWPhysics::ProjectileSimulation& sim) const void operator()(const LockedProjectileSimulation& /*sim*/) const
{ {
} }
}; };
@ -158,12 +192,10 @@ namespace
struct UpdatePosition struct UpdatePosition
{ {
btCollisionWorld* mCollisionWorld; btCollisionWorld* mCollisionWorld;
void operator()(MWPhysics::ActorSimulation& sim) const void operator()(const LockedActorSimulation& sim) const
{ {
auto& [actorPtr, frameData] = sim; auto& [actor, frameDataRef] = sim;
const auto actor = actorPtr.lock(); auto& frameData = frameDataRef.get();
if (actor == nullptr)
return;
if (actor->setPosition(frameData.mPosition)) if (actor->setPosition(frameData.mPosition))
{ {
frameData.mPosition = actor->getPosition(); // account for potential position change made by script frameData.mPosition = actor->getPosition(); // account for potential position change made by script
@ -171,12 +203,10 @@ namespace
mCollisionWorld->updateSingleAabb(actor->getCollisionObject()); mCollisionWorld->updateSingleAabb(actor->getCollisionObject());
} }
} }
void operator()(MWPhysics::ProjectileSimulation& sim) const void operator()(const LockedProjectileSimulation& sim) const
{ {
auto& [projPtr, frameData] = sim; auto& [proj, frameDataRef] = sim;
const auto proj = projPtr.lock(); auto& frameData = frameDataRef.get();
if (proj == nullptr)
return;
proj->setPosition(frameData.mPosition); proj->setPosition(frameData.mPosition);
proj->updateCollisionObjectPosition(); proj->updateCollisionObjectPosition();
mCollisionWorld->updateSingleAabb(proj->getCollisionObject()); mCollisionWorld->updateSingleAabb(proj->getCollisionObject());
@ -188,11 +218,11 @@ namespace
const float mPhysicsDt; const float mPhysicsDt;
const btCollisionWorld* mCollisionWorld; const btCollisionWorld* mCollisionWorld;
const MWPhysics::WorldFrameData& mWorldFrameData; const MWPhysics::WorldFrameData& mWorldFrameData;
void operator()(MWPhysics::ActorSimulation& sim) const void operator()(const LockedActorSimulation& sim) const
{ {
MWPhysics::MovementSolver::move(sim.second, mPhysicsDt, mCollisionWorld, mWorldFrameData); MWPhysics::MovementSolver::move(sim.second, mPhysicsDt, mCollisionWorld, mWorldFrameData);
} }
void operator()(MWPhysics::ProjectileSimulation& sim) const void operator()(const LockedProjectileSimulation& sim) const
{ {
MWPhysics::MovementSolver::move(sim.second, mPhysicsDt, mCollisionWorld); MWPhysics::MovementSolver::move(sim.second, mPhysicsDt, mCollisionWorld);
} }
@ -206,10 +236,11 @@ namespace
const MWPhysics::PhysicsTaskScheduler* scheduler; const MWPhysics::PhysicsTaskScheduler* scheduler;
void operator()(MWPhysics::ActorSimulation& sim) const void operator()(MWPhysics::ActorSimulation& sim) const
{ {
auto& [actorPtr, frameData] = sim; auto locked = sim.lock();
const auto actor = actorPtr.lock(); if (!locked.has_value())
if (actor == nullptr)
return; return;
auto& [actor, frameDataRef] = *locked;
auto& frameData = frameDataRef.get();
auto ptr = actor->getPtr(); auto ptr = actor->getPtr();
MWMechanics::CreatureStats& stats = ptr.getClass().getCreatureStats(ptr); MWMechanics::CreatureStats& stats = ptr.getClass().getCreatureStats(ptr);
@ -241,10 +272,10 @@ namespace
} }
void operator()(MWPhysics::ProjectileSimulation& sim) const void operator()(MWPhysics::ProjectileSimulation& sim) const
{ {
auto& [projPtr, frameData] = sim; auto locked = sim.lock();
const auto proj = projPtr.lock(); if (!locked.has_value())
if (proj == nullptr)
return; return;
auto& [proj, frameData] = *locked;
proj->setSimulationPosition(::interpolateMovements(*proj, mTimeAccum, mPhysicsDt)); proj->setSimulationPosition(::interpolateMovements(*proj, mTimeAccum, mPhysicsDt));
} }
}; };
@ -612,12 +643,10 @@ namespace MWPhysics
void PhysicsTaskScheduler::updateActorsPositions() void PhysicsTaskScheduler::updateActorsPositions()
{ {
const Visitors::UpdatePosition vis{mCollisionWorld}; const Visitors::UpdatePosition impl{mCollisionWorld};
for (auto& sim : mSimulations) const Visitors::WithLockedPtr<Visitors::UpdatePosition, MaybeExclusiveLock> vis{impl, mCollisionWorldMutex, mNumThreads};
{ for (Simulation& sim : mSimulations)
MaybeExclusiveLock lock(mCollisionWorldMutex, mNumThreads);
std::visit(vis, sim); std::visit(vis, sim);
}
} }
bool PhysicsTaskScheduler::hasLineOfSight(const Actor* actor1, const Actor* actor2) bool PhysicsTaskScheduler::hasLineOfSight(const Actor* actor1, const Actor* actor2)
@ -641,12 +670,10 @@ namespace MWPhysics
{ {
mPreStepBarrier->wait([this] { afterPreStep(); }); mPreStepBarrier->wait([this] { afterPreStep(); });
int job = 0; int job = 0;
const Visitors::Move vis{mPhysicsDt, mCollisionWorld, *mWorldFrameData}; const Visitors::Move impl{mPhysicsDt, mCollisionWorld, *mWorldFrameData};
const Visitors::WithLockedPtr<Visitors::Move, MaybeLock> vis{impl, mCollisionWorldMutex, mNumThreads};
while ((job = mNextJob.fetch_add(1, std::memory_order_relaxed)) < mNumJobs) while ((job = mNextJob.fetch_add(1, std::memory_order_relaxed)) < mNumJobs)
{
MaybeLock lockColWorld(mCollisionWorldMutex, mNumThreads);
std::visit(vis, mSimulations[job]); std::visit(vis, mSimulations[job]);
}
mPostStepBarrier->wait([this] { afterPostStep(); }); mPostStepBarrier->wait([this] { afterPostStep(); });
} }
@ -697,12 +724,10 @@ namespace MWPhysics
updateAabbs(); updateAabbs();
if (!mRemainingSteps) if (!mRemainingSteps)
return; return;
const Visitors::PreStep vis{mCollisionWorld}; const Visitors::PreStep impl{mCollisionWorld};
const Visitors::WithLockedPtr<Visitors::PreStep, MaybeExclusiveLock> vis{impl, mCollisionWorldMutex, mNumThreads};
for (auto& sim : mSimulations) for (auto& sim : mSimulations)
{
MaybeExclusiveLock lock(mCollisionWorldMutex, mNumThreads);
std::visit(vis, sim); std::visit(vis, sim);
}
} }
void PhysicsTaskScheduler::afterPostStep() void PhysicsTaskScheduler::afterPostStep()

24
apps/openmw/mwphysics/physicssystem.hpp
View File

@ -8,6 +8,8 @@
#include <unordered_map> #include <unordered_map>
#include <algorithm> #include <algorithm>
#include <variant> #include <variant>
#include <optional>
#include <functional>
#include <osg/Quat> #include <osg/Quat>
#include <osg/BoundingBox> #include <osg/BoundingBox>
@ -117,8 +119,26 @@ namespace MWPhysics
osg::Vec3f mStormDirection; osg::Vec3f mStormDirection;
}; };
using ActorSimulation = std::pair<std::weak_ptr<Actor>, ActorFrameData>; template <class Ptr, class FrameData>
using ProjectileSimulation = std::pair<std::weak_ptr<Projectile>, ProjectileFrameData>; class SimulationImpl
{
public:
explicit SimulationImpl(const std::weak_ptr<Ptr>& ptr, FrameData&& data) : mPtr(ptr), mData(data) {}
std::optional<std::pair<std::shared_ptr<Ptr>, std::reference_wrapper<FrameData>>> lock()
{
if (auto locked = mPtr.lock())
return {{std::move(locked), std::ref(mData)}};
return std::nullopt;
}
private:
std::weak_ptr<Ptr> mPtr;
FrameData mData;
};
using ActorSimulation = SimulationImpl<Actor, ActorFrameData>;
using ProjectileSimulation = SimulationImpl<Projectile, ProjectileFrameData>;
using Simulation = std::variant<ActorSimulation, ProjectileSimulation>; using Simulation = std::variant<ActorSimulation, ProjectileSimulation>;
class PhysicsSystem : public RayCastingInterface class PhysicsSystem : public RayCastingInterface