Fix bug where crossing to a neighbouring region and back again would trigger an exception, and a second recross would stop the user moving until relog

Also fixes an issue where sitting avatar counts became inaccurate after any cross.
Part of the problem was due to cloning code using MemberwiseClone() but not resetting certain collection structures.
Adds regression test for this case.
In relation to http://opensimulator.org/mantis/view.php?id=7050

4 files changed, 138 insertions, 8 deletions

diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
index b70e9df..4dc724d 100644
--- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
@@ -552,12 +552,29 @@ namespace OpenSim.Region.Framework.Scenes
 
                                         av.IsInTransit = true;
 
-                                        CrossAgentToNewRegionDelegate d = entityTransfer.CrossAgentToNewRegionAsync;
-                                        d.BeginInvoke(av, val, destination, av.Flying, version, CrossAgentToNewRegionCompleted, d);
+                                        // A temporary measure to allow regression tests to work.
+                                        // Quite possibly, all BeginInvoke() calls should be replaced by Util.FireAndForget
+                                        // or similar since BeginInvoke() always uses the system threadpool to launch
+                                        // threads rather than any replace threadpool that we might be using.
+                                        if (Util.FireAndForgetMethod == FireAndForgetMethod.RegressionTest)
+                                        { 
+                                            entityTransfer.CrossAgentToNewRegionAsync(av, val, destination, av.Flying, version);
+                                            CrossAgentToNewRegionCompleted(av);
+                                        }
+                                        else
+                                        {
+                                            CrossAgentToNewRegionDelegate d = entityTransfer.CrossAgentToNewRegionAsync;
+                                            d.BeginInvoke(
+                                                av, val, destination, av.Flying, version, 
+                                                ar => CrossAgentToNewRegionCompleted(d.EndInvoke(ar)), null);
+                                        }
                                     }
                                     else
+                                    {
                                         m_log.DebugFormat("[SCENE OBJECT]: Crossing avatar alreasy in transit {0} to {1}", av.Name, val);
+                                    }
                                 }
+
                                 avsToCross.Clear();
                                 return;
                             }
@@ -630,11 +647,8 @@ namespace OpenSim.Region.Framework.Scenes
             set { RootPart.Velocity = value; }
         }
 
