From 7cceab12956dcb8ebeff129375888541831f7976 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Fri, 25 May 2012 01:41:00 +0100 Subject: [PATCH 1/3] In remote QueryAccess, also receive the actual status (true|false) instead of always true no matter what the callee actually returned. This was due to two things 1) SimulationServiceConnector.QueryAccess was always looking to the outer result["success"]. But if a "_Result" map is returned (which is certainly the case right now), then the true success is _Result["success"], result["success"] is always true no matter what 2) If QueryAccess was false at the destination, then AgentHandlers.DoQueryAccess() was never putting this in the result. The default action of SerializeJsonString() is not to put false booleans in the JSON!!!, so this has to be explicitly set. --- OpenSim/Server/Handlers/Simulation/AgentHandlers.cs | 7 +++++-- .../Connectors/Simulation/SimulationServiceConnector.cs | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/OpenSim/Server/Handlers/Simulation/AgentHandlers.cs b/OpenSim/Server/Handlers/Simulation/AgentHandlers.cs index 99ae7f0405..012b14e575 100644 --- a/OpenSim/Server/Handlers/Simulation/AgentHandlers.cs +++ b/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) diff --git a/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs b/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs index cc46ba88c1..032beb532c 100644 --- a/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs +++ b/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs @@ -320,6 +320,10 @@ 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(); From 40c78b06246d1131e07982dc6a9366666d9ea031 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Fri, 25 May 2012 02:02:53 +0100 Subject: [PATCH 2/3] 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 | 201 +++++++++++++++--- .../Interfaces/IEntityTransferModule.cs | 2 +- OpenSim/Region/Framework/Scenes/Scene.cs | 6 +- 3 files changed, 175 insertions(+), 34 deletions(-) diff --git a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs index b5717cdccd..ddb621d81d 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. - /// - /// true if the agent is in the process of being teleported, false otherwise. - /// The agent ID - public bool IsInTransit(UUID id) - { - lock (m_agentsInTransit) - return m_agentsInTransit.Contains(id); - } - - /// - /// Set that an agent is no longer being teleported. + /// Updates the state of an agent that is already in transit. /// + /// + /// /// - /// - /// true if the agent was flagged as being teleported when this method was called, false otherwise - /// - protected bool ResetFromTransit(UUID id) + /// Illegal transitions will throw an Exception + private void UpdateInTransit(UUID id, AgentTransferState newState) { 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); + } + + /// + /// Removes an agent from the transit state machine. + /// + /// + /// 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.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( diff --git a/OpenSim/Region/Framework/Interfaces/IEntityTransferModule.cs b/OpenSim/Region/Framework/Interfaces/IEntityTransferModule.cs index 75c44d5823..69be83eb41 100644 --- a/OpenSim/Region/Framework/Interfaces/IEntityTransferModule.cs +++ b/OpenSim/Region/Framework/Interfaces/IEntityTransferModule.cs @@ -77,8 +77,8 @@ namespace OpenSim.Region.Framework.Interfaces /// /// Show whether the given agent is being teleported. /// - /// true if the agent is in the process of being teleported, false otherwise. /// The agent ID + /// true if the agent is in the process of being teleported, false otherwise. bool IsInTransit(UUID id); bool Cross(ScenePresence agent, bool isFlying); diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 755b1e6c1c..98a75e49a9 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/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( - "[SCENE]: Denying agent {0} entry into {1} since region already has them registered as in transit", + m_log.WarnFormat( + "[SCENE]: Denying agent {0} entry into {1} since region still has them registered as in transit", agentID, RegionInfo.RegionName); return false; From 96cde407ab0d40856fb10b3b9f304433ffe734a2 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Fri, 25 May 2012 02:37:22 +0100 Subject: [PATCH 3/3] Fix bug where a failed QueryAccess to a remove region would always have the reason "Communications failure" no matter what the destination region actually returned --- .../Simulation/RemoteSimulationConnector.cs | 3 +- .../Simulation/SimulationServiceConnector.cs | 33 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Simulation/RemoteSimulationConnector.cs b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Simulation/RemoteSimulationConnector.cs index 3d2851806f..f980f688cc 100644 --- a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Simulation/RemoteSimulationConnector.cs +++ b/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) diff --git a/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs b/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs index 032beb532c..95c4f87b77 100644 --- a/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs +++ b/OpenSim/Services/Connectors/Simulation/SimulationServiceConnector.cs @@ -328,25 +328,32 @@ namespace OpenSim.Services.Connectors.Simulation 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 (message == "Service request failed: [MethodNotAllowed] MethodNotAllowed") // Old style region + if (result.ContainsKey("Message")) { - 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; + string message = result["Message"].AsString(); + 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"]; + } + else + { + reason = "Communications failure"; } - - reason = result["Message"]; - } - else - { - reason = "Communications failure"; } return false; @@ -356,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;