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') 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') 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/Manager/PhysicsActor.cs | 16 ++++++++----- OpenSim/Region/Physics/OdePlugin/ODECharacter.cs | 6 ----- OpenSim/Region/Physics/OdePlugin/ODEPrim.cs | 11 +-------- .../Physics/OdePlugin/ODERayCastRequestManager.cs | 7 +++--- OpenSim/Region/Physics/OdePlugin/OdeScene.cs | 28 ++++++++++------------ .../Region/Physics/OdePlugin/Tests/ODETestClass.cs | 2 +- 6 files changed, 29 insertions(+), 41 deletions(-) (limited to 'OpenSim/Region/Physics') diff --git a/OpenSim/Region/Physics/Manager/PhysicsActor.cs b/OpenSim/Region/Physics/Manager/PhysicsActor.cs index 49f60f8..798a0a4 100644 --- a/OpenSim/Region/Physics/Manager/PhysicsActor.cs +++ b/OpenSim/Region/Physics/Manager/PhysicsActor.cs @@ -25,8 +25,10 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +using log4net; using System; using System.Collections.Generic; +using System.Reflection; using OpenSim.Framework; using OpenMetaverse; @@ -46,10 +48,10 @@ namespace OpenSim.Region.Physics.Manager public enum PIDHoverType { - Ground - , GroundAndWater - , Water - , Absolute + Ground, + GroundAndWater, + Water, + Absolute } public struct ContactPoint @@ -114,6 +116,8 @@ namespace OpenSim.Region.Physics.Manager public abstract class PhysicsActor { +// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); + public delegate void RequestTerseUpdate(); public delegate void CollisionUpdate(EventArgs e); public delegate void OutOfBounds(Vector3 pos); @@ -197,10 +201,10 @@ namespace OpenSim.Region.Physics.Manager { CollisionUpdate handler = OnCollisionUpdate; +// m_log.DebugFormat("[PHYSICS ACTOR]: Sending collision for {0}", LocalID); + if (handler != null) - { handler(e); - } } public virtual void SetMaterial (int material) diff --git a/OpenSim/Region/Physics/OdePlugin/ODECharacter.cs b/OpenSim/Region/Physics/OdePlugin/ODECharacter.cs index 3630510..c37d588 100644 --- a/OpenSim/Region/Physics/OdePlugin/ODECharacter.cs +++ b/OpenSim/Region/Physics/OdePlugin/ODECharacter.cs @@ -108,7 +108,6 @@ namespace OpenSim.Region.Physics.OdePlugin /// private Vector3 m_taintForce; - internal uint m_localID = 0; // taints and their non-tainted counterparts private bool m_isPhysical = false; // the current physical status private bool m_tainted_isPhysical = false; // set when the physical status is tainted (false=not existing in physics engine, true=existing) @@ -231,11 +230,6 @@ namespace OpenSim.Region.Physics.OdePlugin set { m_alwaysRun = value; } } - public override uint LocalID - { - set { m_localID = value; } - } - public override bool Grabbed { set { return; } diff --git a/OpenSim/Region/Physics/OdePlugin/ODEPrim.cs b/OpenSim/Region/Physics/OdePlugin/ODEPrim.cs index 2f9a54b..1ba7ef7 100644 --- a/OpenSim/Region/Physics/OdePlugin/ODEPrim.cs +++ b/OpenSim/Region/Physics/OdePlugin/ODEPrim.cs @@ -142,8 +142,6 @@ namespace OpenSim.Region.Physics.OdePlugin public bool m_taintselected { get; private set; } public bool m_taintCollidesWater { get; private set; } - public uint m_localID { get; private set; } - private bool m_taintforce = false; private bool m_taintaddangularforce = false; private Vector3 m_force; @@ -290,13 +288,6 @@ namespace OpenSim.Region.Physics.OdePlugin set { return; } } - public override uint LocalID - { - set { - //m_log.Info("[PHYSICS]: Setting TrackerID: " + value); - m_localID = value; } - } - public override bool Grabbed { set { return; } @@ -1058,7 +1049,7 @@ Console.WriteLine("ZProcessTaints for " + Name); private void AddChildPrim(OdePrim prim) { //Console.WriteLine("AddChildPrim " + Name); - if (this.m_localID != prim.m_localID) + if (LocalID != prim.LocalID) { if (Body == IntPtr.Zero) { diff --git a/OpenSim/Region/Physics/OdePlugin/ODERayCastRequestManager.cs b/OpenSim/Region/Physics/OdePlugin/ODERayCastRequestManager.cs index 6c2bdde..8d7d3b3 100644 --- a/OpenSim/Region/Physics/OdePlugin/ODERayCastRequestManager.cs +++ b/OpenSim/Region/Physics/OdePlugin/ODERayCastRequestManager.cs @@ -371,12 +371,13 @@ namespace OpenSim.Region.Physics.OdePlugin // Loop over contacts, build results. for (int i = 0; i < count; i++) { - if (p1 != null) { + if (p1 != null) + { if (p1 is OdePrim) { ContactResult collisionresult = new ContactResult(); - collisionresult.ConsumerID = ((OdePrim)p1).m_localID; + collisionresult.ConsumerID = p1.LocalID; collisionresult.Pos = new Vector3(contacts[i].pos.X, contacts[i].pos.Y, contacts[i].pos.Z); collisionresult.Depth = contacts[i].depth; collisionresult.Normal = new Vector3(contacts[i].normal.X, contacts[i].normal.Y, @@ -392,7 +393,7 @@ namespace OpenSim.Region.Physics.OdePlugin { ContactResult collisionresult = new ContactResult(); - collisionresult.ConsumerID = ((OdePrim)p2).m_localID; + collisionresult.ConsumerID = p2.LocalID; collisionresult.Pos = new Vector3(contacts[i].pos.X, contacts[i].pos.Y, contacts[i].pos.Z); collisionresult.Depth = contacts[i].depth; collisionresult.Normal = new Vector3(contacts[i].normal.X, contacts[i].normal.Y, 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) diff --git a/OpenSim/Region/Physics/OdePlugin/Tests/ODETestClass.cs b/OpenSim/Region/Physics/OdePlugin/Tests/ODETestClass.cs index 2ea810f..cbc6b95 100644 --- a/OpenSim/Region/Physics/OdePlugin/Tests/ODETestClass.cs +++ b/OpenSim/Region/Physics/OdePlugin/Tests/ODETestClass.cs @@ -104,7 +104,7 @@ namespace OpenSim.Region.Physics.OdePlugin.Tests m_log.Info("TargetSpace: " + oprim.m_targetSpace + " - SceneMainSpace: " + pscene.space); Assert.That(!oprim.m_taintadd); - m_log.Info("Prim Position (" + oprim.m_localID + "): " + prim.Position.ToString()); + m_log.Info("Prim Position (" + oprim.LocalID + "): " + prim.Position); // Make sure we're above the ground //Assert.That(prim.Position.Z > 20f); -- cgit v1.1