From 40c78b06246d1131e07982dc6a9366666d9ea031 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 25 May 2012 02:02:53 +0100 Subject: Stop it being possible for an agent to teleport back to its source region before the source region has finished cleaning up old agent data and structures. If this is allowed, then the client usually gets forcibly logged out and data structures might be put into bad states. To prevent this, the binary state machine of EMT.m_agentsInTransit is replaced with a 4 state machine (Preparing, Transferring, ReceivedAtDestination, CleaningUp). This is necessary because the source region needs to know when the destination region has received the user but a teleport back cannot happen until the source region has cleaned up. Tested on standalone, grid and with v1 and v3 clients. --- .../EntityTransfer/EntityTransferModule.cs | 189 ++++++++++++++++++--- 1 file changed, 165 insertions(+), 24 deletions(-) (limited to 'OpenSim/Region/CoreModules/Framework') diff --git a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs index b5717cd..ddb621d 100644 --- a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs +++ b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs @@ -46,6 +46,29 @@ using Nini.Config; namespace OpenSim.Region.CoreModules.Framework.EntityTransfer { + /// + /// The possible states that an agent can be in when its being transferred between regions. + /// + /// + /// 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. + /// + 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 m_agentsInTransit; + private Dictionary m_agentsInTransit; private ExpiringCache> m_bannedRegions = new ExpiringCache>(); @@ -120,7 +143,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer MaxTransferDistance = DefaultMaxTransferDistance; } - m_agentsInTransit = new List(); + m_agentsInTransit = new Dictionary(); 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); } @@ -454,7 +482,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer return; } - m_log.DebugFormat("[ENTITY TRANSFER MODULE]: Destination is running version {0}", version); + m_log.DebugFormat("[ENTITY TRANSFER MODULE]: Destination is running version {0}", version); // Fixing a bug where teleporting while sitting results in the avatar ending up removed from // both regions @@ -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 @@ -1141,8 +1181,16 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer capsPath); } - // SUCCESS! + // 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 } /// - /// Set that an agent is in the process of being teleported. + /// Set that an agent is in transit. /// /// The ID of the agent being teleported /// true if the agent was not already in transit, false if it was - 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 } /// - /// Show whether the given agent is being teleported. + /// Updates the state of an agent that is already in transit. /// - /// true if the agent is in the process of being teleported, false otherwise. - /// The agent ID + /// + /// + /// + /// Illegal transitions will throw an Exception + private void UpdateInTransit(UUID id, AgentTransferState newState) + { + lock (m_agentsInTransit) + { + // 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.Contains(id); + return m_agentsInTransit.ContainsKey(id); } /// - /// Set that an agent is no longer being teleported. + /// Removes an agent from the transit state machine. /// - /// - /// - /// true if the agent was flagged as being teleported when this method was called, false otherwise - /// - protected bool ResetFromTransit(UUID id) + /// + /// true if the agent was flagged as being teleported when this method was called, false otherwise + private bool ResetFromTransit(UUID id) { lock (m_agentsInTransit) { - if (m_agentsInTransit.Contains(id)) + 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( -- cgit v1.1