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/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs

@@ -351,7 +351,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 
                 // the avatar.Close below will clear the child region list. We need this below for (possibly)
                 // closing the child agents, so save it here (we need a copy as it is Clear()-ed).
-                //List<ulong> childRegions = new List<ulong>(avatar.GetKnownRegionList());
+                //List<ulong> childRegions = avatar.KnownRegionHandles;
                 // Compared to ScenePresence.CrossToNewRegion(), there's no obvious code to handle a teleport
                 // failure at this point (unlike a border crossing failure).  So perhaps this can never fail
                 // once we reach here...

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.RemoveNeighbourRegion(RegionInfo.RegionHandle);
-                                         avatar.KnownChildRegionHandles.Remove(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> regions = avatar.KnownRegionHandles;
-                        //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);
                         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);

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)
                     {

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))
+                // 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
-                    m_knownChildRegions.Remove(regionHandle);
+                //if (m_knownChildRegions.ContainsKey(regionHandle))
-                   //m_log.Debug(" !!! removing known region {0} in {1}. Count = {2}", regionHandle, Scene.RegionInfo.RegionName, m_knownChildRegions.Count);
+                //    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()
+        public Dictionary<ulong, string> KnownRegions
-        {
+        {
-            return new List<ulong>(m_knownChildRegions.Keys);
+            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);
+                knownRegions.Count, Scene.RegionInfo.RegionName);
-            //DumpKnownRegions();
+            //DumpKnownRegions();
-
+
-            lock (m_knownChildRegions)
+            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
+                    uint x, y;
-                    if (handle != Scene.RegionInfo.RegionHandle)
+                    Utils.LongToUInts(handle, out x, out y);
-                    {
+                    x = x / Constants.RegionSize;
-                        uint x, y;
+                    y = y / Constants.RegionSize;
-                        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("---> 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)));
+                    //m_log.Debug("---> y: " + y + "; newy:" + newRegionY + "; Abs:" + (int)Math.Abs((int)(y - newRegionY)));
-                        if (Util.IsOutsideView(DrawDistance, x, newRegionX, 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);
+                m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false);
-            
+
-            lock (m_knownChildRegions)
+            // Clear known regions
-            {
+            KnownRegions = new Dictionary<ulong, string>();
-                m_knownChildRegions.Clear();
-            }
 
             lock (m_reprioritization_timer)
             {

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