-        private void CrossAgentToNewRegionCompleted(IAsyncResult iar)
+        private void CrossAgentToNewRegionCompleted(ScenePresence agent)
         {
-            CrossAgentToNewRegionDelegate icon = (CrossAgentToNewRegionDelegate)iar.AsyncState;
-            ScenePresence agent = icon.EndInvoke(iar);
-
             //// If the cross was successful, this agent is a child agent
             if (agent.IsChildAgent)
             {
@@ -1698,10 +1712,15 @@ namespace OpenSim.Region.Framework.Scenes
         /// <returns></returns>
         public SceneObjectGroup Copy(bool userExposed)
         {
+            // FIXME: This is dangerous since it's easy to forget to reset some references when necessary and end up 
+            // with bugs that only occur in some circumstances (e.g. crossing between regions on the same simulator
+            // but not between regions on different simulators).  Really, all copying should be done explicitly.
             SceneObjectGroup dupe = (SceneObjectGroup)MemberwiseClone();
+
             dupe.Backup = false;
             dupe.m_parts = new MapAndArray<OpenMetaverse.UUID, SceneObjectPart>();
             dupe.m_sittingAvatars = new List<UUID>();
+            dupe.m_linkedAvatars = new List<ScenePresence>();
             dupe.CopyRootPart(m_rootPart, OwnerID, GroupID, userExposed);
             dupe.m_rootPart.LinkNum = m_rootPart.LinkNum;
 
diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs
index db4e285..c06175e 100644
--- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs
@@ -1753,7 +1753,11 @@ namespace OpenSim.Region.Framework.Scenes
         /// <returns></returns>
         public SceneObjectPart Copy(uint localID, UUID AgentID, UUID GroupID, int linkNum, bool userExposed)
         {
+            // FIXME: This is dangerous since it's easy to forget to reset some references when necessary and end up 
+            // with bugs that only occur in some circumstances (e.g. crossing between regions on the same simulator
+            // but not between regions on different simulators).  Really, all copying should be done explicitly.
             SceneObjectPart dupe = (SceneObjectPart)MemberwiseClone();
+
             dupe.m_shape = m_shape.Copy();
             dupe.m_regionHandle = m_regionHandle;
             if (userExposed)
@@ -1799,6 +1803,8 @@ namespace OpenSim.Region.Framework.Scenes
             Array.Copy(Shape.ExtraParams, extraP, extraP.Length);
             dupe.Shape.ExtraParams = extraP;
 
+            dupe.m_sittingAvatars = new HashSet<OpenMetaverse.UUID>();
+
             // safeguard  actual copy is done in sog.copy
             dupe.KeyframeMotion = null;
             dupe.PayPrice = (int[])PayPrice.Clone();
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index 080cdb4..6386a45 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -1023,6 +1023,7 @@ namespace OpenSim.Region.Framework.Scenes
                     else
                     {
                         part.ParentGroup.AddAvatar(UUID);
+                        part.AddSittingAvatar(UUID);
                         if (part.SitTargetPosition != Vector3.Zero)
                             part.SitTargetAvatar = UUID;
     //                    ParentPosition = part.GetWorldPosition();
@@ -2848,7 +2849,6 @@ namespace OpenSim.Region.Framework.Scenes
             part.ParentGroup.TriggerScriptChangedEvent(Changed.LINK);
         }
 
-
         public void HandleAgentSit(IClientAPI remoteClient, UUID agentID)
         {
             if (IsChildAgent)
diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs
index 4d07741..d65b0b6 100644
--- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs
+++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs
@@ -26,10 +26,12 @@
  */
 
 using System;
+using System.Collections.Generic;
 using Nini.Config;
 using NUnit.Framework;
 using OpenMetaverse;
 using OpenSim.Framework;
+using OpenSim.Region.CoreModules.Framework;
 using OpenSim.Region.CoreModules.Framework.EntityTransfer;
 using OpenSim.Region.CoreModules.ServiceConnectorsOut.Simulation;
 using OpenSim.Region.CoreModules.World.Land;
@@ -101,7 +103,110 @@ namespace OpenSim.Region.Framework.Scenes.Tests
         /// Test cross with no prim limit module.
         /// </summary>
         /// <remarks>
-        /// XXX: This test may be better off in a specific PrimLimitsModuleTest class in optional module tests in the
+        /// Possibly this should belong in ScenePresenceCrossingTests, though here it is the object that is being moved
+        /// where the avatar is just a passenger.
+        /// </remarks>
+        [Test]
+        public void TestCrossOnSameSimulatorWithSittingAvatar()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+            int sceneObjectIdTail = 0x2;
+            Vector3 so1StartPos = new Vector3(128, 10, 20);
+
+            EntityTransferModule etmA = new EntityTransferModule();
+            EntityTransferModule etmB = new EntityTransferModule();
+            LocalSimulationConnectorModule lscm = new LocalSimulationConnectorModule();
+
+            IConfigSource config = new IniConfigSource();
+            IConfig modulesConfig = config.AddConfig("Modules");
+            modulesConfig.Set("EntityTransferModule", etmA.Name);
+            modulesConfig.Set("SimulationServices", lscm.Name);
+            IConfig entityTransferConfig = config.AddConfig("EntityTransfer");
+
+            // In order to run a single threaded regression test we do not want the entity transfer module waiting
+            // for a callback from the destination scene before removing its avatar data.
+            entityTransferConfig.Set("wait_for_callback", false);
+
+            SceneHelpers sh = new SceneHelpers();
+            TestScene sceneA = sh.SetupScene("sceneA", TestHelpers.ParseTail(0x100), 1000, 1000);
+            TestScene sceneB = sh.SetupScene("sceneB", TestHelpers.ParseTail(0x200), 1000, 999);
+
+            SceneHelpers.SetupSceneModules(new Scene[] { sceneA, sceneB }, config, lscm);
+            SceneHelpers.SetupSceneModules(sceneA, config, new CapabilitiesModule(), etmA);
+            SceneHelpers.SetupSceneModules(sceneB, config, new CapabilitiesModule(), etmB);
+
+            SceneObjectGroup so1 = SceneHelpers.AddSceneObject(sceneA, 1, userId, "", sceneObjectIdTail);
+            UUID so1Id = so1.UUID;
+            so1.AbsolutePosition = so1StartPos;
+
+            AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+            TestClient tc = new TestClient(acd, sceneA);
+            List<TestClient> destinationTestClients = new List<TestClient>();
+            EntityTransferHelpers.SetupInformClientOfNeighbourTriggersNeighbourClientCreate(tc, destinationTestClients);
+
+            ScenePresence sp1SceneA = SceneHelpers.AddScenePresence(sceneA, tc, acd);
+            sp1SceneA.AbsolutePosition = so1StartPos;
+            sp1SceneA.HandleAgentRequestSit(sp1SceneA.ControllingClient, sp1SceneA.UUID, so1.UUID, Vector3.Zero);
+
+            // Cross
+            sceneA.SceneGraph.UpdatePrimGroupPosition(
+                so1.LocalId, new Vector3(so1StartPos.X, so1StartPos.Y - 20, so1StartPos.Z), userId);
+
+            SceneObjectGroup so1PostCross;
+
+            {
+                ScenePresence sp1SceneAPostCross = sceneA.GetScenePresence(userId);
+                Assert.IsTrue(sp1SceneAPostCross.IsChildAgent, "sp1SceneAPostCross.IsChildAgent unexpectedly false");
+
+                ScenePresence sp1SceneBPostCross = sceneB.GetScenePresence(userId);
+                TestClient sceneBTc = ((TestClient)sp1SceneBPostCross.ControllingClient);
+                sceneBTc.CompleteMovement();
+
+                Assert.IsFalse(sp1SceneBPostCross.IsChildAgent, "sp1SceneAPostCross.IsChildAgent unexpectedly true");
+                Assert.IsTrue(sp1SceneBPostCross.IsSatOnObject);
+
+                Assert.IsNull(sceneA.GetSceneObjectGroup(so1Id), "uck");
+                so1PostCross = sceneB.GetSceneObjectGroup(so1Id);
+                Assert.NotNull(so1PostCross);
+                Assert.AreEqual(1, so1PostCross.GetSittingAvatarsCount());
+                Assert.AreEqual(1, so1PostCross.GetLinkedAvatars().Count);
+            }
+
+            Vector3 so1PostCrossPos = so1PostCross.AbsolutePosition;
+
+//            Console.WriteLine("CRISSCROSS");
+
+            // Recross
+            sceneB.SceneGraph.UpdatePrimGroupPosition(
+                so1PostCross.LocalId, new Vector3(so1PostCrossPos.X, so1PostCrossPos.Y + 20, so1PostCrossPos.Z), userId);
+
+            {
+                ScenePresence sp1SceneBPostReCross = sceneB.GetScenePresence(userId);
+                Assert.IsTrue(sp1SceneBPostReCross.IsChildAgent, "sp1SceneBPostReCross.IsChildAgent unexpectedly false");
+
+                ScenePresence sp1SceneAPostReCross = sceneA.GetScenePresence(userId);
+                TestClient sceneATc = ((TestClient)sp1SceneAPostReCross.ControllingClient);
+                sceneATc.CompleteMovement();
+
+                Assert.IsFalse(sp1SceneAPostReCross.IsChildAgent, "sp1SceneAPostCross.IsChildAgent unexpectedly true");
+                Assert.IsTrue(sp1SceneAPostReCross.IsSatOnObject);
+
+                Assert.IsNull(sceneB.GetSceneObjectGroup(so1Id), "uck2");
+                SceneObjectGroup so1PostReCross = sceneA.GetSceneObjectGroup(so1Id);
+                Assert.NotNull(so1PostReCross);
+                Assert.AreEqual(1, so1PostReCross.GetSittingAvatarsCount());
+                Assert.AreEqual(1, so1PostReCross.GetLinkedAvatars().Count);
+            }
+        }
+
+        /// <summary>
+        /// Test cross with no prim limit module.
+        /// </summary>
+        /// <remarks>
+        /// XXX: This test may FCbe better off in a specific PrimLimitsModuleTest class in optional module tests in the
         /// future (though it is configured as active by default, so not really optional).
         /// </remarks>
         [Test]