From fe37cb999055c0df27a3fc1e038013575a63e2ea Mon Sep 17 00:00:00 2001 From: Robert Adams Date: Sun, 9 Aug 2015 15:36:50 -0700 Subject: BulletSim: rearrange code and add different locking to eliminate chances of race conditions and, especially, race conditions when an object is removed and quickly re-added to a scene. This hopefully reduces the occurance of problems when avatars TP within a region -- the main problem being the loss of collisions. --- .../Region/Physics/BulletSPlugin/BSCharacter.cs | 4 +- .../Region/Physics/BulletSPlugin/BSPhysObject.cs | 28 ++++++------ OpenSim/Region/Physics/BulletSPlugin/BSScene.cs | 51 +++++++++++----------- 3 files changed, 40 insertions(+), 43 deletions(-) diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs b/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs index d0cc1eb..9c3f160 100644 --- a/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs @@ -459,7 +459,7 @@ public sealed class BSCharacter : BSPhysObject RawVelocity = value; OMV.Vector3 vel = RawVelocity; - DetailLog("{0}: set Velocity = {1}", LogHeader, value); + DetailLog("{0}: set Velocity = {1}", LocalID, value); PhysScene.TaintedObject(LocalID, "BSCharacter.setVelocity", delegate() { @@ -477,7 +477,7 @@ public sealed class BSCharacter : BSPhysObject set { PhysScene.AssertInTaintTime("BSCharacter.ForceVelocity"); // Util.PrintCallStack(); - DetailLog("{0}: set ForceVelocity = {1}", LogHeader, value); + DetailLog("{0}: set ForceVelocity = {1}", LocalID, value); RawVelocity = value; PhysScene.PE.SetLinearVelocity(PhysBody, RawVelocity); diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs b/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs index b758408..90da7a6 100755 --- a/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs @@ -80,12 +80,13 @@ public abstract class BSPhysObject : PhysicsActor Name = name; // PhysicsActor also has the name of the object. Someday consolidate. TypeName = typeName; - // The collection of things that push me around - PhysicalActors = new BSActorCollection(PhysScene); + // Oddity if object is destroyed and recreated very quickly it could still have the old body. + if (!PhysBody.HasPhysicalBody) + PhysBody = new BulletBody(localID); - // We don't have any physical representation yet. - PhysBody = new BulletBody(localID); - PhysShape = new BSShapeNull(); + // Clean out anything that might be in the physical actor list. + // Again, a workaround for destroying and recreating an object very quickly. + PhysicalActors.Dispose(); UserSetCenterOfMassDisplacement = null; @@ -100,9 +101,6 @@ public abstract class BSPhysObject : PhysicsActor // Default material type. Also sets Friction, Restitution and Density. SetMaterial((int)MaterialAttributes.Material.Wood); - CollisionCollection = new CollisionEventUpdate(); - CollisionsLastReported = CollisionCollection; - CollisionsLastTick = new CollisionEventUpdate(); CollisionsLastTickStep = -1; SubscribedEventsMs = 0; @@ -158,9 +156,9 @@ public abstract class BSPhysObject : PhysicsActor public OMV.Vector3 Inertia { get; set; } // Reference to the physical body (btCollisionObject) of this object - public BulletBody PhysBody; + public BulletBody PhysBody = new BulletBody(0); // Reference to the physical shape (btCollisionShape) of this object - public BSShape PhysShape; + public BSShape PhysShape = new BSShapeNull(); // The physical representation of the prim might require an asset fetch. // The asset state is first 'Unknown' then 'Waiting' then either 'Failed' or 'Fetched'. @@ -445,12 +443,12 @@ public abstract class BSPhysObject : PhysicsActor } // The collisions that have been collected for the next collision reporting (throttled by subscription) - protected CollisionEventUpdate CollisionCollection; + protected CollisionEventUpdate CollisionCollection = new CollisionEventUpdate(); // This is the collision collection last reported to the Simulator. - public CollisionEventUpdate CollisionsLastReported; + public CollisionEventUpdate CollisionsLastReported = new CollisionEventUpdate(); // Remember the collisions recorded in the last tick for fancy collision checking // (like a BSCharacter walking up stairs). - public CollisionEventUpdate CollisionsLastTick; + public CollisionEventUpdate CollisionsLastTick = new CollisionEventUpdate(); private long CollisionsLastTickStep = -1; // The simulation step is telling this object about a collision. @@ -495,7 +493,7 @@ public abstract class BSPhysObject : PhysicsActor { CollisionCollection.AddCollider(collidingWith, new ContactPoint(contactPoint, contactNormal, pentrationDepth)); } - DetailLog("{0},{1}.Collison.AddCollider,call,with={2},point={3},normal={4},depth={5},colliderMoving={6}", + DetailLog("{0},{1}.Collision.AddCollider,call,with={2},point={3},normal={4},depth={5},colliderMoving={6}", LocalID, TypeName, collidingWith, contactPoint, contactNormal, pentrationDepth, ColliderIsMoving); ret = true; @@ -596,7 +594,7 @@ public abstract class BSPhysObject : PhysicsActor #region Per Simulation Step actions - public BSActorCollection PhysicalActors; + public BSActorCollection PhysicalActors = new BSActorCollection(); // When an update to the physical properties happens, this event is fired to let // different actors to modify the update before it is passed around diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs b/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs index f8e8f57..1a95f91 100644 --- a/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs @@ -76,7 +76,8 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters // Keep track of all the avatars so we can send them a collision event // every tick so OpenSim will update its animation. - private HashSet m_avatars = new HashSet(); + private HashSet AvatarsInScene = new HashSet(); + private Object AvatarsInSceneLock = new Object(); // let my minuions use my logger public ILog Logger { get { return m_log; } } @@ -425,11 +426,14 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters // make sure no stepping happens while we're deleting stuff m_initialized = false; - foreach (KeyValuePair kvp in PhysObjects) + lock (PhysObjects) { - kvp.Value.Destroy(); + foreach (KeyValuePair kvp in PhysObjects) + { + kvp.Value.Destroy(); + } + PhysObjects.Clear(); } - PhysObjects.Clear(); // Now that the prims are all cleaned up, there should be no constraints left if (Constraints != null) @@ -480,15 +484,8 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters // TODO: Remove kludge someday. // We must generate a collision for avatars whether they collide or not. // This is required by OpenSim to update avatar animations, etc. - lock (m_avatars) - { - // The funky copy is because this list has few and infrequent changes but is - // read zillions of times. This allows the reader/iterator to use the - // list and this creates a new list with any updates. - HashSet avatarTemp = new HashSet(m_avatars); - avatarTemp.Add(actor); - m_avatars = avatarTemp; - } + lock (AvatarsInSceneLock) + AvatarsInScene.Add(actor); return actor; } @@ -507,12 +504,8 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters lock (PhysObjects) PhysObjects.Remove(bsactor.LocalID); // Remove kludge someday - lock (m_avatars) - { - HashSet avatarTemp = new HashSet(m_avatars); - avatarTemp.Remove(bsactor); - m_avatars = avatarTemp; - } + lock (AvatarsInSceneLock) + AvatarsInScene.Remove(bsactor); } catch (Exception e) { @@ -757,13 +750,18 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters // The simulator expects collisions for avatars even if there are have been no collisions. // The event updates avatar animations and stuff. // If you fix avatar animation updates, remove this overhead and let normal collision processing happen. - // Note that we copy the root of the list to search. Any updates will create a new list - // thus freeing this code from having to do an extra lock for every collision. - HashSet avatarTemp = m_avatars; - foreach (BSPhysObject bsp in avatarTemp) - if (!ObjectsWithCollisions.Contains(bsp)) // don't call avatars twice - bsp.SendCollisions(); - avatarTemp = null; + // Note that we get a copy of the list to search because SendCollision() can take a while. + HashSet tempAvatarsInScene; + lock (AvatarsInSceneLock) + { + tempAvatarsInScene = new HashSet(AvatarsInScene); + } + foreach (BSPhysObject actor in tempAvatarsInScene) + { + if (!ObjectsWithCollisions.Contains(actor)) // don't call avatars twice + actor.SendCollisions(); + } + tempAvatarsInScene = null; // Objects that are done colliding are removed from the ObjectsWithCollisions list. // Not done above because it is inside an iteration of ObjectWithCollisions. @@ -813,6 +811,7 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters } BSPhysObject collider; + // NOTE that PhysObjects was locked before the call to SendCollision(). if (!PhysObjects.TryGetValue(localID, out collider)) { // If the object that is colliding cannot be found, just ignore the collision. -- cgit v1.1