Inconsistent locking of ScenePresence array in SceneGraph. Fixed by eliminating option to return the actual list. Callers can now either request a copy of the array as a new List or ask the SceneGraph to call a delegate function on every ScenePresence. Iteration and locking of the ScenePresences now takes place only within the SceneGraph class.

This patch also applies a fix to Combat/CombatModule.cs which had unlocked iteration of the ScenePresences and inconsistent try/catch around the use of those ScenePresences.

3 files changed, 46 insertions, 67 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Combat/CombatModule.cs b/OpenSim/Region/CoreModules/Avatar/Combat/CombatModule.cs
index 3614915..9df6074 100644
--- a/OpenSim/Region/CoreModules/Avatar/Combat/CombatModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Combat/CombatModule.cs
@@ -89,60 +89,57 @@ namespace OpenSim.Region.CoreModules.Avatar.Combat.CombatModule
             get { return true; }
         }
 
-        private void KillAvatar(uint killerObjectLocalID, ScenePresence DeadAvatar)
+        private void KillAvatar(uint killerObjectLocalID, ScenePresence deadAvatar)
         {
+            string deadAvatarMessage;
+            ScenePresence killingAvatar = null;
+            string killingAvatarMessage;
+
             if (killerObjectLocalID == 0)
-                DeadAvatar.ControllingClient.SendAgentAlertMessage("You committed suicide!", true);
+                deadAvatarMessage = "You committed suicide!";
             else
             {
-                bool foundResult = false;
-                string resultstring = String.Empty;
-                ScenePresence[] allav = DeadAvatar.Scene.GetScenePresences();
-                try
+                // Try to get the avatar responsible for the killing
+                killingAvatar = deadAvatar.Scene.GetScenePresence(killerObjectLocalID);
+                if (killingAvatar == null)
                 {
-                    for (int i = 0; i < allav.Length; i++)
+                    // Try to get the object which was responsible for the killing
+                    SceneObjectPart part = deadAvatar.Scene.GetSceneObjectPart(killerObjectLocalID);
+                    if (part == null)
                     {
-                        ScenePresence av = allav[i];
-
-                        if (av.LocalId == killerObjectLocalID)
-                        {
-                            av.ControllingClient.SendAlertMessage("You fragged " + DeadAvatar.Firstname + " " + DeadAvatar.Lastname);
-                            resultstring = av.Firstname + " " + av.Lastname;
-                            foundResult = true;
-                        }
+                        // Cause of death: Unknown
+                        deadAvatarMessage = "You died!";
                     }
-                } catch (InvalidOperationException)
-                {
-
-                }
-
-                if (!foundResult)
-                {
-                    SceneObjectPart part = DeadAvatar.Scene.GetSceneObjectPart(killerObjectLocalID);
-                    if (part != null)
+                    else
                     {
-                        ScenePresence av = DeadAvatar.Scene.GetScenePresence(part.OwnerID);
-                        if (av != null)
-                        {
-                            av.ControllingClient.SendAlertMessage("You fragged " + DeadAvatar.Firstname + " " + DeadAvatar.Lastname);
-                            resultstring = av.Firstname + " " + av.Lastname;
-                            DeadAvatar.ControllingClient.SendAgentAlertMessage("You got killed by " + resultstring + "!", true);
-                        }
+                        // Try to find the avatar wielding the killing object
+                        killingAvatar = deadAvatar.Scene.GetScenePresence(part.OwnerID);
+                        if (killingAvatar == null)
+                            deadAvatarMessage = String.Format("You impaled yourself on {0} owned by {1}!", part.Name, deadAvatar.Scene.GetUserName(part.OwnerID));
                         else
                         {
-                            string killer = DeadAvatar.Scene.GetUserName(part.OwnerID);
-                            DeadAvatar.ControllingClient.SendAgentAlertMessage("You impaled yourself on " + part.Name + " owned by " + killer +"!", true);
+                            killingAvatarMessage = String.Format("You fragged {0}!", deadAvatar.Name);
+                            deadAvatarMessage = String.Format("You got killed by {0}!", killingAvatar.Name);
                         }
-                        //DeadAvatar.Scene. part.ObjectOwner
-                    }
-                    else
-                    {
-                        DeadAvatar.ControllingClient.SendAgentAlertMessage("You died!", true);
                     }
                 }
+                else
+                {
+                    killingAvatarMessage = String.Format("You fragged {0}!", deadAvatar.Name);
+                    deadAvatarMessage = String.Format("You got killed by {0}!", killingAvatar.Name);
+                }
             }
-            DeadAvatar.Health = 100;
-            DeadAvatar.Scene.TeleportClientHome(DeadAvatar.UUID, DeadAvatar.ControllingClient);
+            try
+            {
+                deadAvatar.ControllingClient.SendAgentAlertMessage(deadAvatarMessage, true);
+                if(killingAvatar != null)
+                    killingAvatar.ControllingClient.SendAlertMessage("You fragged " + deadAvatar.Firstname + " " + deadAvatar.Lastname);
+            }
+            catch (InvalidOperationException)
+            { }
+
+            deadAvatar.Health = 100;
+            deadAvatar.Scene.TeleportClientHome(deadAvatar.UUID, deadAvatar.ControllingClient);
         }
 
         private void AvatarEnteringParcel(ScenePresence avatar, int localLandID, UUID regionID)
