Rewrote parts of the code that were double-locking different objects. This is about half of the code base reviewed.

OpenSim/Framework/Communications/Cache/UserProfileCacheService.cs

@@ -128,24 +128,18 @@ namespace OpenSim.Framework.Communications.Cache
         /// <returns>null if no user details are found</returns>
         public CachedUserInfo GetUserDetails(string fname, string lname)
         {
+            CachedUserInfo userInfo;
             lock (m_userProfilesByName)
-            {
+            {    
-                CachedUserInfo userInfo;
-                
                 if (m_userProfilesByName.TryGetValue(string.Format(NAME_FORMAT, fname, lname), out userInfo))
-                {
                     return userInfo;
-                }
-                else
-                {
-                    UserProfileData userProfile = m_commsManager.UserService.GetUserProfile(fname, lname);
-                
-                    if (userProfile != null)
-                        return AddToCaches(userProfile);
-                    else
-                        return null;
-                }
             }
+            UserProfileData userProfile = m_commsManager.UserService.GetUserProfile(fname, lname);
+        
+            if (userProfile != null)
+                return AddToCaches(userProfile);
+            else
+                return null;
         }
         
         /// <summary>
@@ -160,20 +154,14 @@ namespace OpenSim.Framework.Communications.Cache
                 return null;
 
             lock (m_userProfilesById)
-            {
                 if (m_userProfilesById.ContainsKey(userID))
-                {
                     return m_userProfilesById[userID];
-                }
+
-                else
+            UserProfileData userProfile = m_commsManager.UserService.GetUserProfile(userID);
-                {
+            if (userProfile != null)
-                    UserProfileData userProfile = m_commsManager.UserService.GetUserProfile(userID);
+                return AddToCaches(userProfile);
-                    if (userProfile != null)
+            else
-                        return AddToCaches(userProfile);
+                return null;
-                    else
-                        return null;
-                }
-            }
         }
         
         /// <summary>
@@ -211,14 +199,10 @@ namespace OpenSim.Framework.Communications.Cache
             CachedUserInfo createdUserInfo = new CachedUserInfo(m_InventoryService, userProfile);
             
             lock (m_userProfilesById)
-            {
                 m_userProfilesById[createdUserInfo.UserProfile.ID] = createdUserInfo;
-                
+
-                lock (m_userProfilesByName)
+            lock (m_userProfilesByName)
-                {
+                m_userProfilesByName[createdUserInfo.UserProfile.Name] = createdUserInfo;
-                    m_userProfilesByName[createdUserInfo.UserProfile.Name] = createdUserInfo;
-                }
-            }
             
             return createdUserInfo;
         }
@@ -230,21 +214,25 @@ namespace OpenSim.Framework.Communications.Cache
         /// <returns>true if there was a profile to remove, false otherwise</returns>
         protected bool RemoveFromCaches(UUID userId)
         {
+            CachedUserInfo userInfo = null;
             lock (m_userProfilesById)
             {
                 if (m_userProfilesById.ContainsKey(userId))
                 {
-                    CachedUserInfo userInfo = m_userProfilesById[userId];
+                    userInfo = m_userProfilesById[userId];
                     m_userProfilesById.Remove(userId);
-                    
-                    lock (m_userProfilesByName)
-                    {
-                        m_userProfilesByName.Remove(userInfo.UserProfile.Name);
-                    }
-                    
-                    return true;
                 }
             }
+
+            if (userInfo != null)
+                lock (m_userProfilesByName)
+                {
+                    if (m_userProfilesByName.ContainsKey(userInfo.UserProfile.Name))
+                    {
+                        m_userProfilesByName.Remove(userInfo.UserProfile.Name);
+                        return true;
+                    }
+                }
             
             return false;
         }

OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs

@@ -3122,6 +3122,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 objectData.TextureAnim = textureanim;
             }
 
+            bool doUpdate = false;
             lock (m_primFullUpdates)
             {
                 if (m_primFullUpdates.Count == 0)
@@ -3130,8 +3131,10 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 m_primFullUpdates.Add(objectData);
 
                 if (m_primFullUpdates.Count >= m_primFullUpdatesPerPacket)
-                    ProcessPrimFullUpdates(this, null);
+                    doUpdate = true;
             }
+            if (doUpdate)
+                ProcessPrimFullUpdates(this, null);
         }
 
         void HandleQueueEmpty(ThrottleOutPacketType queue)
