From 45c7789b54c4c04db908a0645ee58e7a285fcccd Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 15 Nov 2011 19:42:33 +0000 Subject: use a more efficient dictionary in OdeScene._collisionEventPrim rather than a list --- OpenSim/Region/Physics/OdePlugin/OdeScene.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'OpenSim/Region/Physics/OdePlugin/OdeScene.cs') diff --git a/OpenSim/Region/Physics/OdePlugin/OdeScene.cs b/OpenSim/Region/Physics/OdePlugin/OdeScene.cs index c3279c6..a4e5c1e 100644 --- a/OpenSim/Region/Physics/OdePlugin/OdeScene.cs +++ b/OpenSim/Region/Physics/OdePlugin/OdeScene.cs @@ -221,7 +221,7 @@ namespace OpenSim.Region.Physics.OdePlugin /// /// A list of actors that should receive collision events. /// - private readonly List _collisionEventPrim = new List(); + private readonly Dictionary _collisionEventPrim = new Dictionary(); private readonly HashSet _badCharacter = new HashSet(); public Dictionary geom_name_map = new Dictionary(); @@ -1636,10 +1636,7 @@ namespace OpenSim.Region.Physics.OdePlugin // m_log.DebugFormat("[PHYSICS]: Adding {0} to collision event reporting", obj.SOPName); lock (_collisionEventPrim) - { - if (!_collisionEventPrim.Contains(obj)) - _collisionEventPrim.Add(obj); - } + _collisionEventPrim[obj.LocalID] = obj; } /// @@ -1651,7 +1648,7 @@ namespace OpenSim.Region.Physics.OdePlugin // m_log.DebugFormat("[PHYSICS]: Removing {0} from collision event reporting", obj.SOPName); lock (_collisionEventPrim) - _collisionEventPrim.Remove(obj); + _collisionEventPrim.Remove(obj.LocalID); } #region Add/Remove Entities @@ -2792,7 +2789,7 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name); lock (_collisionEventPrim) { - foreach (PhysicsActor obj in _collisionEventPrim) + foreach (PhysicsActor obj in _collisionEventPrim.Values) { // m_log.DebugFormat("[PHYSICS]: Assessing {0} for collision events", obj.SOPName); -- cgit v1.1 From e16d7fe1da3432d819f72e8c2af420601009854b Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 15 Nov 2011 20:02:09 +0000 Subject: Instead of having scene add/remove collision events directly to the OdeScene collision event dictionary, marshall them via a change dictionary first. This is to avoid a complicated tri-thread deadlock on region crossing for avatars with attachments, where 1) XEngine starting up scripts can lock XEngine.m_Scripts and then try to lock OdeScene._collisionEventPrim while starting up a script due to avatar border crossing 2) An existing collision event will lock OdeScene._collisionEventPrim and then try to lock SP.m_attachments while trying to send the collision event to attachments 3) The avatar still entering the region will lock SP.m_attachments and then try to lock m_Scripts to start more attachment scripts. --- OpenSim/Region/Physics/OdePlugin/OdeScene.cs | 58 ++++++++++++++++++---------- 1 file changed, 38 insertions(+), 20 deletions(-) (limited to 'OpenSim/Region/Physics/OdePlugin/OdeScene.cs') diff --git a/OpenSim/Region/Physics/OdePlugin/OdeScene.cs b/OpenSim/Region/Physics/OdePlugin/OdeScene.cs index a4e5c1e..740037f 100644 --- a/OpenSim/Region/Physics/OdePlugin/OdeScene.cs +++ b/OpenSim/Region/Physics/OdePlugin/OdeScene.cs @@ -219,9 +219,14 @@ namespace OpenSim.Region.Physics.OdePlugin private readonly List _perloopContact = new List(); /// - /// A list of actors that should receive collision events. + /// A dictionary of actors that should receive collision events. /// private readonly Dictionary _collisionEventPrim = new Dictionary(); + + /// + /// A dictionary of collision event changes that are waiting to be processed. + /// + private readonly Dictionary _collisionEventPrimChanges = new Dictionary(); private readonly HashSet _badCharacter = new HashSet(); public Dictionary geom_name_map = new Dictionary(); @@ -1635,8 +1640,8 @@ namespace OpenSim.Region.Physics.OdePlugin { // m_log.DebugFormat("[PHYSICS]: Adding {0} to collision event reporting", obj.SOPName); - lock (_collisionEventPrim) - _collisionEventPrim[obj.LocalID] = obj; + lock (_collisionEventPrimChanges) + _collisionEventPrimChanges[obj.LocalID] = obj; } /// @@ -1647,8 +1652,8 @@ namespace OpenSim.Region.Physics.OdePlugin { // m_log.DebugFormat("[PHYSICS]: Removing {0} from collision event reporting", obj.SOPName); - lock (_collisionEventPrim) - _collisionEventPrim.Remove(obj.LocalID); + lock (_collisionEventPrimChanges) + _collisionEventPrimChanges[obj.LocalID] = null; } #region Add/Remove Entities @@ -2660,6 +2665,22 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name); // m_physicsiterations = 10; // } + // We change _collisionEventPrimChanges to avoid locking _collisionEventPrim itself and causing potential + // deadlock if the collision event tries to lock something else later on which is already locked by a + // caller that is adding or removing the collision event. + lock (_collisionEventPrimChanges) + { + foreach (KeyValuePair kvp in _collisionEventPrimChanges) + { + if (kvp.Value == null) + _collisionEventPrim.Remove(kvp.Key); + else + _collisionEventPrim[kvp.Key] = kvp.Value; + } + + _collisionEventPrimChanges.Clear(); + } + if (SupportsNINJAJoints) { DeleteRequestedJoints(); // this must be outside of the lock (OdeLock) to avoid deadlocks @@ -2787,25 +2808,22 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name); collision_optimized(); - lock (_collisionEventPrim) + foreach (PhysicsActor obj in _collisionEventPrim.Values) { - foreach (PhysicsActor obj in _collisionEventPrim.Values) - { // m_log.DebugFormat("[PHYSICS]: Assessing {0} for collision events", obj.SOPName); - switch ((ActorTypes)obj.PhysicsActorType) - { - case ActorTypes.Agent: - OdeCharacter cobj = (OdeCharacter)obj; - cobj.AddCollisionFrameTime(100); - cobj.SendCollisions(); - break; + switch ((ActorTypes)obj.PhysicsActorType) + { + case ActorTypes.Agent: + OdeCharacter cobj = (OdeCharacter)obj; + cobj.AddCollisionFrameTime(100); + cobj.SendCollisions(); + break; - case ActorTypes.Prim: - OdePrim pobj = (OdePrim)obj; - pobj.SendCollisions(); - break; - } + case ActorTypes.Prim: + OdePrim pobj = (OdePrim)obj; + pobj.SendCollisions(); + break; } } -- cgit v1.1 From b6d83e9c0f4bd1d450e36270278285be50d5ace8 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Wed, 16 Nov 2011 23:01:59 +0000 Subject: Stop OdePrim and OdeCharacter insanely overriding set LocalID to set their own private m_localID property but leaving get to return the then unset PhysicsActor.LocalId! Instead, just have both subclasses use the PhysicsActor.LocalID property. This restores collision functionality that fell away in 45c7789 yesterday --- OpenSim/Region/Physics/OdePlugin/OdeScene.cs | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'OpenSim/Region/Physics/OdePlugin/OdeScene.cs') diff --git a/OpenSim/Region/Physics/OdePlugin/OdeScene.cs b/OpenSim/Region/Physics/OdePlugin/OdeScene.cs index 740037f..43d852b 100644 --- a/OpenSim/Region/Physics/OdePlugin/OdeScene.cs +++ b/OpenSim/Region/Physics/OdePlugin/OdeScene.cs @@ -1306,8 +1306,8 @@ namespace OpenSim.Region.Physics.OdePlugin { case ActorTypes.Agent: cc1 = (OdeCharacter)p1; - obj2LocalID = cc1.m_localID; - cc1.AddCollisionEvent(cc2.m_localID, contact); + obj2LocalID = cc1.LocalID; + cc1.AddCollisionEvent(cc2.LocalID, contact); //ctype = (int)CollisionCategories.Character; //if (cc1.CollidingObj) @@ -1322,8 +1322,8 @@ namespace OpenSim.Region.Physics.OdePlugin if (p1 is OdePrim) { cp1 = (OdePrim) p1; - obj2LocalID = cp1.m_localID; - cp1.AddCollisionEvent(cc2.m_localID, contact); + obj2LocalID = cp1.LocalID; + cp1.AddCollisionEvent(cc2.LocalID, contact); } //ctype = (int)CollisionCategories.Geom; @@ -1359,8 +1359,8 @@ namespace OpenSim.Region.Physics.OdePlugin if (p1 is OdeCharacter) { cc1 = (OdeCharacter) p1; - obj2LocalID = cc1.m_localID; - cc1.AddCollisionEvent(cp2.m_localID, contact); + obj2LocalID = cc1.LocalID; + cc1.AddCollisionEvent(cp2.LocalID, contact); //ctype = (int)CollisionCategories.Character; //if (cc1.CollidingObj) @@ -1375,8 +1375,8 @@ namespace OpenSim.Region.Physics.OdePlugin if (p1 is OdePrim) { cp1 = (OdePrim) p1; - obj2LocalID = cp1.m_localID; - cp1.AddCollisionEvent(cp2.m_localID, contact); + obj2LocalID = cp1.LocalID; + cp1.AddCollisionEvent(cp2.LocalID, contact); //ctype = (int)CollisionCategories.Geom; //if (cp1.CollidingObj) @@ -1638,7 +1638,7 @@ namespace OpenSim.Region.Physics.OdePlugin /// internal void AddCollisionEventReporting(PhysicsActor obj) { -// m_log.DebugFormat("[PHYSICS]: Adding {0} to collision event reporting", obj.SOPName); +// m_log.DebugFormat("[PHYSICS]: Adding {0} {1} to collision event reporting", obj.SOPName, obj.LocalID); lock (_collisionEventPrimChanges) _collisionEventPrimChanges[obj.LocalID] = obj; @@ -1650,7 +1650,7 @@ namespace OpenSim.Region.Physics.OdePlugin /// internal void RemoveCollisionEventReporting(PhysicsActor obj) { -// m_log.DebugFormat("[PHYSICS]: Removing {0} from collision event reporting", obj.SOPName); +// m_log.DebugFormat("[PHYSICS]: Removing {0} {1} from collision event reporting", obj.SOPName, obj.LocalID); lock (_collisionEventPrimChanges) _collisionEventPrimChanges[obj.LocalID] = null; @@ -1754,9 +1754,7 @@ namespace OpenSim.Region.Physics.OdePlugin public override PhysicsActor AddPrimShape(string primName, PrimitiveBaseShape pbs, Vector3 position, Vector3 size, Quaternion rotation, bool isPhysical, uint localid) { -#if SPAM - m_log.DebugFormat("[PHYSICS]: Adding physics actor to {0}", primName); -#endif +// m_log.DebugFormat("[ODE SCENE]: Adding physics actor to {0} {1}", primName, localid); return AddPrim(primName, position, size, rotation, pbs, isPhysical, localid); } @@ -2810,7 +2808,7 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name); foreach (PhysicsActor obj in _collisionEventPrim.Values) { -// m_log.DebugFormat("[PHYSICS]: Assessing {0} for collision events", obj.SOPName); +// m_log.DebugFormat("[PHYSICS]: Assessing {0} {1} for collision events", obj.SOPName, obj.LocalID); switch ((ActorTypes)obj.PhysicsActorType) { @@ -3746,7 +3744,7 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name); { if (prm.CollisionScore > 0) { - returncolliders.Add(prm.m_localID, prm.CollisionScore); + returncolliders.Add(prm.LocalID, prm.CollisionScore); cnt++; prm.CollisionScore = 0f; if (cnt > 25) -- cgit v1.1