From e992ca025571a891333a57012c2cd4419b6581e5 Mon Sep 17 00:00:00 2001 From: Diva Canto Date: Tue, 6 Oct 2009 15:39:53 -0700 Subject: Rewrote parts of the code that were double-locking different objects. This is about half of the code base reviewed. --- .../Cache/UserProfileCacheService.cs | 66 +++++------- .../Region/ClientStack/LindenUDP/LLClientView.cs | 116 ++++++++++++--------- .../CoreModules/Avatar/Friends/FriendsModule.cs | 42 ++++---- .../Avatar/InstantMessage/PresenceModule.cs | 11 +- .../Inventory/Transfer/InventoryTransferModule.cs | 15 +-- .../Scripting/EMailModules/EmailModule.cs | 37 +++---- .../CoreModules/World/Land/LandManagementModule.cs | 8 +- .../Scenes/AsyncSceneObjectGroupDeleter.cs | 36 ++++--- OpenSim/Region/Framework/Scenes/SceneGraph.cs | 36 ++++--- 9 files changed, 201 insertions(+), 166 deletions(-) diff --git a/OpenSim/Framework/Communications/Cache/UserProfileCacheService.cs b/OpenSim/Framework/Communications/Cache/UserProfileCacheService.cs index 9e12d948..b02cf5b 100644 --- a/OpenSim/Framework/Communications/Cache/UserProfileCacheService.cs +++ b/OpenSim/Framework/Communications/Cache/UserProfileCacheService.cs @@ -128,24 +128,18 @@ namespace OpenSim.Framework.Communications.Cache /// null if no user details are found 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; } /// @@ -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) - return AddToCaches(userProfile); - else - return null; - } - } + + UserProfileData userProfile = m_commsManager.UserService.GetUserProfile(userID); + if (userProfile != null) + return AddToCaches(userProfile); + else + return null; } /// @@ -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) - { - m_userProfilesByName[createdUserInfo.UserProfile.Name] = createdUserInfo; - } - } + + lock (m_userProfilesByName) + m_userProfilesByName[createdUserInfo.UserProfile.Name] = createdUserInfo; return createdUserInfo; } @@ -230,21 +214,25 @@ namespace OpenSim.Framework.Communications.Cache /// true if there was a profile to remove, false otherwise 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) + } + } + + if (userInfo != null) + lock (m_userProfilesByName) + { + if (m_userProfilesByName.ContainsKey(userInfo.UserProfile.Name)) { m_userProfilesByName.Remove(userInfo.UserProfile.Name); + return true; } - - return true; } - } return false; } diff --git a/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs b/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs index 0052729..fe37a08 100644 --- a/OpenSim/Region/ClientStack/LindenUDP/LLClientView.cs +++ b/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) - { - lock (m_primFullUpdateTimer) - m_primFullUpdateTimer.Stop(); - - return; - } + stopTimer = true; + } + if (stopTimer) + { + lock (m_primFullUpdateTimer) + m_primFullUpdateTimer.Stop(); + return; + } - ObjectUpdatePacket outPacket = - (ObjectUpdatePacket)PacketPool.Instance.GetPacket( - PacketType.ObjectUpdate); + ObjectUpdatePacket outPacket = + (ObjectUpdatePacket)PacketPool.Instance.GetPacket( + PacketType.ObjectUpdate); - outPacket.RegionData.RegionHandle = - Scene.RegionInfo.RegionHandle; - outPacket.RegionData.TimeDilation = - (ushort)(Scene.TimeDilation * ushort.MaxValue); + outPacket.RegionData.RegionHandle = + Scene.RegionInfo.RegionHandle; + outPacket.RegionData.TimeDilation = + (ushort)(Scene.TimeDilation * ushort.MaxValue); - int max = m_primFullUpdates.Count; - if (max > m_primFullUpdatesPerPacket) - max = m_primFullUpdatesPerPacket; + int max = m_primFullUpdates.Count; + if (max > m_primFullUpdatesPerPacket) + max = m_primFullUpdatesPerPacket; - int count = 0; - int size = 0; + int count = 0; + int size = 0; - byte[] zerobuffer = new byte[1024]; - 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) - m_primFullUpdateTimer.Stop(); + stopTimer = true; } + + if (stopTimer) + lock (m_primFullUpdateTimer) + m_primFullUpdateTimer.Stop(); } /// @@ -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) - { - lock (m_primTerseUpdateTimer) - m_primTerseUpdateTimer.Stop(); + stopTimer = true; + } + if (stopTimer) + { + lock (m_primTerseUpdateTimer) + m_primTerseUpdateTimer.Stop(); - return; - } + return; + } - ImprovedTerseObjectUpdatePacket outPacket = - (ImprovedTerseObjectUpdatePacket) - PacketPool.Instance.GetPacket( - PacketType.ImprovedTerseObjectUpdate); + ImprovedTerseObjectUpdatePacket outPacket = + (ImprovedTerseObjectUpdatePacket) + PacketPool.Instance.GetPacket( + PacketType.ImprovedTerseObjectUpdate); - outPacket.RegionData.RegionHandle = - Scene.RegionInfo.RegionHandle; - outPacket.RegionData.TimeDilation = - (ushort)(Scene.TimeDilation * ushort.MaxValue); + outPacket.RegionData.RegionHandle = + Scene.RegionInfo.RegionHandle; + outPacket.RegionData.TimeDilation = + (ushort)(Scene.TimeDilation * ushort.MaxValue); - int max = m_primTerseUpdates.Count; - if (max > m_primTerseUpdatesPerPacket) - max = m_primTerseUpdatesPerPacket; + int max = m_primTerseUpdates.Count; + if (max > m_primTerseUpdatesPerPacket) + max = m_primTerseUpdatesPerPacket; - int count = 0; - int size = 0; + int count = 0; + int size = 0; - byte[] zerobuffer = new byte[1024]; - byte[] blockbuffer = new byte[1024]; + byte[] zerobuffer = 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) - m_primTerseUpdateTimer.Stop(); + stopTimer = true; } + if (stopTimer) + lock (m_primTerseUpdateTimer) + m_primTerseUpdateTimer.Stop(); } public void FlushPrimUpdates() diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs index fc7d63a..4abad81 100644 --- a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs +++ b/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 scenes = null; lock (m_scenes) + scenes = new List(m_scenes.Values); + + ScenePresence returnAgent = null; + ScenePresence queryagent = null; + foreach (Scene scene in scenes) { - ScenePresence queryagent = null; - foreach (Scene scene in m_scenes.Values) + queryagent = scene.GetScenePresence(AgentID); + if (queryagent != null) { - queryagent = scene.GetScenePresence(AgentID); - if (queryagent != null) + if (!queryagent.IsChildAgent) { - if (!queryagent.IsChildAgent) - { - returnAgent = queryagent; - break; - } + returnAgent = queryagent; + break; } } } + return returnAgent; } private ScenePresence GetAnyPresenceFromAgentID(UUID AgentID) { - ScenePresence returnAgent = null; + List scenes = null; lock (m_scenes) + scenes = new List(m_scenes.Values); + + ScenePresence returnAgent = null; + ScenePresence queryagent = null; + foreach (Scene scene in m_scenes.Values) { - ScenePresence queryagent = null; - foreach (Scene scene in m_scenes.Values) + queryagent = scene.GetScenePresence(AgentID); + if (queryagent != null) { - queryagent = scene.GetScenePresence(AgentID); - if (queryagent != null) - { - returnAgent = queryagent; - break; - } + returnAgent = queryagent; + break; } } + return returnAgent; } diff --git a/OpenSim/Region/CoreModules/Avatar/InstantMessage/PresenceModule.cs b/OpenSim/Region/CoreModules/Avatar/InstantMessage/PresenceModule.cs index ad05bab..42dd7ff 100644 --- a/OpenSim/Region/CoreModules/Avatar/InstantMessage/PresenceModule.cs +++ b/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 scenes = null; lock (m_Scenes) + scenes = new List(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... diff --git a/OpenSim/Region/CoreModules/Avatar/Inventory/Transfer/InventoryTransferModule.cs b/OpenSim/Region/CoreModules/Avatar/Inventory/Transfer/InventoryTransferModule.cs index d9a021f..e6e0483 100644 --- a/OpenSim/Region/CoreModules/Avatar/Inventory/Transfer/InventoryTransferModule.cs +++ b/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 scenes = null; lock (m_Scenelist) + scenes = new List(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 != null) - { - if (!presence.IsChildAgent) - return scene; - } + if (!presence.IsChildAgent) + return scene; } } return null; diff --git a/OpenSim/Region/CoreModules/Scripting/EMailModules/EmailModule.cs b/OpenSim/Region/CoreModules/Scripting/EMailModules/EmailModule.cs index 83f004d..f9f01fe 100644 --- a/OpenSim/Region/CoreModules/Scripting/EMailModules/EmailModule.cs +++ b/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 scenes = null; lock (m_Scenes) + scenes = new List(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); - if (part != null) - { - ObjectRegionName = s.RegionInfo.RegionName; - uint localX = (s.RegionInfo.RegionLocX * (int)Constants.RegionSize); - uint localY = (s.RegionInfo.RegionLocY * (int)Constants.RegionSize); - ObjectRegionName = ObjectRegionName + " (" + localX + ", " + localY + ")"; - return part; - } + ObjectRegionName = s.RegionInfo.RegionName; + uint localX = (s.RegionInfo.RegionLocX * (int)Constants.RegionSize); + 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 queue = null; + List removal = new List(); 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 removal = new List(); 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) - { - m_MailQueues.Remove(remove); - } - } + + lock (m_MailQueues) + m_MailQueues.Remove(remove); } lock (m_MailQueues) diff --git a/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs b/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs index d2b5cb1..0fed1bd 100644 --- a/OpenSim/Region/CoreModules/World/Land/LandManagementModule.cs +++ b/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 /// Land.localID of the peice of land to remove. 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) diff --git a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs b/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs index 5b571c7..df9473d 100644 --- a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs +++ b/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,23 +137,26 @@ 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) diff --git a/OpenSim/Region/Framework/Scenes/SceneGraph.cs b/OpenSim/Region/Framework/Scenes/SceneGraph.cs index 54ac792..20b3b5c 100644 --- a/OpenSim/Region/Framework/Scenes/SceneGraph.cs +++ b/OpenSim/Region/Framework/Scenes/SceneGraph.cs @@ -297,34 +297,44 @@ namespace OpenSim.Region.Framework.Scenes sceneObject.AttachToScene(m_parentScene); + List 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) - OnObjectCreate(sceneObject); - - lock (m_dictionary_lock) + parts = new List(sceneObject.Children.Values); + + } + } + + if (found) + { + lock (m_dictionary_lock) + { + SceneObjectGroupsByFullID[sceneObject.UUID] = sceneObject; + SceneObjectGroupsByLocalID[sceneObject.LocalId] = sceneObject; + foreach (SceneObjectPart part in parts) { - SceneObjectGroupsByFullID[sceneObject.UUID] = sceneObject; - SceneObjectGroupsByLocalID[sceneObject.LocalId] = sceneObject; - foreach (SceneObjectPart part in sceneObject.Children.Values) - { - SceneObjectGroupsByFullID[part.UUID] = sceneObject; - SceneObjectGroupsByLocalID[part.LocalId] = sceneObject; - } + SceneObjectGroupsByFullID[part.UUID] = sceneObject; + SceneObjectGroupsByLocalID[part.LocalId] = sceneObject; } - - return true; } + + if (OnObjectCreate != null) + OnObjectCreate(sceneObject); + + return true; } + return false; } -- cgit v1.1