Merge branch 'master' of /home/opensim/var/repo/opensim

OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs

@@ -46,6 +46,29 @@ using Nini.Config;
 
 namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 {
+    /// <summary>
+    /// The possible states that an agent can be in when its being transferred between regions.
+    /// </summary>
+    /// <remarks>
+    /// This is a state machine.
+    ///
+    /// [Entry]               => Preparing
+    /// Preparing             => { Transferring || CleaningUp || [Exit] }
+    /// Transferring          => { ReceivedAtDestination || CleaningUp }
+    /// ReceivedAtDestination => CleaningUp
+    /// CleaningUp            => [Exit]
+    ///
+    /// In other words, agents normally travel throwing Preparing => Transferring => ReceivedAtDestination => CleaningUp
+    /// However, any state can transition to CleaningUp if the teleport has failed.
+    /// </remarks>
+    enum AgentTransferState
+    {
+        Preparing,              // The agent is being prepared for transfer
+        Transferring,           // The agent is in the process of being transferred to a destination
+        ReceivedAtDestination,  // The destination has notified us that the agent has been successfully received
+        CleaningUp              // The agent is being changed to child/removed after a transfer
+    }
+
     public class EntityTransferModule : INonSharedRegionModule, IEntityTransferModule
     {
         private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
@@ -68,7 +91,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 
         protected Scene m_scene;
 
-        protected List<UUID> m_agentsInTransit;
+        private Dictionary<UUID, AgentTransferState> m_agentsInTransit;
 
         private ExpiringCache<UUID, ExpiringCache<ulong, DateTime>> m_bannedRegions =
                 new ExpiringCache<UUID, ExpiringCache<ulong, DateTime>>();
@@ -120,7 +143,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                 MaxTransferDistance = DefaultMaxTransferDistance;
             }
 
-            m_agentsInTransit = new List<UUID>();
+            m_agentsInTransit = new Dictionary<UUID, AgentTransferState>();
             m_Enabled = true;
         }
 
@@ -259,17 +282,22 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                 position.Z = newPosZ;
             }
 
+            UpdateInTransit(sp.UUID, AgentTransferState.Transferring);
+
             sp.ControllingClient.SendTeleportStart(teleportFlags);
 
             sp.ControllingClient.SendLocalTeleport(position, lookAt, teleportFlags);
             sp.Velocity = Vector3.Zero;
             sp.Teleport(position);
 
+            UpdateInTransit(sp.UUID, AgentTransferState.ReceivedAtDestination);
+
             foreach (SceneObjectGroup grp in sp.GetAttachments())
             {
                 sp.Scene.EventManager.TriggerOnScriptChangedEvent(grp.LocalId, (uint)Changed.TELEPORT);
             }
 
+            UpdateInTransit(sp.UUID, AgentTransferState.CleaningUp);
             ResetFromTransit(sp.UUID);            
         }
 
@@ -508,6 +536,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                 return;
             }
 
+            // Past this point we have to attempt clean up if the teleport fails, so update transfer state.
+            UpdateInTransit(sp.UUID, AgentTransferState.Transferring);
+
             // OK, it got this agent. Let's close some child agents
             sp.CloseChildAgents(newRegionX, newRegionY);
 
@@ -598,6 +629,8 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                 return;
             }
 
+            UpdateInTransit(sp.UUID, AgentTransferState.CleaningUp);
+
             // For backwards compatibility
             if (version == "Unknown" || version == string.Empty)
             {
@@ -624,8 +657,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                 // We need to delay here because Imprudence viewers, unlike v1 or v3, have a short (<200ms, <500ms) delay before
                 // they regard the new region as the current region after receiving the AgentMovementComplete
                 // response.  If close is sent before then, it will cause the viewer to quit instead.
-                // However, if this delay is longer, then a viewer can teleport back to this region and experience
+                //
-                // a failure because the old ScenePresence has not yet been cleaned up.
+                // This sleep can be increased if necessary.  However, whilst it's active,
+                // an agent cannot teleport back to this region if it has teleported away.
                 Thread.Sleep(2000);
 
                 sp.Close();
@@ -644,6 +678,8 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                     "[ENTITY TRANSFER MODULE]: User {0} is going to another region, profile cache removed",
                     sp.UUID);
             }
+
+            ResetFromTransit(sp.UUID);
         }
 
         protected virtual void Fail(ScenePresence sp, GridRegion finalDestination, bool logout)
@@ -1089,6 +1125,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                     Vector3 vel2 = new Vector3(agent.Velocity.X, agent.Velocity.Y, 0);
 
                     agent.RemoveFromPhysicalScene();