@@ -3152,35 +3155,40 @@ namespace OpenSim.Region.ClientStack.LindenUDP
 
         void ProcessPrimFullUpdates(object sender, ElapsedEventArgs e)
         {
+            bool stopTimer = false;
             lock (m_primFullUpdates)
             {
                 if (m_primFullUpdates.Count == 0 && m_primFullUpdateTimer.Enabled)
-                {
+                    stopTimer = true;
-                    lock (m_primFullUpdateTimer)
+            }
-                        m_primFullUpdateTimer.Stop();
+            if (stopTimer)
+            {
+                lock (m_primFullUpdateTimer)
+                    m_primFullUpdateTimer.Stop();
+                return;
+            }
 
-                    return;
+            ObjectUpdatePacket outPacket =
-                }
+                    (ObjectUpdatePacket)PacketPool.Instance.GetPacket(
+                    PacketType.ObjectUpdate);
 
-                ObjectUpdatePacket outPacket =
+            outPacket.RegionData.RegionHandle =
-                        (ObjectUpdatePacket)PacketPool.Instance.GetPacket(
+                    Scene.RegionInfo.RegionHandle;
-                        PacketType.ObjectUpdate);
+            outPacket.RegionData.TimeDilation =
+                    (ushort)(Scene.TimeDilation * ushort.MaxValue);
 
-                outPacket.RegionData.RegionHandle =
+            int max = m_primFullUpdates.Count;
-                        Scene.RegionInfo.RegionHandle;
+            if (max > m_primFullUpdatesPerPacket)
-                outPacket.RegionData.TimeDilation =
+                max = m_primFullUpdatesPerPacket;
-                        (ushort)(Scene.TimeDilation * ushort.MaxValue);
 
-                int max = m_primFullUpdates.Count;
+            int count = 0;
-                if (max > m_primFullUpdatesPerPacket)
+            int size = 0;
-                    max = m_primFullUpdatesPerPacket;
 
-                int count = 0;
+            byte[] zerobuffer = new byte[1024];
-                int size = 0;
+            byte[] blockbuffer = new byte[1024];
-
-                byte[] zerobuffer = new byte[1024];
-                byte[] blockbuffer = new byte[1024];
 
+            lock (m_primFullUpdates)
+            {
                 for (count = 0 ; count < max ; count++)
                 {
                     int length = 0;
@@ -3204,9 +3212,12 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 OutPacket(outPacket, ThrottleOutPacketType.Task | ThrottleOutPacketType.LowPriority);
 
                 if (m_primFullUpdates.Count == 0 && m_primFullUpdateTimer.Enabled)
-                    lock (m_primFullUpdateTimer)
+                    stopTimer = true;
-                        m_primFullUpdateTimer.Stop();
             }
+
+            if (stopTimer)
+                lock (m_primFullUpdateTimer)
+                    m_primFullUpdateTimer.Stop();
         }
 
         /// <summary>
@@ -3225,6 +3236,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                     CreatePrimImprovedBlock(localID, position, rotation,
                     velocity, rotationalvelocity, state);
 
+            bool doUpdate = false;
             lock (m_primTerseUpdates)
             {
                 if (m_primTerseUpdates.Count == 0)
@@ -3233,43 +3245,51 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 m_primTerseUpdates.Add(objectData);
 
                 if (m_primTerseUpdates.Count >= m_primTerseUpdatesPerPacket)
-                    ProcessPrimTerseUpdates(this, null);
+                    doUpdate = true;
             }
+            if (doUpdate)
+                ProcessPrimTerseUpdates(this, null);
         }
 
         void ProcessPrimTerseUpdates(object sender, ElapsedEventArgs e)
         {
+            bool stopTimer = false;
             lock (m_primTerseUpdates)
             {
                 if (m_primTerseUpdates.Count == 0)
-                {
+                    stopTimer = true;
-                    lock (m_primTerseUpdateTimer)
+            }
-                        m_primTerseUpdateTimer.Stop();
+            if (stopTimer)
+            {
+                lock (m_primTerseUpdateTimer)
+                    m_primTerseUpdateTimer.Stop();
 
-                    return;
+                return;
-                }
+            }
 
-                ImprovedTerseObjectUpdatePacket outPacket =
+            ImprovedTerseObjectUpdatePacket outPacket =
-                        (ImprovedTerseObjectUpdatePacket)
+                    (ImprovedTerseObjectUpdatePacket)
-                        PacketPool.Instance.GetPacket(
+                    PacketPool.Instance.GetPacket(
-                        PacketType.ImprovedTerseObjectUpdate);
+                    PacketType.ImprovedTerseObjectUpdate);
 
-                outPacket.RegionData.RegionHandle =
+            outPacket.RegionData.RegionHandle =
-                        Scene.RegionInfo.RegionHandle;
+                    Scene.RegionInfo.RegionHandle;
-                outPacket.RegionData.TimeDilation =
+            outPacket.RegionData.TimeDilation =
-                        (ushort)(Scene.TimeDilation * ushort.MaxValue);
+                    (ushort)(Scene.TimeDilation * ushort.MaxValue);
 
-                int max = m_primTerseUpdates.Count;
+            int max = m_primTerseUpdates.Count;
-                if (max > m_primTerseUpdatesPerPacket)
+            if (max > m_primTerseUpdatesPerPacket)
-                    max = m_primTerseUpdatesPerPacket;
+                max = m_primTerseUpdatesPerPacket;
 
-                int count = 0;
+            int count = 0;
-                int size = 0;
+            int size = 0;
 
-                byte[] zerobuffer = new byte[1024];
+            byte[] zerobuffer = new byte[1024];
-                byte[] blockbuffer = new byte[1024];
+            byte[] blockbuffer = new byte[1024];
 
-                for (count = 0 ; count < max ; count++)
+            lock (m_primTerseUpdates)
+            {
+                for (count = 0; count < max; count++)
                 {
                     int length = 0;
                     m_primTerseUpdates[count].ToBytes(blockbuffer, ref length);
@@ -3294,9 +3314,11 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 OutPacket(outPacket, ThrottleOutPacketType.Task | ThrottleOutPacketType.LowPriority);
 
                 if (m_primTerseUpdates.Count == 0)
-                    lock (m_primTerseUpdateTimer)
+                    stopTimer = true;
-                        m_primTerseUpdateTimer.Stop();
             }
+            if (stopTimer)
+                lock (m_primTerseUpdateTimer)
+                    m_primTerseUpdateTimer.Stop();
         }
 
         public void FlushPrimUpdates()

OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs

@@ -442,42 +442,46 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
         private ScenePresence GetRootPresenceFromAgentID(UUID AgentID)
         {
-            ScenePresence returnAgent = null;
+            List<Scene> scenes = null;
             lock (m_scenes)
-            {
+                scenes = new List<Scene>(m_scenes.Values);
-                ScenePresence queryagent = null;
-                foreach (Scene scene in m_scenes.Values)
-                {
-                    queryagent = scene.GetScenePresence(AgentID);
-                    if (queryagent != null)
-                    {
-                        if (!queryagent.IsChildAgent)
-                        {
-                            returnAgent = queryagent;
-                            break;
-                        }
-                    }
-                }
-            }
-            return returnAgent;
-        }
 
-        private ScenePresence GetAnyPresenceFromAgentID(UUID AgentID)
-        {
             ScenePresence returnAgent = null;
-            lock (m_scenes)
+            ScenePresence queryagent = null;
+            foreach (Scene scene in scenes)
             {
-                ScenePresence queryagent = null;
+                queryagent = scene.GetScenePresence(AgentID);
-                foreach (Scene scene in m_scenes.Values)
+                if (queryagent != null)
                 {
-                    queryagent = scene.GetScenePresence(AgentID);
+                    if (!queryagent.IsChildAgent)
-                    if (queryagent != null)
                     {
                         returnAgent = queryagent;
                         break;
                     }
                 }
             }
+
+            return returnAgent;
+        }
+
+        private ScenePresence GetAnyPresenceFromAgentID(UUID AgentID)
+        {
+            List<Scene> scenes = null;
+            lock (m_scenes)
+                scenes = new List<Scene>(m_scenes.Values);
+
+            ScenePresence returnAgent = null;
+            ScenePresence queryagent = null;
+            foreach (Scene scene in m_scenes.Values)
+            {
+                queryagent = scene.GetScenePresence(AgentID);
+                if (queryagent != null)
+                {
+                    returnAgent = queryagent;
+                    break;
+                }
+            }
+ 
             return returnAgent;
         }
         

OpenSim/Region/CoreModules/Avatar/InstantMessage/PresenceModule.cs

@@ -290,13 +290,14 @@ namespace OpenSim.Region.CoreModules.Avatar.InstantMessage
 
             // get the agent. This should work every time, as we just got a packet from it
             ScenePresence agent = null;
+            List<Scene> scenes = null;
             lock (m_Scenes)
+                scenes = new List<Scene>(m_Scenes);
+
+            foreach (Scene scene in scenes)
             {
-                foreach (Scene scene in m_Scenes)
+                agent = scene.GetScenePresence(agentID);
-                {
+                if (agent != null) break;
-                    agent = scene.GetScenePresence(agentID);
-                    if (agent != null) break;
-                }
             }
 
             // just to be paranoid...

OpenSim/Region/CoreModules/Avatar/Inventory/Transfer/InventoryTransferModule.cs

@@ -111,16 +111,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Inventory.Transfer
 
         private Scene FindClientScene(UUID agentId)
         {
+            List<Scene> scenes = null;
             lock (m_Scenelist)
+                scenes = new List<Scene>(m_Scenelist);
+
+            foreach (Scene scene in scenes)
             {
-                foreach (Scene scene in m_Scenelist)
+                ScenePresence presence = scene.GetScenePresence(agentId);
+                if (presence != null)
                 {
-                    ScenePresence presence = scene.GetScenePresence(agentId);
+                    if (!presence.IsChildAgent)
-                    if (presence != null)
+                        return scene;
-                    {
-                        if (!presence.IsChildAgent)
-                            return scene;
-                    }
                 }
             }
             return null;

OpenSim/Region/CoreModules/Scripting/EMailModules/EmailModule.cs

@@ -198,19 +198,20 @@ namespace OpenSim.Region.CoreModules.Scripting.EmailModules
 
         private SceneObjectPart findPrim(UUID objectID, out string ObjectRegionName)
         {
+            List<Scene> scenes = null;
             lock (m_Scenes)
+                scenes = new List<Scene>(m_Scenes.Values);
+
+            foreach (Scene s in scenes)
             {
-                foreach (Scene s in m_Scenes.Values)
+                SceneObjectPart part = s.GetSceneObjectPart(objectID);
+                if (part != null)
                 {
-                    SceneObjectPart part = s.GetSceneObjectPart(objectID);
+                    ObjectRegionName = s.RegionInfo.RegionName;
-                    if (part != null)
+                    uint localX = (s.RegionInfo.RegionLocX * (int)Constants.RegionSize);
-                    {
+                    uint localY = (s.RegionInfo.RegionLocY * (int)Constants.RegionSize);
-                        ObjectRegionName = s.RegionInfo.RegionName;
+                    ObjectRegionName = ObjectRegionName + " (" + localX + ", " + localY + ")";
-                        uint localX = (s.RegionInfo.RegionLocX * (int)Constants.RegionSize);
+                    return part;
-                        uint localY = (s.RegionInfo.RegionLocY * (int)Constants.RegionSize);
-                        ObjectRegionName = ObjectRegionName + " (" + localX + ", " + localY + ")";
-                        return part;
-                    }
                 }
             }
             ObjectRegionName = string.Empty;
@@ -363,6 +364,7 @@ namespace OpenSim.Region.CoreModules.Scripting.EmailModules
         public Email GetNextEmail(UUID objectID, string sender, string subject)
         {
             List<Email> queue = null;
+            List<UUID> removal = new List<UUID>();
 
             lock (m_LastGetEmailCall)
             {
@@ -375,7 +377,6 @@ namespace OpenSim.Region.CoreModules.Scripting.EmailModules
 
                 // Hopefully this isn't too time consuming.  If it is, we can always push it into a worker thread.
                 DateTime now = DateTime.Now;
-                List<UUID> removal = new List<UUID>();
                 foreach (UUID uuid in m_LastGetEmailCall.Keys)
                 {
                     if ((now - m_LastGetEmailCall[uuid]) > m_QueueTimeout)
@@ -383,15 +384,15 @@ namespace OpenSim.Region.CoreModules.Scripting.EmailModules
                         removal.Add(uuid);
                     }
                 }
+            }
 
-                foreach (UUID remove in removal)
+            foreach (UUID remove in removal)
-                {
+            {
+                lock (m_LastGetEmailCall)
                     m_LastGetEmailCall.Remove(remove);
-                    lock (m_MailQueues)
+    
-                    {
+                lock (m_MailQueues)
-                        m_MailQueues.Remove(remove);
+                    m_MailQueues.Remove(remove);
-                    }
-                }
             }
 
             lock (m_MailQueues)

OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs

@@ -187,14 +187,16 @@ namespace OpenSim.Region.CoreModules.World.Land
             LandData newData = data.Copy();
             newData.LocalID = local_id;
 
+            ILandObject land = null;
             lock (m_landList)
             {
                 if (m_landList.ContainsKey(local_id))
                 {
                     m_landList[local_id].LandData = newData;
-                    m_scene.EventManager.TriggerLandObjectUpdated((uint)local_id, m_landList[local_id]);
+                    land = m_landList[local_id];
                 }
             }
+            m_scene.EventManager.TriggerLandObjectUpdated((uint)local_id, land);
         }
 
         public bool AllowedForcefulBans
@@ -504,6 +506,7 @@ namespace OpenSim.Region.CoreModules.World.Land
         /// <param name="local_id">Land.localID of the peice of land to remove.</param>
         public void removeLandObject(int local_id)
         {
+            UUID id = UUID.Zero;
             lock (m_landList)
             {
                 for (int x = 0; x < 64; x++)
@@ -520,9 +523,10 @@ namespace OpenSim.Region.CoreModules.World.Land
                     }
                 }
 
-                m_scene.EventManager.TriggerLandObjectRemoved(m_landList[local_id].LandData.GlobalID);
+                id = m_landList[local_id].LandData.GlobalID;
                 m_landList.Remove(local_id);
             }
+            m_scene.EventManager.TriggerLandObjectRemoved(id);
         }
 
         private void performFinalLandJoin(ILandObject master, ILandObject slave)

OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs

@@ -122,12 +122,13 @@ namespace OpenSim.Region.Framework.Scenes
         public bool InventoryDeQueueAndDelete()
         {
             DeleteToInventoryHolder x = null;
+            int left = 0;
  
             try
             {
                 lock (m_inventoryDeletes)
                 {
-                    int left = m_inventoryDeletes.Count;
+                    left = m_inventoryDeletes.Count;
                     if (left > 0)
                     {
                         x = m_inventoryDeletes.Dequeue();
@@ -136,24 +137,27 @@ namespace OpenSim.Region.Framework.Scenes
                             m_inventoryDeletes.Enqueue(x);
                             return true;
                         }
-
-                        m_log.DebugFormat(
-                            "[SCENE]: Sending object to user's inventory, {0} item(s) remaining.", left);
-                        
-                        try
-                        {
-                            m_scene.DeleteToInventory(x.action, x.folderID, x.objectGroup, x.remoteClient);
-                            if (x.permissionToDelete)
-                                m_scene.DeleteSceneObject(x.objectGroup, false);
-                        }
-                        catch (Exception e)
-                        {
-                            m_log.DebugFormat("Exception background sending object: " + e);
-                        }
-                        
-                        return true;
                     }
                 }
+
+                if (left > 0)
+                {
+                    m_log.DebugFormat(
+                        "[SCENE]: Sending object to user's inventory, {0} item(s) remaining.", left);
+                    
+                    try
+                    {
+                        m_scene.DeleteToInventory(x.action, x.folderID, x.objectGroup, x.remoteClient);
+                        if (x.permissionToDelete)
+                            m_scene.DeleteSceneObject(x.objectGroup, false);
+                    }
+                    catch (Exception e)
+                    {
+                        m_log.DebugFormat("Exception background sending object: " + e);
+                    }
+                    
+                    return true;
+                }
             }
             catch (Exception e)
             {

OpenSim/Region/Framework/Scenes/SceneGraph.cs

@@ -297,34 +297,44 @@ namespace OpenSim.Region.Framework.Scenes
 
             sceneObject.AttachToScene(m_parentScene);
 
+            List<SceneObjectPart> parts = null;
+            bool found = false;
             lock (sceneObject)
             {
                 if (!Entities.ContainsKey(sceneObject.UUID))
                 {
+                    found = true;
                     Entities.Add(sceneObject);
                     m_numPrim += sceneObject.Children.Count;
 
                     if (attachToBackup)
                         sceneObject.AttachToBackup();
 
-                    if (OnObjectCreate != null)
+                    parts = new List<SceneObjectPart>(sceneObject.Children.Values);
-                        OnObjectCreate(sceneObject);
-                    
-                    lock (m_dictionary_lock)
-                    {
-                        SceneObjectGroupsByFullID[sceneObject.UUID] = sceneObject;
-                        SceneObjectGroupsByLocalID[sceneObject.LocalId] = sceneObject;
-                        foreach (SceneObjectPart part in sceneObject.Children.Values)
-                        {
-                            SceneObjectGroupsByFullID[part.UUID] = sceneObject;
-                            SceneObjectGroupsByLocalID[part.LocalId] = sceneObject;
-                        }
-                    }
 
-                    return true;
                 }
             }
 
+            if (found)
+            {
+                lock (m_dictionary_lock)
+                {
+                    SceneObjectGroupsByFullID[sceneObject.UUID] = sceneObject;
+                    SceneObjectGroupsByLocalID[sceneObject.LocalId] = sceneObject;
+                    foreach (SceneObjectPart part in parts)
+                    {
+                        SceneObjectGroupsByFullID[part.UUID] = sceneObject;
+                        SceneObjectGroupsByLocalID[part.LocalId] = sceneObject;
+                    }
+                }
+
+                if (OnObjectCreate != null)
+                    OnObjectCreate(sceneObject);
+
+                return true;
+            }
+
+
             return false;
         }