Improve liveness by operating on list copies of SOG.Children where appropriate

5 files changed, 84 insertions, 75 deletions

diff --git a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs
index 8ed8b96..2ac46e2 100644
--- a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs
+++ b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs
@@ -2004,11 +2004,12 @@ namespace OpenSim.Region.Framework.Scenes
                     sog.SetGroup(groupID, remoteClient);
                     sog.ScheduleGroupForFullUpdate();
 
+                    List<SceneObjectPart> partList = null;
                     lock (sog.Children)
-                    {
-                        foreach (SceneObjectPart child in sog.Children.Values)
-                            child.Inventory.ChangeInventoryOwner(ownerID);
-                    }
+                        partList = new List<SceneObjectPart>(sog.Children.Values);                    
+                    
+                    foreach (SceneObjectPart child in partList)
+                        child.Inventory.ChangeInventoryOwner(ownerID);
                 }
                 else
                 {
@@ -2017,14 +2018,15 @@ namespace OpenSim.Region.Framework.Scenes
 
                     if (sog.GroupID != groupID)
                         continue;
-
+                    
+                    List<SceneObjectPart> partList = null;
                     lock (sog.Children)
+                        partList = new List<SceneObjectPart>(sog.Children.Values);                    
+
+                    foreach (SceneObjectPart child in partList)
                     {
-                        foreach (SceneObjectPart child in sog.Children.Values)
-                        {
-                            child.LastOwnerID = child.OwnerID;
-                            child.Inventory.ChangeInventoryOwner(groupID);
-                        }
+                        child.LastOwnerID = child.OwnerID;
+                        child.Inventory.ChangeInventoryOwner(groupID);
                     }
 
                     sog.SetOwnerId(groupID);
diff --git a/OpenSim/Region/Framework/Scenes/Scene.PacketHandlers.cs b/OpenSim/Region/Framework/Scenes/Scene.PacketHandlers.cs
index 9f1575d..c511774 100644
--- a/OpenSim/Region/Framework/Scenes/Scene.PacketHandlers.cs
+++ b/OpenSim/Region/Framework/Scenes/Scene.PacketHandlers.cs
@@ -164,16 +164,17 @@ namespace OpenSim.Region.Framework.Scenes
                         
                         SceneObjectGroup sog = ent as SceneObjectGroup;
                         
+                        List<SceneObjectPart> partList = null;
                         lock (sog.Children)
+                            partList = new List<SceneObjectPart>(sog.Children.Values);    
+
+                        foreach (SceneObjectPart part in partList)
                         {
-                            foreach (KeyValuePair<UUID, SceneObjectPart> child in (sog.Children))
+                            if (part.LocalId == primLocalID) 
                             {
-                                if (child.Value.LocalId == primLocalID) 
-                                {
-                                    child.Value.GetProperties(remoteClient);
-                                    foundPrim = true;
-                                    break;
-                                }
+                                part.GetProperties(remoteClient);
+                                foundPrim = true;
+                                break;
                             }
                         }
                             
diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs
index 5fe944d..db081cc 100644
--- a/OpenSim/Region/Framework/Scenes/Scene.cs
+++ b/OpenSim/Region/Framework/Scenes/Scene.cs
@@ -2065,19 +2065,20 @@ namespace OpenSim.Region.Framework.Scenes
                 group.RemoveScriptInstances(true);
             }
 
+            List<SceneObjectPart> partList = null;
             lock (group.Children)
+                partList = new List<SceneObjectPart>(group.Children.Values);
+
+            foreach (SceneObjectPart part in partList)
             {
-                foreach (SceneObjectPart part in group.Children.Values)
+                if (part.IsJoint() && ((part.Flags & PrimFlags.Physics) != 0))
                 {
-                    if (part.IsJoint() && ((part.Flags & PrimFlags.Physics) != 0))
-                    {
-                        PhysicsScene.RequestJointDeletion(part.Name); // FIXME: what if the name changed?
-                    }
-                    else if (part.PhysActor != null)
-                    {
-                        PhysicsScene.RemovePrim(part.PhysActor);
-                        part.PhysActor = null;
-                    }
+                    PhysicsScene.RequestJointDeletion(part.Name); // FIXME: what if the name changed?
+                }
+                else if (part.PhysActor != null)
+                {
+                    PhysicsScene.RemovePrim(part.PhysActor);
+                    part.PhysActor = null;
                 }
             }
             
diff --git a/OpenSim/Region/Framework/Scenes/SceneGraph.cs b/OpenSim/Region/Framework/Scenes/SceneGraph.cs
index 9db2691..2c242c9 100644
--- a/OpenSim/Region/Framework/Scenes/SceneGraph.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneGraph.cs
@@ -380,34 +380,35 @@ namespace OpenSim.Region.Framework.Scenes
                             part.Shape.Scale = scale;
                         }
                     }
+                    
+                    m_numPrim += sceneObject.Children.Count;
+                }
         
-                    sceneObject.AttachToScene(m_parentScene);
+                sceneObject.AttachToScene(m_parentScene);
 
