Restore defects list. In hindsight, the reason for this is becuase we can't remove the character whilst iterating over the list.

This commit also removes locking on OdeScene._characters since code is single threaded

OpenSim/Region/Physics/OdePlugin/ODECharacter.cs

@@ -868,11 +868,10 @@ namespace OpenSim.Region.Physics.OdePlugin
         /// Called from Simulate
         /// This is the avatar's movement control + PID Controller
         /// </summary>
-        /// <remarks>
+        /// <param name="defects">The character will be added to this list if there is something wrong (non-finite
-        /// This routine will remove the character from the physics scene if it detects something wrong (non-finite
         /// position or velocity).
-        /// </remarks>
+        /// </param>
-        internal void Move()
+        internal void Move(List<OdeCharacter> defects)
         {
             //  no lock; for now it's only called from within Simulate()
             
@@ -897,8 +896,7 @@ namespace OpenSim.Region.Physics.OdePlugin
                     "[ODE CHARACTER]: Avatar position of {0} for {1} is non-finite!  Removing from physics scene.",
                     localPos, Name);
 
-                _parent_scene.RemoveCharacter(this);
+                defects.Add(this);
-                DestroyOdeStructures();
 
                 return;
             }
@@ -1037,15 +1035,19 @@ namespace OpenSim.Region.Physics.OdePlugin
                     "[ODE CHARACTER]: Got a NaN force vector {0} in Move() for {1}.  Removing character from physics scene.",
                     vec, Name);
 
-                _parent_scene.RemoveCharacter(this);
+                defects.Add(this);
-                DestroyOdeStructures();
+
+                return;
             }
         }
 
         /// <summary>
         /// Updates the reported position and velocity.  This essentially sends the data up to ScenePresence.
         /// </summary>
