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.

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<BSPhysObject> m_avatars = new HashSet<BSPhysObject>();
+    private HashSet<BSPhysObject> AvatarsInScene = new HashSet<BSPhysObject>();
+    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<uint, BSPhysObject> kvp in PhysObjects)
+        lock (PhysObjects)
         {
-            kvp.Value.Destroy();
+            foreach (KeyValuePair<uint, BSPhysObject> 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<BSPhysObject> avatarTemp = new HashSet<BSPhysObject>(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<BSPhysObject> avatarTemp = new HashSet<BSPhysObject>(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<BSPhysObject> 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<BSPhysObject> tempAvatarsInScene;
+            lock (AvatarsInSceneLock)
+            {
+                tempAvatarsInScene = new HashSet<BSPhysObject>(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.