+
                     SetInTransit(agent.UUID);
 
                     AgentData cAgent = new AgentData(); 
@@ -1100,6 +1137,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                     // We don't need the callback anymnore
                     cAgent.CallbackURI = String.Empty;
 
+                    // Beyond this point, extra cleanup is needed beyond removing transit state
+                    UpdateInTransit(agent.UUID, AgentTransferState.Transferring);
+
                     if (!m_scene.SimulationService.UpdateAgent(neighbourRegion, cAgent))
                     {
                         // region doesn't take it
@@ -1142,7 +1182,15 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                     }
 
                     // SUCCESS!
+                    UpdateInTransit(agent.UUID, AgentTransferState.ReceivedAtDestination);
+
+                    // Unlike a teleport, here we do not wait for the destination region to confirm the receipt.
+                    UpdateInTransit(agent.UUID, AgentTransferState.CleaningUp);
+
                     agent.MakeChildAgent();
+
+                    // FIXME: Possibly this should occur lower down after other commands to close other agents,
+                    // but not sure yet what the side effects would be.
                     ResetFromTransit(agent.UUID);
 
                     // now we have a child agent in this region. Request all interesting data about other (root) agents
@@ -1622,7 +1670,37 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
         #region Agent Arrived
         public void AgentArrivedAtDestination(UUID id)
         {
-            ResetFromTransit(id);
+            lock (m_agentsInTransit)
+            {
+                if (!m_agentsInTransit.ContainsKey(id))
+                {
+                    m_log.WarnFormat(
+                        "[ENTITY TRANSFER MODULE]: Region {0} received notification of arrival in destination scene of agent {1} but no teleport request is active",
+                        m_scene.RegionInfo.RegionName, id);
+
+                    return;
+                }
+
+                AgentTransferState currentState = m_agentsInTransit[id];
+
+                if (currentState == AgentTransferState.ReceivedAtDestination)
+                {
+                    // An anomoly but don't make this an outright failure - destination region could be overzealous in sending notification.
+                    m_log.WarnFormat(
+                        "[ENTITY TRANSFER MODULE]: Region {0} received notification of arrival in destination scene of agent {1} but notification has already previously been received",
+                        m_scene.RegionInfo.RegionName, id);
+                }
+                else if (currentState != AgentTransferState.Transferring)
+                {
+                    m_log.ErrorFormat(
+                        "[ENTITY TRANSFER MODULE]: Region {0} received notification of arrival in destination scene of agent {1} but agent is in transfer state {2}",
+                        m_scene.RegionInfo.RegionName, id, currentState);
+
+                    return;
+                }
+
+                m_agentsInTransit[id] = AgentTransferState.ReceivedAtDestination;
+            }
         }
 
         #endregion
@@ -1964,10 +2042,29 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 
         #region Misc
 