-                    if (sendClientUpdates)
-                        sceneObject.ScheduleGroupForFullUpdate();
-                                     
-                    Entities.Add(sceneObject);
-                    m_numPrim += sceneObject.Children.Count;
-    
-                    if (attachToBackup)
-                        sceneObject.AttachToBackup();
-    
-                    if (OnObjectCreate != null)
-                        OnObjectCreate(sceneObject);
+                if (sendClientUpdates)
+                    sceneObject.ScheduleGroupForFullUpdate();
+                                 
+                Entities.Add(sceneObject);                    
+
+                if (attachToBackup)
+                    sceneObject.AttachToBackup();
+
+                if (OnObjectCreate != null)
+                    OnObjectCreate(sceneObject);
+                
+                lock (SceneObjectGroupsByFullID)
+                {
+                    SceneObjectGroupsByFullID[sceneObject.UUID] = sceneObject;
+                    foreach (SceneObjectPart part in sceneObject.Children.Values)
+                        SceneObjectGroupsByFullID[part.UUID] = sceneObject;
+                }
                     
-                    lock (SceneObjectGroupsByFullID)
-                    {
-                        SceneObjectGroupsByFullID[sceneObject.UUID] = sceneObject;
-                        foreach (SceneObjectPart part in sceneObject.Children.Values)
-                            SceneObjectGroupsByFullID[part.UUID] = sceneObject;
-                    }
-                        
-                    lock (SceneObjectGroupsByLocalID)
-                    {
-                        SceneObjectGroupsByLocalID[sceneObject.LocalId] = sceneObject;
-                        foreach (SceneObjectPart part in sceneObject.Children.Values)
-                            SceneObjectGroupsByLocalID[part.LocalId] = sceneObject;
-                    }
+                lock (SceneObjectGroupsByLocalID)
+                {
+                    SceneObjectGroupsByLocalID[sceneObject.LocalId] = sceneObject;
+                    foreach (SceneObjectPart part in sceneObject.Children.Values)
+                        SceneObjectGroupsByLocalID[part.LocalId] = sceneObject;
                 }
             }
 
diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
index fc5eeed..40a8f83 100644
--- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
@@ -237,6 +237,8 @@ namespace OpenSim.Region.Framework.Scenes
 
         /// <value>
         /// The parts of this scene object group.  You must lock this property before using it.
+        /// If you're doing anything other than reading values, please take a copy of the values rather than locking
+        /// the dictionary for the entirety of the operation.  This increases liveness and reduces the danger of deadlock
         /// If you want to know the number of children, consider using the PrimCount property instead
         /// </value>
         public Dictionary<UUID, SceneObjectPart> Children
@@ -1968,28 +1970,29 @@ namespace OpenSim.Region.Framework.Scenes
             //if ((RootPart.Flags & PrimFlags.TemporaryOnRez) != 0)
             //    return;
 
-            lock (m_parts)
-            {
-                bool UsePhysics = ((RootPart.Flags & PrimFlags.Physics) != 0);
+            bool UsePhysics = ((RootPart.Flags & PrimFlags.Physics) != 0);
 
-                if (UsePhysics && !AbsolutePosition.ApproxEquals(lastPhysGroupPos, 0.02f))
-                {
-                    m_rootPart.UpdateFlag = 1;
-                    lastPhysGroupPos = AbsolutePosition;
-                }
+            if (UsePhysics && !AbsolutePosition.ApproxEquals(lastPhysGroupPos, 0.02f))
+            {
+                m_rootPart.UpdateFlag = 1;
+                lastPhysGroupPos = AbsolutePosition;
+            }
 
-                if (UsePhysics && !GroupRotation.ApproxEquals(lastPhysGroupRot, 0.1f))
-                {
-                    m_rootPart.UpdateFlag = 1;
-                    lastPhysGroupRot = GroupRotation;
-                }
+            if (UsePhysics && !GroupRotation.ApproxEquals(lastPhysGroupRot, 0.1f))
+            {
+                m_rootPart.UpdateFlag = 1;
+                lastPhysGroupRot = GroupRotation;
+            }
 
-                foreach (SceneObjectPart part in m_parts.Values)
-                {
-                    if (!IsSelected)
-                        part.UpdateLookAt();
-                    part.SendScheduledUpdates();
-                }
+            List<SceneObjectPart> partList = null;
+            lock (m_parts)
+                partList = new List<SceneObjectPart>(m_parts.Values);
+                
+            foreach (SceneObjectPart part in partList)
+            {
+                if (!IsSelected)
+                    part.UpdateLookAt();
+                part.SendScheduledUpdates();
             }
         }
 
@@ -2787,11 +2790,12 @@ namespace OpenSim.Region.Framework.Scenes
         public void UpdatePermissions(UUID AgentID, byte field, uint localID,
                 uint mask, byte addRemTF)
         {
+            List<SceneObjectPart> partList = null;
             lock (m_parts)
-            {
-                foreach (SceneObjectPart part in m_parts.Values)
-                    part.UpdatePermissions(AgentID, field, localID, mask, addRemTF);
-            }
+                partList = new List<SceneObjectPart>(m_parts.Values);
+
+            foreach (SceneObjectPart part in partList)
+                part.UpdatePermissions(AgentID, field, localID, mask, addRemTF);
 
             HasGroupChanged = true;
         }