* Increasing ScenePresences locking to prevent race conditions such as those seen in one of the crashes of mantis 1163

* It's not impossible that this could lead to deadlock where sessions simply appear to freeze, even though the region console still responds.
* If this is the case, please file a mantis

3 files changed, 125 insertions, 83 deletions

diff --git a/OpenSim/Region/Environment/Scenes/InnerScene.cs b/OpenSim/Region/Environment/Scenes/InnerScene.cs
index 80e71c8..9d4119d 100644
--- a/OpenSim/Region/Environment/Scenes/InnerScene.cs
+++ b/OpenSim/Region/Environment/Scenes/InnerScene.cs
@@ -116,7 +116,11 @@ namespace OpenSim.Region.Environment.Scenes
 
         public void Close()
         {
-            ScenePresences.Clear();
+            lock (ScenePresences)
+            {
+                ScenePresences.Clear();
+            }
+            
             //SceneObjects.Clear();
             Entities.Clear();
         }
@@ -792,6 +796,8 @@ namespace OpenSim.Region.Environment.Scenes
         /// <returns></returns>
         public List<ScenePresence> GetScenePresences(FilterAvatarList filter)
         {
+            // No locking of scene presences here since we're passing back a list...
+            
             List<ScenePresence> result = new List<ScenePresence>();
             List<ScenePresence> ScenePresencesList = GetScenePresences();
 
@@ -813,9 +819,12 @@ namespace OpenSim.Region.Environment.Scenes
         /// <returns>null if the agent was not found</returns>
         public ScenePresence GetScenePresence(LLUUID agentID)
         {
-            if (ScenePresences.ContainsKey(agentID))
+            lock (ScenePresences)
             {
-                return ScenePresences[agentID];
+                if (ScenePresences.ContainsKey(agentID))
+                {
+                    return ScenePresences[agentID];
+                }
             }
             
             return null;
@@ -917,16 +926,19 @@ namespace OpenSim.Region.Environment.Scenes
 
         internal bool TryGetAvatarByName(string avatarName, out ScenePresence avatar)
         {
-            foreach (ScenePresence presence in ScenePresences.Values)
+            lock (ScenePresences)
             {
-                if (!presence.IsChildAgent)
+                foreach (ScenePresence presence in ScenePresences.Values)
                 {
-                    string name = presence.ControllingClient.FirstName + " " + presence.ControllingClient.LastName;
-
-                    if (String.Compare(avatarName, name, true) == 0)
+                    if (!presence.IsChildAgent)
                     {
-                        avatar = presence;
-                        return true;
+                        string name = presence.ControllingClient.FirstName + " " + presence.ControllingClient.LastName;
+
+                        if (String.Compare(avatarName, name, true) == 0)
+                        {
+                            avatar = presence;
+                            return true;
+                        }
                     }
                 }
             }
@@ -1008,9 +1020,12 @@ namespace OpenSim.Region.Environment.Scenes
 
         internal void ForEachClient(Action<IClientAPI> action)
         {
-            foreach (ScenePresence presence in ScenePresences.Values)
+            lock (ScenePresences)
             {
-                action(presence.ControllingClient);
+                foreach (ScenePresence presence in ScenePresences.Values)
+                {
+                    action(presence.ControllingClient);
+                }
             }
         }
 
diff --git a/OpenSim/Region/Environment/Scenes/Scene.cs b/OpenSim/Region/Environment/Scenes/Scene.cs
index 72512c7..ebbfece 100644
--- a/OpenSim/Region/Environment/Scenes/Scene.cs
+++ b/OpenSim/Region/Environment/Scenes/Scene.cs
@@ -2076,18 +2076,21 @@ namespace OpenSim.Region.Environment.Scenes
         {
             if (regionHandle == m_regInfo.RegionHandle)
             {
-                if (m_scenePresences.ContainsKey(agentID))
+                lock (m_scenePresences)
                 {
-                    try
-                    {
-                        m_scenePresences[agentID].MakeRootAgent(position, isFlying);
-                    }
-                    catch (Exception e)
+                    if (m_scenePresences.ContainsKey(agentID))
                     {
-                        m_log.Info("[SCENE]: Unable to do Agent Crossing.");
-                        m_log.Debug("[SCENE]: " + e.ToString());
+                        try
+                        {
+                            m_scenePresences[agentID].MakeRootAgent(position, isFlying);
+                        }
+                        catch (Exception e)
+                        {
+                            m_log.Info("[SCENE]: Unable to do Agent Crossing.");
+                            m_log.Debug("[SCENE]: " + e.ToString());
+                        }
+                        //m_innerScene.SwapRootChildAgent(false);
                     }
-                    //m_innerScene.SwapRootChildAgent(false);
                 }
             }
         }
@@ -2203,10 +2206,13 @@ namespace OpenSim.Region.Environment.Scenes
         public void RequestTeleportLocation(IClientAPI remoteClient, ulong regionHandle, LLVector3 position,
                                             LLVector3 lookAt, uint flags)
         {
-            if (m_scenePresences.ContainsKey(remoteClient.AgentId))
+            lock (m_scenePresences)
             {
-                m_sceneGridService.RequestTeleportToLocation(m_scenePresences[remoteClient.AgentId], regionHandle,
-                                                             position, lookAt, flags);
+                if (m_scenePresences.ContainsKey(remoteClient.AgentId))
+                {
+                    m_sceneGridService.RequestTeleportToLocation(m_scenePresences[remoteClient.AgentId], regionHandle,
+                                                                 position, lookAt, flags);
+                }
             }
         }
 
@@ -2218,10 +2224,13 @@ namespace OpenSim.Region.Environment.Scenes
         /// <param name="position"></param>
         public void RequestTeleportLandmark(IClientAPI remoteClient, ulong regionHandle, LLVector3 position)
         {
-            if (m_scenePresences.ContainsKey(remoteClient.AgentId))
+            lock (m_scenePresences)
             {
-                m_sceneGridService.RequestTeleportToLocation(m_scenePresences[remoteClient.AgentId], regionHandle,
-                                                             position, LLVector3.Zero, 0);
+                if (m_scenePresences.ContainsKey(remoteClient.AgentId))
+                {
+                    m_sceneGridService.RequestTeleportToLocation(m_scenePresences[remoteClient.AgentId], regionHandle,
+                                                                 position, LLVector3.Zero, 0);
+                }
             }
         }
 
@@ -2353,18 +2362,25 @@ namespace OpenSim.Region.Environment.Scenes
         public void SendUrlToUser(LLUUID avatarID, string objectName, LLUUID objectID, LLUUID ownerID, bool groupOwned,
                                   string message, string url)
         {
-            if (m_scenePresences.ContainsKey(avatarID))
+            lock (m_scenePresences)
             {
-                m_scenePresences[avatarID].ControllingClient.SendLoadURL(objectName, objectID, ownerID, groupOwned,
-                                                                         message, url);
+                if (m_scenePresences.ContainsKey(avatarID))
+                {
+                    m_scenePresences[avatarID].ControllingClient.SendLoadURL(objectName, objectID, ownerID, groupOwned,
+                                                                             message, url);
+                }
             }
         }
 
         public void SendDialogToUser(LLUUID avatarID, string objectName, LLUUID objectID, LLUUID ownerID, string message, LLUUID TextureID, int ch, string[] buttonlabels)
         {
-            if (m_scenePresences.ContainsKey(avatarID))
+            lock (m_scenePresences)
             {
-                m_scenePresences[avatarID].ControllingClient.SendDialog(objectName, objectID, ownerID, message, TextureID, ch, buttonlabels);
+                if (m_scenePresences.ContainsKey(avatarID))
+                {
+                    m_scenePresences[avatarID].ControllingClient.SendDialog(
+                        objectName, objectID, ownerID, message, TextureID, ch, buttonlabels);
+                }
             }
         }
 
@@ -2478,9 +2494,12 @@ namespace OpenSim.Region.Environment.Scenes
         /// <param name="modal"></param>
         public void SendAlertToUser(LLUUID agentID, string message, bool modal)
         {
-            if (m_scenePresences.ContainsKey(agentID))
+            lock (m_scenePresences)
             {
-                m_scenePresences[agentID].ControllingClient.SendAgentAlertMessage(message, modal);
+                if (m_scenePresences.ContainsKey(agentID))
+                {
+                    m_scenePresences[agentID].ControllingClient.SendAgentAlertMessage(message, modal);
+                }
             }
         }
 
@@ -2494,28 +2513,31 @@ namespace OpenSim.Region.Environment.Scenes
         public void handleRequestGodlikePowers(LLUUID agentID, LLUUID sessionID, LLUUID token, bool godLike,
                                                IClientAPI controllingClient)
         {
-            // First check that this is the sim owner
-            if (Permissions.GenericEstatePermission(agentID))
+            lock (m_scenePresences)
             {
                 // User needs to be logged into this sim
                 if (m_scenePresences.ContainsKey(agentID))
-                {
-                    // Next we check for spoofing.....
-                    LLUUID testSessionID = m_scenePresences[agentID].ControllingClient.SessionId;
-                    if (sessionID == testSessionID)
+                {   
+                    // First check that this is the sim owner
+                    if (Permissions.GenericEstatePermission(agentID))
                     {
-                        if (sessionID == controllingClient.SessionId)
+                        // Next we check for spoofing.....
+                        LLUUID testSessionID = m_scenePresences[agentID].ControllingClient.SessionId;
+                        if (sessionID == testSessionID)
                         {
-                            //m_log.Info("godlike: " + godLike.ToString());
-                            m_scenePresences[agentID].GrantGodlikePowers(agentID, testSessionID, token, godLike);
+                            if (sessionID == controllingClient.SessionId)
+                            {
+                                //m_log.Info("godlike: " + godLike.ToString());
+                                m_scenePresences[agentID].GrantGodlikePowers(agentID, testSessionID, token, godLike);
+                            }
                         }
                     }
+                    else
+                    {
+                        m_scenePresences[agentID].ControllingClient.SendAgentAlertMessage("Request for god powers denied", false);
+                    }
                 }
             }
-            else
-            {
-                m_scenePresences[agentID].ControllingClient.SendAgentAlertMessage("Request for god powers denied", false);
-            }
         }
 
         /// <summary>
@@ -2571,55 +2593,59 @@ namespace OpenSim.Region.Environment.Scenes
         {
             // For some reason the client sends this seemingly hard coded UUID for kicking everyone.   Dun-know.
             LLUUID kickUserID = new LLUUID("44e87126e7944ded05b37c42da3d5cdb");
-            if (m_scenePresences.ContainsKey(agentID) || agentID == kickUserID)
+            lock (m_scenePresences)
             {
-                if (Permissions.GenericEstatePermission(godID))
+                if (m_scenePresences.ContainsKey(agentID) || agentID == kickUserID)
                 {
-                    if (agentID == kickUserID)
+                    if (Permissions.GenericEstatePermission(godID))
                     {
-                        ClientManager.ForEachClient(delegate(IClientAPI controller)
-                                                    {
-                                                        if (controller.AgentId != godID)
-                                                            controller.Kick(Helpers.FieldToUTF8String(reason));
+                        if (agentID == kickUserID)
+                        {
+                            ClientManager.ForEachClient(delegate(IClientAPI controller)
+                                                        {
+                                                            if (controller.AgentId != godID)
+                                                                controller.Kick(Helpers.FieldToUTF8String(reason));
 
 
 
-                                                    }
-                            );
-                        // This is a bit crude.   It seems the client will be null before it actually stops the thread
-                        // The thread will kill itself eventually :/
-                        // Is there another way to make sure *all* clients get this 'inter region' message?
-                        ClientManager.ForEachClient(delegate(IClientAPI controller)
-                                                    {
-                                                        ScenePresence p = GetScenePresence(controller.AgentId);
-                                                        bool childagent = !p.Equals(null) && p.IsChildAgent;
-                                                        if (controller.AgentId != godID && !childagent)
-                                                            // Do we really want to kick the initiator of this madness?
+                                                        }
+                                );
+                            
+                            // This is a bit crude.   It seems the client will be null before it actually stops the thread
+                            // The thread will kill itself eventually :/
+                            // Is there another way to make sure *all* clients get this 'inter region' message?
+                            ClientManager.ForEachClient(delegate(IClientAPI controller)
                                                         {
-                                                            controller.Close(true);
+                                                            ScenePresence p = GetScenePresence(controller.AgentId);
+                                                            bool childagent = !p.Equals(null) && p.IsChildAgent;
+                                                            if (controller.AgentId != godID && !childagent)
+                                                                // Do we really want to kick the initiator of this madness?
+                                                            {
+                                                                controller.Close(true);
+                                                            }
                                                         }
-                                                    }
-                            );
-                    }
-                    else
-                    {
-                        if (m_scenePresences[agentID].IsChildAgent)
-                        {
-                            m_innerScene.removeUserCount(false);
+                                );
                         }
                         else
                         {
-                            m_innerScene.removeUserCount(true);
-                        }
+                            if (m_scenePresences[agentID].IsChildAgent)
+                            {
+                                m_innerScene.removeUserCount(false);
+                            }
+                            else
+                            {
+                                m_innerScene.removeUserCount(true);
+                            }
 
-                        m_scenePresences[agentID].ControllingClient.Kick(Helpers.FieldToUTF8String(reason));
-                        m_scenePresences[agentID].ControllingClient.Close(true);
+                            m_scenePresences[agentID].ControllingClient.Kick(Helpers.FieldToUTF8String(reason));
+                            m_scenePresences[agentID].ControllingClient.Close(true);
+                        }
+                    }
+                    else
+                    {
+                        if (m_scenePresences.ContainsKey(godID))
+                            m_scenePresences[godID].ControllingClient.SendAgentAlertMessage("Kick request denied", false);
                     }
-                }
-                else
-                {
-                    if (m_scenePresences.ContainsKey(godID))
-                        m_scenePresences[godID].ControllingClient.SendAgentAlertMessage("Kick request denied", false);
                 }
             }
         }
diff --git a/OpenSim/Region/Environment/Scenes/ScenePresence.cs b/OpenSim/Region/Environment/Scenes/ScenePresence.cs
index 052b85a..3554289 100644
--- a/OpenSim/Region/Environment/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Environment/Scenes/ScenePresence.cs
@@ -1506,6 +1506,7 @@ namespace OpenSim.Region.Environment.Scenes
             foreach (ScenePresence avatar in avatars)
             {
                 SendFullUpdateToOtherClient(avatar);
+                
                 if (avatar.LocalId != LocalId)
                 {
                     if (!avatar.m_isChildAgent || m_scene.m_seeIntoRegionFromNeighbor)