From 34afed6792ef8a31532daebee86d168a2c3c135f Mon Sep 17 00:00:00 2001
From: amzn-sean <75276488+amzn-sean@users.noreply.github.com>
Date: Thu, 5 Aug 2021 14:27:31 +0100
Subject: [PATCH] fixed Ragdoll component can crash on deactivate (#2834)

fixes #2650
The root of the crash was connecting/disconnecting to the AZ::Event SceneSimulationStart when not on the main thread, as that is not thread safe.

The connection/disconnection was originally handled from Enable/EnableQueued and Disable/DisabledQueued which can be called from other threads within EmotionFX. I've moved the connection/disconnection to the Constructor / destructor, as the handler is responsible for executing the queued enable/disable actions and it makes sense to have that connection happen external to the Enable/disable path.

Signed-off-by: amzn-sean 75276488+amzn-sean@users.noreply.github.com
---
 .../Source/PhysXCharacters/API/Ragdoll.cpp    | 30 ++++---------------
 .../Code/Source/PhysXCharacters/API/Ragdoll.h |  1 -
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp
index 5d3f6bf0e1..038a1e2ee7 100644
--- a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp
+++ b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.cpp
@@ -36,9 +36,6 @@ namespace PhysX
         }
     } // namespace Internal
 
-    // PhysX::Ragdoll
-    /*static*/ AZStd::mutex Ragdoll::m_sceneEventMutex;
-
     void Ragdoll::Reflect(AZ::ReflectContext* context)
     {
         AZ::SerializeContext* serializeContext = azrtti_cast<AZ::SerializeContext*>(context);
@@ -109,14 +106,15 @@ namespace PhysX
             })
     {
         m_sceneOwner = sceneHandle;
+        if (auto* sceneInterface = AZ::Interface<AzPhysics::SceneInterface>::Get())
+        {
+            sceneInterface->RegisterSceneSimulationStartHandler(m_sceneOwner, m_sceneStartSimHandler);
+        }
     }
 
     Ragdoll::~Ragdoll()
     {
-        {
-            AZStd::scoped_lock lock(m_sceneEventMutex);
-            m_sceneStartSimHandler.Disconnect();
-        }
+        m_sceneStartSimHandler.Disconnect();
 
         m_nodes.clear(); //the nodes destructor will remove the simulated body from the scene.
     }
@@ -214,13 +212,6 @@ namespace PhysX
             }
         }
 
-        // the handler is also connected in EnableSimulationQueued(),
-        // which will call this function, so if called from that path dont connect here.
-        if (!m_sceneStartSimHandler.IsConnected()) 
-        {
-            AZStd::scoped_lock lock(m_sceneEventMutex);
-            sceneInterface->RegisterSceneSimulationStartHandler(m_sceneOwner, m_sceneStartSimHandler);
-        }
         sceneInterface->EnableSimulationOfBody(m_sceneOwner, m_bodyHandle);
     }
 
@@ -231,12 +222,6 @@ namespace PhysX
             return;
         }
 
-        if (auto* sceneInterface = AZ::Interface<AzPhysics::SceneInterface>::Get())
-        {
-            AZStd::scoped_lock lock(m_sceneEventMutex);
-            sceneInterface->RegisterSceneSimulationStartHandler(m_sceneOwner, m_sceneStartSimHandler);
-        }
-
         m_queuedInitialState = initialState;
     }
 
@@ -253,11 +238,6 @@ namespace PhysX
             return;
         }
 
-        {
-            AZStd::scoped_lock lock(m_sceneEventMutex);
-            m_sceneStartSimHandler.Disconnect();
-        }
-
         physx::PxScene* pxScene = Internal::GetPxScene(m_sceneOwner);
         const size_t numNodes = m_nodes.size();
 
diff --git a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h
index 94a39d9732..c9d0d5d661 100644
--- a/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h
+++ b/Gems/PhysX/Code/Source/PhysXCharacters/API/Ragdoll.h
@@ -81,6 +81,5 @@ namespace PhysX
         bool m_queuedDisableSimulation = false;
 
         AzPhysics::SceneEvents::OnSceneSimulationStartHandler m_sceneStartSimHandler;
-        static AZStd::mutex m_sceneEventMutex;
     };
 } // namespace PhysX