From 17c7ef06ba7f1329ff0494f4777b961d8847452e Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 14 May 2012 18:36:26 +0100 Subject: [PATCH] Set the agent in transit teleport flag at the first available opportunity (i.e. when IsInTransit() was being checked) to close down a race condition. On EntityTransferModule.DoTeleport() there was an IsInTransit() check to prevent multiple simultaneous teleport attempts. However, the SetInTransit() was only performed later on, which left a window in which multiple threads could pass the IsInTransit() check. This has been seen in the field and the results aren't pretty. This commit effectively combines the IsInTransit() and SetInTransit() checks so there is no such window. More failure cases are made to to call ResetInTransit() to adjust to this move. --- .../EntityTransfer/EntityTransferModule.cs | 54 ++++++++++++++----- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs index 3a5263bae4..6d149dc346 100644 --- a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs +++ b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs @@ -204,6 +204,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer sp.Name, sp.AbsolutePosition, sp.Scene.RegionInfo.RegionName, position, destinationRegionName, e.Message, e.StackTrace); + // Make sure that we clear the in-transit flag so that future teleport attempts don't always fail. + ResetFromTransit(sp.UUID); + sp.ControllingClient.SendTeleportFailed("Internal error"); } } @@ -378,7 +381,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer return; } - if (IsInTransit(sp.UUID)) // Avie is already on the way. Caller shouldn't do this. + if (!SetInTransit(sp.UUID)) // Avie is already on the way. Caller shouldn't do this. { m_log.DebugFormat( "[ENTITY TRANSFER MODULE]: Ignoring teleport request of {0} {1} to {2} ({3}) {4}/{5} - agent is already in transit.", @@ -426,8 +429,11 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer if (!m_aScene.SimulationService.QueryAccess(finalDestination, sp.ControllingClient.AgentId, Vector3.Zero, out version, out reason)) { sp.ControllingClient.SendTeleportFailed("Teleport failed: " + reason); + ResetFromTransit(sp.UUID); + return; } + m_log.DebugFormat("[ENTITY TRANSFER MODULE]: Destination is running version {0}", version); sp.ControllingClient.SendTeleportStart(teleportFlags); @@ -467,13 +473,16 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer bool logout = false; if (!CreateAgent(sp, reg, finalDestination, agentCircuit, teleportFlags, out reason, out logout)) { - sp.ControllingClient.SendTeleportFailed(String.Format("Teleport refused: {0}", - reason)); + sp.ControllingClient.SendTeleportFailed( + String.Format("Teleport refused: {0}", reason)); + ResetFromTransit(sp.UUID); + return; } // OK, it got this agent. Let's close some child agents sp.CloseChildAgents(newRegionX, newRegionY); + IClientIPEndpoint ipepClient; if (NeedsNewAgent(sp.DrawDistance, oldRegionX, newRegionX, oldRegionY, newRegionY)) { @@ -510,8 +519,6 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer capsPath = finalDestination.ServerURI + CapsUtil.GetCapsSeedPath(agentCircuit.CapsPath); } - SetInTransit(sp.UUID); - // Let's send a full update of the agent. This is a synchronous call. AgentData agent = new AgentData(); sp.CopyTo(agent); @@ -1950,25 +1957,43 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer return count > 0; } - protected void SetInTransit(UUID id) + /// + /// Set that an agent is in the process of being teleported. + /// + /// 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) { lock (m_agentsInTransit) { if (!m_agentsInTransit.Contains(id)) + { m_agentsInTransit.Add(id); - } - } - - protected bool IsInTransit(UUID id) - { - lock (m_agentsInTransit) - { - if (m_agentsInTransit.Contains(id)) return true; + } } + return false; } + /// + /// Show whether the given agent is being teleported. + /// + /// true if the agent is in the process of being teleported, false otherwise. + /// The agent ID + protected bool IsInTransit(UUID id) + { + lock (m_agentsInTransit) + return m_agentsInTransit.Contains(id); + } + + /// + /// Set that an agent is no longer being teleported. + /// + /// + /// + /// true if the agent was flagged as being teleported when this method was called, false otherwise + /// protected bool ResetFromTransit(UUID id) { lock (m_agentsInTransit) @@ -1979,6 +2004,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer return true; } } + return false; }