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.

4 files changed, 78 insertions, 88 deletions

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<ulong> childknownRegions = new List<ulong>();
-                        //List<ulong> ckn = avatar.KnownChildRegionHandles;
-                        //for (int i = 0; i < ckn.Count; i++)
-                        //{
-                        //    childknownRegions.Add(ckn[i]);
-                        //}
-                        List<ulong> regions = new List<ulong>(avatar.KnownChildRegionHandles);
+                    {

+                        List<ulong> 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<ulong>(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<string> simulatorList = new List<string>();
-                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<ulong, string> m_knownChildRegions = new Dictionary<ulong, string>();
-
         /// <summary>
         /// 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; }
         }
 
-        /// <summary>
-        /// These are the region handles known by the avatar.
-        /// </summary>
-        public List<ulong> KnownChildRegionHandles
-        {
-            get 
-            {
-                if (m_knownChildRegions.Count == 0) 
-                    return new List<ulong>();
-                else
-                    return new List<ulong>(m_knownChildRegions.Keys); 
-            }
-        }
-
-        public Dictionary<ulong, string> 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<ulong, string> m_knownChildRegions = new Dictionary<ulong, string>();
 
         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<ulong> GetKnownRegionList()
-        {
-            return new List<ulong>(m_knownChildRegions.Keys);
+        }

+

+        public Dictionary<ulong, string> KnownRegions

+        {

+            get

+            {

+                lock (m_knownChildRegions)

+                    return new Dictionary<ulong, string>(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<ulong> KnownRegionHandles

+        {

+            get

+            {

+                return new List<ulong>(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
         /// <returns></returns>
         public void CloseChildAgents(uint newRegionX, uint newRegionY)
         {
-            List<ulong> byebyeRegions = new List<ulong>();
+            List<ulong> byebyeRegions = new List<ulong>();

+            List<ulong> 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<ulong, string>();

 
             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<ulong> 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<ulong> neighbours = presence.GetKnownRegionList();
-            Assert.That(neighbours.Count,Is.EqualTo(1));
+            Assert.That(presence.KnownRegionCount,Is.EqualTo(1));
             /*
             presence.MakeChildAgent;
             presence.MakeRootAgent;