From 4748c19bdbcdcaf6050e1f04a5f7394a88e0bf3e Mon Sep 17 00:00:00 2001 From: Dan Lake Date: Thu, 6 Oct 2011 22:47:33 -0700 Subject: Refactored "known child region" in ScenePresence. There were 4 different ways to access the list/dictionary of child regions and locking was inconsistent. There are now public properties which enforce locks. Callers are no longer required to create new copies of lists. --- OpenSim/Region/Framework/Scenes/Scene.cs | 19 +-- .../Framework/Scenes/SceneCommunicationService.cs | 2 +- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 134 ++++++++++----------- .../Scenes/Tests/ScenePresenceAgentTests.cs | 11 +- 4 files changed, 78 insertions(+), 88 deletions(-) (limited to 'OpenSim/Region/Framework') diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index b1755ac..992e6c5 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -1109,9 +1109,8 @@ namespace OpenSim.Region.Framework.Scenes // Kick all ROOT agents with the message, 'The simulator is going down' ForEachScenePresence(delegate(ScenePresence avatar) - { - if (avatar.KnownChildRegionHandles.Contains(RegionInfo.RegionHandle)) - avatar.KnownChildRegionHandles.Remove(RegionInfo.RegionHandle); + { + avatar.RemoveNeighbourRegion(RegionInfo.RegionHandle); if (!avatar.IsChildAgent) avatar.ControllingClient.Kick("The simulator is going down."); @@ -3103,14 +3102,8 @@ namespace OpenSim.Region.Framework.Scenes avatar.Scene.NeedSceneCacheClear(avatar.UUID); if (closeChildAgents && !avatar.IsChildAgent) - { - //List childknownRegions = new List(); - //List ckn = avatar.KnownChildRegionHandles; - //for (int i = 0; i < ckn.Count; i++) - //{ - // childknownRegions.Add(ckn[i]); - //} - List regions = new List(avatar.KnownChildRegionHandles); + { + List regions = avatar.KnownRegionHandles; regions.Remove(RegionInfo.RegionHandle); m_sceneGridService.SendCloseChildAgentConnections(agentID, regions); } @@ -3181,7 +3174,7 @@ namespace OpenSim.Region.Framework.Scenes { for (int i = 0; i < regionslst.Count; i++) { - av.KnownChildRegionHandles.Remove(regionslst[i]); + av.RemoveNeighbourRegion(regionslst[i]); } } } @@ -3674,7 +3667,7 @@ namespace OpenSim.Region.Framework.Scenes if (RegionSecret == loggingOffUser.ControllingClient.SecureSessionId || (parsedsecret && RegionSecret == localRegionSecret)) { - m_sceneGridService.SendCloseChildAgentConnections(loggingOffUser.UUID, new List(loggingOffUser.KnownRegions.Keys)); + m_sceneGridService.SendCloseChildAgentConnections(loggingOffUser.UUID, loggingOffUser.KnownRegionHandles); loggingOffUser.ControllingClient.Kick(message); // Give them a second to receive the message! Thread.Sleep(1000); diff --git a/OpenSim/Region/Framework/Scenes/SceneCommunicationService.cs b/OpenSim/Region/Framework/Scenes/SceneCommunicationService.cs index 837e655..7cffa70 100644 --- a/OpenSim/Region/Framework/Scenes/SceneCommunicationService.cs +++ b/OpenSim/Region/Framework/Scenes/SceneCommunicationService.cs @@ -229,7 +229,7 @@ namespace OpenSim.Region.Framework.Scenes { uint x = 0, y = 0; List simulatorList = new List(); - foreach (ulong regionHandle in presence.KnownChildRegionHandles) + foreach (ulong regionHandle in presence.KnownRegionHandles) { if (regionHandle != m_regionInfo.RegionHandle) { diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index e4e5f17..e7ef377 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -219,11 +219,6 @@ namespace OpenSim.Region.Framework.Scenes } } - - // neighbouring regions we have enabled a child agent in - // holds the seed cap for the child agent in that region - private Dictionary m_knownChildRegions = new Dictionary(); - /// /// Copy of the script states while the agent is in transit. This state may /// need to be placed back in case of transfer fail. @@ -635,29 +630,6 @@ namespace OpenSim.Region.Framework.Scenes set { m_health = value; } } - /// - /// These are the region handles known by the avatar. - /// - public List KnownChildRegionHandles - { - get - { - if (m_knownChildRegions.Count == 0) - return new List(); - else - return new List(m_knownChildRegions.Keys); - } - } - - public Dictionary KnownRegions - { - get { return m_knownChildRegions; } - set - { - m_knownChildRegions = value; - } - } - private ISceneViewer m_sceneViewer; public ISceneViewer SceneViewer @@ -1103,7 +1075,11 @@ namespace OpenSim.Region.Framework.Scenes public void StopFlying() { ControllingClient.StopFlying(this); - } + } + + // neighbouring regions we have enabled a child agent in + // holds the seed cap for the child agent in that region + private Dictionary m_knownChildRegions = new Dictionary(); public void AddNeighbourRegion(ulong regionHandle, string cap) { @@ -1119,14 +1095,14 @@ namespace OpenSim.Region.Framework.Scenes } public void RemoveNeighbourRegion(ulong regionHandle) - { + { lock (m_knownChildRegions) { - if (m_knownChildRegions.ContainsKey(regionHandle)) - { - m_knownChildRegions.Remove(regionHandle); - //m_log.Debug(" !!! removing known region {0} in {1}. Count = {2}", regionHandle, Scene.RegionInfo.RegionName, m_knownChildRegions.Count); - } + // Checking ContainsKey is redundant as Remove works either way and returns a bool + // This is here to allow the Debug output to be conditional on removal + //if (m_knownChildRegions.ContainsKey(regionHandle)) + // m_log.DebugFormat(" !!! removing known region {0} in {1}. Count = {2}", regionHandle, Scene.RegionInfo.RegionName, m_knownChildRegions.Count); + m_knownChildRegions.Remove(regionHandle); } } @@ -1137,11 +1113,39 @@ namespace OpenSim.Region.Framework.Scenes RemoveNeighbourRegion(handle); Scene.CapsModule.DropChildSeed(UUID, handle); } - } - - public List GetKnownRegionList() - { - return new List(m_knownChildRegions.Keys); + } + + public Dictionary KnownRegions + { + get + { + lock (m_knownChildRegions) + return new Dictionary(m_knownChildRegions); + } + set + { + // Replacing the reference is atomic but we still need to lock on + // the original dictionary object which may be in use elsewhere + lock (m_knownChildRegions) + m_knownChildRegions = value; + } + } + + public List KnownRegionHandles + { + get + { + return new List(KnownRegions.Keys); + } + } + + public int KnownRegionCount + { + get + { + lock (m_knownChildRegions) + return m_knownChildRegions.Count; + } } #endregion @@ -2730,7 +2734,7 @@ namespace OpenSim.Region.Framework.Scenes // Throttles float multiplier = 1; - int childRegions = m_knownChildRegions.Count; + int childRegions = KnownRegionCount; if (childRegions != 0) multiplier = 1f / childRegions; @@ -2982,30 +2986,28 @@ namespace OpenSim.Region.Framework.Scenes /// public void CloseChildAgents(uint newRegionX, uint newRegionY) { - List byebyeRegions = new List(); + List byebyeRegions = new List(); + List knownRegions = KnownRegionHandles; m_log.DebugFormat( "[SCENE PRESENCE]: Closing child agents. Checking {0} regions in {1}", - m_knownChildRegions.Keys.Count, Scene.RegionInfo.RegionName); - //DumpKnownRegions(); - - lock (m_knownChildRegions) + knownRegions.Count, Scene.RegionInfo.RegionName); + //DumpKnownRegions(); + + foreach (ulong handle in knownRegions) { - foreach (ulong handle in m_knownChildRegions.Keys) + // Don't close the agent on this region yet + if (handle != Scene.RegionInfo.RegionHandle) { - // Don't close the agent on this region yet - if (handle != Scene.RegionInfo.RegionHandle) + uint x, y; + Utils.LongToUInts(handle, out x, out y); + x = x / Constants.RegionSize; + y = y / Constants.RegionSize; + + //m_log.Debug("---> x: " + x + "; newx:" + newRegionX + "; Abs:" + (int)Math.Abs((int)(x - newRegionX))); + //m_log.Debug("---> y: " + y + "; newy:" + newRegionY + "; Abs:" + (int)Math.Abs((int)(y - newRegionY))); + if (Util.IsOutsideView(DrawDistance, x, newRegionX, y, newRegionY)) { - uint x, y; - Utils.LongToUInts(handle, out x, out y); - x = x / Constants.RegionSize; - y = y / Constants.RegionSize; - - //m_log.Debug("---> x: " + x + "; newx:" + newRegionX + "; Abs:" + (int)Math.Abs((int)(x - newRegionX))); - //m_log.Debug("---> y: " + y + "; newy:" + newRegionY + "; Abs:" + (int)Math.Abs((int)(y - newRegionY))); - if (Util.IsOutsideView(DrawDistance, x, newRegionX, y, newRegionY)) - { - byebyeRegions.Add(handle); - } + byebyeRegions.Add(handle); } } } @@ -3123,7 +3125,7 @@ namespace OpenSim.Region.Framework.Scenes // Throttles float multiplier = 1; - int childRegions = m_knownChildRegions.Count; + int childRegions = KnownRegionCount; if (childRegions != 0) multiplier = 1f / childRegions; @@ -3432,12 +3434,10 @@ namespace OpenSim.Region.Framework.Scenes public void Close() { if (!IsChildAgent) - m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false); - - lock (m_knownChildRegions) - { - m_knownChildRegions.Clear(); - } + m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false); + + // Clear known regions + KnownRegions = new Dictionary(); lock (m_reprioritization_timer) { diff --git a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs index ce9d418..119eb1f 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceAgentTests.cs @@ -215,11 +215,9 @@ namespace OpenSim.Region.Framework.Scenes.Tests string cap = presence.ControllingClient.RequestClientInfo().CapsPath; presence.AddNeighbourRegion(region2, cap); - presence.AddNeighbourRegion(region3, cap); - - List neighbours = presence.GetKnownRegionList(); - - Assert.That(neighbours.Count, Is.EqualTo(2)); + presence.AddNeighbourRegion(region3, cap); + + Assert.That(presence.KnownRegionCount, Is.EqualTo(2)); } [Test] @@ -230,8 +228,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests ScenePresence presence = scene.GetScenePresence(agent1); presence.RemoveNeighbourRegion(region3); - List neighbours = presence.GetKnownRegionList(); - Assert.That(neighbours.Count,Is.EqualTo(1)); + Assert.That(presence.KnownRegionCount,Is.EqualTo(1)); /* presence.MakeChildAgent; presence.MakeRootAgent; -- cgit v1.1