diff --git a/OpenSim/Region/CoreModules/Avatar/Dialog/DialogModule.cs b/OpenSim/Region/CoreModules/Avatar/Dialog/DialogModule.cs
index b8e013c..c31266c 100644
--- a/OpenSim/Region/CoreModules/Avatar/Dialog/DialogModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Dialog/DialogModule.cs
@@ -87,31 +87,18 @@ namespace OpenSim.Region.CoreModules.Avatar.Dialog
         
         public void SendAlertToUser(string firstName, string lastName, string message, bool modal)
         {
-            ScenePresence[] presenceList = m_scene.GetScenePresences();
-
-            for (int i = 0; i < presenceList.Length; i++)
-            {
-                ScenePresence presence = presenceList[i];
-
-                if (presence.Firstname == firstName && presence.Lastname == lastName)
-                {
-                    presence.ControllingClient.SendAgentAlertMessage(message, modal);
-                    break;
-                }
-            }
+            ScenePresence presence = m_scene.GetScenePresence(firstName, lastName);
+            if(presence != null)
+                presence.ControllingClient.SendAgentAlertMessage(message, modal);
         }
         
         public void SendGeneralAlert(string message)
         {
-            ScenePresence[] presenceList = m_scene.GetScenePresences();
-
-            for (int i = 0; i < presenceList.Length; i++)
+            m_scene.ForEachScenePresence(delegate(ScenePresence presence)
             {
-                ScenePresence presence = presenceList[i];
-
                 if (!presence.IsChildAgent)
                     presence.ControllingClient.SendAlertMessage(message);
-            }
+            });
         }
 
         public void SendDialogToUser(
@@ -179,14 +166,11 @@ namespace OpenSim.Region.CoreModules.Avatar.Dialog
         public void SendNotificationToUsersInRegion(
             UUID fromAvatarID, string fromAvatarName, string message)
         {
-            ScenePresence[] presences = m_scene.GetScenePresences();
-
-            for (int i = 0; i < presences.Length; i++)
+            m_scene.ForEachScenePresence(delegate(ScenePresence presence)
             {
-                ScenePresence presence = presences[i];
                 if (!presence.IsChildAgent)
                     presence.ControllingClient.SendBlueBoxMessage(fromAvatarID, fromAvatarName, message);
-            }
+            });
         }
         
         /// <summary>
diff --git a/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs b/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs
index e3bab2d..464d922 100644
--- a/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs
+++ b/OpenSim/Region/CoreModules/World/Estate/EstateManagementModule.cs
@@ -469,12 +469,10 @@ namespace OpenSim.Region.CoreModules.World.Estate
         private void handleEstateTeleportAllUsersHomeRequest(IClientAPI remover_client, UUID invoice, UUID senderID)
         {
             // Get a fresh list that will not change as people get teleported away
-            ScenePresence[] presences = m_scene.GetScenePresences();
+            List<ScenePresence> presences = m_scene.GetScenePresences();
 
-            for (int i = 0; i < presences.Length; i++)
+            foreach(ScenePresence p in presences)
             {
-                ScenePresence p = presences[i];
-
                 if (p.UUID != senderID)
                 {
                     // make sure they are still there, we could be working down a long list