-        protected bool WaitForCallback(UUID id)
+        private bool WaitForCallback(UUID id)
         {
+            lock (m_agentsInTransit)
+            {
+                if (!IsInTransit(id))
+                    throw new Exception(
+                        string.Format(
+                            "Asked to wait for destination callback for agent with ID {0} but it is not in transit"));
+
+                AgentTransferState currentState = m_agentsInTransit[id];
+
+                if (currentState != AgentTransferState.Transferring && currentState != AgentTransferState.ReceivedAtDestination)
+                    throw new Exception(
+                        string.Format(
+                            "Asked to wait for destination callback for agent with ID {0} but it is in state {1}",
+                            currentState));
+            }
+
             int count = 200;
-            while (m_agentsInTransit.Contains(id) && count-- > 0)
+
+            // There should be no race condition here since no other code should be removing the agent transfer or
+            // changing the state to another other than Transferring => ReceivedAtDestination.
+            while (m_agentsInTransit[id] != AgentTransferState.ReceivedAtDestination && count-- > 0)
             {
 //                m_log.Debug("  >>> Waiting... " + count);
                 Thread.Sleep(100);
@@ -1977,17 +2074,17 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
         }
 
         /// <summary>
-        /// Set that an agent is in the process of being teleported.
+        /// Set that an agent is in transit.
         /// </summary>
         /// <param name='id'>The ID of the agent being teleported</param>
         /// <returns>true if the agent was not already in transit, false if it was</returns>
-        protected bool SetInTransit(UUID id)
+        private bool SetInTransit(UUID id)
         {
             lock (m_agentsInTransit)
             {
-                if (!m_agentsInTransit.Contains(id))
+                if (!m_agentsInTransit.ContainsKey(id))
                 {
-                    m_agentsInTransit.Add(id);
+                    m_agentsInTransit[id] = AgentTransferState.Preparing;
                     return true;
                 }
             }
@@ -1996,29 +2093,73 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
         }
 
         /// <summary>
-        /// Show whether the given agent is being teleported.
+        /// Updates the state of an agent that is already in transit.
-        /// </summary>
-        /// <returns>true if the agent is in the process of being teleported, false otherwise.</returns>
-        /// <param name='id'>The agent ID</para></param>
-        public bool IsInTransit(UUID id)
-        {
-            lock (m_agentsInTransit)
-                return m_agentsInTransit.Contains(id);
-        }
-
-        /// <summary>
-        /// Set that an agent is no longer being teleported.
         /// </summary>
+        /// <param name='id'></param>
+        /// <param name='newState'></param>
         /// <returns></returns>
-        /// <param name='id'>
+        /// <exception cref='Exception'>Illegal transitions will throw an Exception</exception>
-        /// true if the agent was flagged as being teleported when this method was called, false otherwise
+        private void UpdateInTransit(UUID id, AgentTransferState newState)
-        /// </param>
-        protected bool ResetFromTransit(UUID id)
         {
             lock (m_agentsInTransit)
             {
-                if (m_agentsInTransit.Contains(id))
+                // Illegal to try and update an agent that's not actually in transit.
+                if (!m_agentsInTransit.ContainsKey(id))
+                    throw new Exception(string.Format("Agent with ID {0} is not registered as in transit", id));
+
+                AgentTransferState oldState = m_agentsInTransit[id];
+
+                bool transitionOkay = false;
+
+                if (newState == AgentTransferState.CleaningUp && oldState != AgentTransferState.CleaningUp)
+                    transitionOkay = true;
+                else if (newState == AgentTransferState.Transferring && oldState == AgentTransferState.Preparing)
+                    transitionOkay = true;
+                else if (newState == AgentTransferState.ReceivedAtDestination && oldState == AgentTransferState.Transferring)
+                    transitionOkay = true;
+
+                if (transitionOkay)
+                    m_agentsInTransit[id] = newState;
+                else
+                    throw new Exception(
+                        string.Format(
+                            "Agent with ID {0} is not allowed to move from old transit state {1} to new state {2}",
+                            id, oldState, newState));
+            }
+        }
+
+        public bool IsInTransit(UUID id)
+        {
+            lock (m_agentsInTransit)
+                return m_agentsInTransit.ContainsKey(id);
+        }
+
+        /// <summary>
+        /// Removes an agent from the transit state machine.
+        /// </summary>
+        /// <param name='id'></param>
+        /// <returns>true if the agent was flagged as being teleported when this method was called, false otherwise</returns>
+        private bool ResetFromTransit(UUID id)
+        {
+            lock (m_agentsInTransit)
+            {
+                if (m_agentsInTransit.ContainsKey(id))
                 {
+                    AgentTransferState state = m_agentsInTransit[id];
+
+                    if (state == AgentTransferState.Transferring || state == AgentTransferState.ReceivedAtDestination)
+                    {
+                        // FIXME: For now, we allow exit from any state since a thrown exception in teleport is now guranteed
+                        // to be handled properly - ResetFromTransit() could be invoked at any step along the process
+                        m_log.WarnFormat(
+                            "[ENTITY TRANSFER MODULE]: Agent with ID should not exit directly from state {1}, should go to {2} state first",
+                            state, AgentTransferState.CleaningUp);
+
+//                        throw new Exception(
+//                            "Agent with ID {0} cannot exit directly from state {1}, it must go to {2} state first",
+//                            state, AgentTransferState.CleaningUp);
+                    }
+
                     m_agentsInTransit.Remove(id);
 
                     m_log.DebugFormat(

OpenSim/Region/CoreModules/ServiceConnectorsOut/Simulation/RemoteSimulationConnector.cs

@@ -226,13 +226,13 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Simulation
                 return m_remoteConnector.RetrieveAgent(destination, id, out agent);
 
             return false;
-
         }
 
         public bool QueryAccess(GridRegion destination, UUID id, Vector3 position, out string version, out string reason)
         {
             reason = "Communications failure";
             version = "Unknown";
+
             if (destination == null)
                 return false;
 
@@ -245,7 +245,6 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Simulation
                 return m_remoteConnector.QueryAccess(destination, id, position, out version, out reason);
 
             return false;
-
         }
 
         public bool ReleaseAgent(UUID origin, UUID id, string uri)

OpenSim/Region/Framework/Interfaces/IEntityTransferModule.cs

@@ -77,8 +77,8 @@ namespace OpenSim.Region.Framework.Interfaces
         /// <summary>
         /// Show whether the given agent is being teleported.
         /// </summary>
-        /// <returns>true if the agent is in the process of being teleported, false otherwise.</returns>
         /// <param name='id'>The agent ID</para></param>
+        /// <returns>true if the agent is in the process of being teleported, false otherwise.</returns>
         bool IsInTransit(UUID id);        
 
         bool Cross(ScenePresence agent, bool isFlying);

OpenSim/Region/Framework/Scenes/Scene.cs

@@ -5212,10 +5212,10 @@ namespace OpenSim.Region.Framework.Scenes
         {
             if (EntityTransferModule.IsInTransit(agentID))
             {
-                reason = "Agent is already in transit on this region";
+                reason = "Agent is still in transit from this region";
 
-                m_log.DebugFormat(
+                m_log.WarnFormat(
-                    "[SCENE]: Denying agent {0} entry into {1} since region already has them registered as in transit",
+                    "[SCENE]: Denying agent {0} entry into {1} since region still has them registered as in transit",
                     agentID, RegionInfo.RegionName);
 
                 return false;

OpenSim/Server/Handlers/Simulation/AgentHandlers.cs

@@ -144,13 +144,16 @@ namespace OpenSim.Server.Handlers.Simulation
 
             responsedata["int_response_code"] = HttpStatusCode.OK;
 
-            OSDMap resp = new OSDMap(2);
+            OSDMap resp = new OSDMap(3);
 
             resp["success"] = OSD.FromBoolean(result);
             resp["reason"] = OSD.FromString(reason);
             resp["version"] = OSD.FromString(version);
 
-            responsedata["str_response_string"] = OSDParser.SerializeJsonString(resp);
+            // We must preserve defaults here, otherwise a false "success" will not be put into the JSON map!
+            responsedata["str_response_string"] = OSDParser.SerializeJsonString(resp, true);
+
+//            Console.WriteLine("str_response_string [{0}]", responsedata["str_response_string"]);
         }
 
         protected virtual void DoAgentGet(Hashtable request, Hashtable responsedata, UUID id, UUID regionID)

OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs

@@ -320,29 +320,40 @@ namespace OpenSim.Services.Connectors.Simulation
                 {
                     OSDMap data = (OSDMap)result["_Result"];
 
+                    // FIXME: If there is a _Result map then it's the success key here that indicates the true success
+                    // or failure, not the sibling result node.
+                    success = data["success"];
+
                     reason = data["reason"].AsString();
                     if (data["version"] != null && data["version"].AsString() != string.Empty)
                         version = data["version"].AsString();
 
-                    m_log.DebugFormat("[REMOTE SIMULATION CONNECTOR]: QueryAccess to {0} returned {1} version {2} ({3})", uri, success, version, data["version"].AsString());
+                    m_log.DebugFormat(
+                        "[REMOTE SIMULATION CONNECTOR]: QueryAccess to {0} returned {1}, reason {2}, version {3} ({4})",
+                        uri, success, reason, version, data["version"].AsString());
                 }
 
                 if (!success)
                 {
-                    if (result.ContainsKey("Message"))
+                    // If we don't check this then OpenSimulator 0.7.3.1 and some period before will never see the
+                    // actual failure message
+                    if (!result.ContainsKey("_Result"))
                     {
-                        string message = result["Message"].AsString();
+                        if (result.ContainsKey("Message"))
-                        if (message == "Service request failed: [MethodNotAllowed] MethodNotAllowed") // Old style region
                         {
-                            m_log.Info("[REMOTE SIMULATION CONNECTOR]: The above web util error was caused by a TP to a sim that doesn't support QUERYACCESS and can be ignored");
+                            string message = result["Message"].AsString();
-                            return true;
+                            if (message == "Service request failed: [MethodNotAllowed] MethodNotAllowed") // Old style region
-                        }
+                            {
+                                m_log.Info("[REMOTE SIMULATION CONNECTOR]: The above web util error was caused by a TP to a sim that doesn't support QUERYACCESS and can be ignored");
+                                return true;
+                            }
     
-                        reason = result["Message"];
+                            reason = result["Message"];
-                    }
+                        }
-                    else
+                        else
-                    {
+                        {
-                        reason = "Communications failure";
+                            reason = "Communications failure";
+                        }
                     }
 
                     return false;
@@ -352,7 +363,7 @@ namespace OpenSim.Services.Connectors.Simulation
             }
             catch (Exception e)
             {
-                m_log.WarnFormat("[REMOTE SIMULATION CONNECTOR] QueryAcess failed with exception; {0}",e.ToString());
+                m_log.WarnFormat("[REMOTE SIMULATION CONNECTOR] QueryAcesss failed with exception; {0}",e.ToString());
             }
             
             return false;