-        internal void UpdatePositionAndVelocity()
+        /// <param name="defects">The character will be added to this list if there is something wrong (non-finite
+        /// position or velocity).
+        /// </param>
+        internal void UpdatePositionAndVelocity(List<OdeCharacter> defects)
         {
             //  no lock; called from Simulate() -- if you call this from elsewhere, gotta lock or do Monitor.Enter/Exit!
             d.Vector3 newPos;
@@ -1056,11 +1058,12 @@ namespace OpenSim.Region.Physics.OdePlugin
             catch (NullReferenceException)
             {
                 bad = true;
-                _parent_scene.RemoveCharacter(this);
+                defects.Add(this);
-                DestroyOdeStructures();
                 newPos = new d.Vector3(_position.X, _position.Y, _position.Z);
                 base.RaiseOutOfBounds(_position); // Tells ScenePresence that there's a problem!
                 m_log.WarnFormat("[ODE CHARACTER]: Avatar Null reference for Avatar {0}, physical actor {1}", Name, m_uuid);
+
+                return;
             }
 
             //  kluge to keep things in bounds.  ODE lets dead avatars drift away (they should be removed!)
@@ -1140,7 +1143,7 @@ namespace OpenSim.Region.Physics.OdePlugin
         /// <summary>
         /// Used internally to destroy the ODE structures associated with this character.
         /// </summary>
-        private void DestroyOdeStructures()
+        internal void DestroyOdeStructures()
         {
             // destroy avatar capsule and related ODE data
             if (Amotor != IntPtr.Zero)

OpenSim/Region/Physics/OdePlugin/OdeScene.cs

@@ -189,7 +189,11 @@ namespace OpenSim.Region.Physics.OdePlugin
         public d.TriCallback triCallback;
         public d.TriArrayCallback triArrayCallback;
 
+        /// <summary>
+        /// Avatars in the physics scene.
+        /// </summary>
         private readonly HashSet<OdeCharacter> _characters = new HashSet<OdeCharacter>();
+        
         private readonly HashSet<OdePrim> _prims = new HashSet<OdePrim>();
         private readonly HashSet<OdePrim> _activeprims = new HashSet<OdePrim>();
 
@@ -251,6 +255,14 @@ namespace OpenSim.Region.Physics.OdePlugin
         /// </remarks>
         public Dictionary<IntPtr, PhysicsActor> actor_name_map = new Dictionary<IntPtr, PhysicsActor>();
 
+        /// <summary>
+        /// Defects list to remove characters that no longer have finite positions due to some other bug.
+        /// </summary>
+        /// <remarks>
+        /// Used repeatedly in Simulate() but initialized once here.
+        /// </remarks>
+        private readonly List<OdeCharacter> defects = new List<OdeCharacter>();
+
         private bool m_NINJA_physics_joints_enabled = false;
         //private Dictionary<String, IntPtr> jointpart_name_map = new Dictionary<String,IntPtr>();
         private readonly Dictionary<String, List<PhysicsJoint>> joints_connecting_actor = new Dictionary<String, List<PhysicsJoint>>();
@@ -1512,8 +1524,6 @@ namespace OpenSim.Region.Physics.OdePlugin
         {
             _perloopContact.Clear();
 
-            lock (_characters)
-            {
             foreach (OdeCharacter chr in _characters)
             {
                 // Reset the collision values to false
@@ -1551,7 +1561,6 @@ namespace OpenSim.Region.Physics.OdePlugin
                 //forcedZ = true;
                 //}
             }
-            }
 
             lock (_activeprims)
             {
@@ -1715,8 +1724,6 @@ namespace OpenSim.Region.Physics.OdePlugin
         }
 
         internal void AddCharacter(OdeCharacter chr)
-        {
-            lock (_characters)
         {
             if (!_characters.Contains(chr))
             {
@@ -1726,11 +1733,8 @@ namespace OpenSim.Region.Physics.OdePlugin
                     m_log.ErrorFormat("[PHYSICS] Added BAD actor {0} to characters list", chr.m_uuid);
             }
         }
-        }
 
         internal void RemoveCharacter(OdeCharacter chr)
-        {
-            lock (_characters)
         {
             if (_characters.Contains(chr))
             {
@@ -1739,7 +1743,6 @@ namespace OpenSim.Region.Physics.OdePlugin
                 actor_name_map.Remove(chr.Shell);
             }
         }
-        }
 
         private PhysicsActor AddPrim(String name, Vector3 position, Vector3 size, Quaternion rotation,
                                      PrimitiveBaseShape pbs, bool isphysical, uint localID)
@@ -2783,13 +2786,21 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name);
                         }
 
                         // Move characters
-                        lock (_characters)
-                        {
                         foreach (OdeCharacter actor in _characters)
                         {
                             if (actor != null)
-                                    actor.Move();
+                                actor.Move(defects);
                         }
+
+                        if (defects.Count != 0)
+                        {
+                            foreach (OdeCharacter actor in defects)
+                            {
+                                RemoveCharacter(actor);
+                                actor.DestroyOdeStructures();
+                            }
+
+                            defects.Clear();
                         }
 
                         // Move other active objects
@@ -2849,8 +2860,6 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name);
                     timeLeft -= ODE_STEPSIZE;
                 }
 
-                lock (_characters)
-                {
                 foreach (OdeCharacter actor in _characters)
                 {
                     if (actor != null)
@@ -2858,9 +2867,19 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name);
                         if (actor.bad)
                             m_log.WarnFormat("[PHYSICS]: BAD Actor {0} in _characters list was not removed?", actor.m_uuid);
 
-                            actor.UpdatePositionAndVelocity();
+                        actor.UpdatePositionAndVelocity(defects);
                     }
                 }
+
+                if (defects.Count != 0)
+                {
+                    foreach (OdeCharacter actor in defects)
+                    {
+                        RemoveCharacter(actor);
+                        actor.DestroyOdeStructures();
+                    }
+
+                    defects.Clear();
                 }
 
                 lock (_activeprims)
@@ -3882,8 +3901,7 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name);
                 }
             }
             ds.SetColor(1.0f, 0.0f, 0.0f);
-            lock (_characters)
+
-            {
             foreach (OdeCharacter chr in _characters)
             {
                 if (chr.Shell != IntPtr.Zero)
@@ -3908,7 +3926,6 @@ Console.WriteLine("AddPhysicsActorTaint to " + taintedprim.Name);
                 }
             }
         }
-        }
 
         public void start(int unused)
         {