From 857f24a5e2b59072ad4d987d5e64318f5249c7e7 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 2 Sep 2013 19:15:10 +0100 Subject: [PATCH] Fix bug where users teleporting to non-neighbour regions could continue to hear chat from their source region for some time after teleport completion. This occurs on v2 teleport since the source region now waits 15 secs before closing the old child agent, which could still receive chat. This commit introduces a ScenePresenceState.PreClose which is set before the wait, so that ChatModule can check for ScenePresenceState.Running. This was theoretically also an issue on v1 teleport but since the pause before close was only 2 secs there, it was not noticed. --- .../Caps/EventQueue/Tests/EventQueueTests.cs | 4 +- .../CoreModules/Avatar/Chat/ChatModule.cs | 15 +++-- .../EntityTransfer/EntityTransferModule.cs | 7 +++ OpenSim/Region/Framework/Scenes/Scene.cs | 59 +++++++++++++++++-- .../Scenes/ScenePresenceStateMachine.cs | 15 ++++- 5 files changed, 85 insertions(+), 15 deletions(-) diff --git a/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/Tests/EventQueueTests.cs b/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/Tests/EventQueueTests.cs index 626932f53d..b3b0b8acc0 100644 --- a/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/Tests/EventQueueTests.cs +++ b/OpenSim/Region/ClientStack/Linden/Caps/EventQueue/Tests/EventQueueTests.cs @@ -76,7 +76,7 @@ namespace OpenSim.Region.ClientStack.Linden.Tests } [Test] - public void AddForClient() + public void TestAddForClient() { TestHelpers.InMethod(); // log4net.Config.XmlConfigurator.Configure(); @@ -88,7 +88,7 @@ namespace OpenSim.Region.ClientStack.Linden.Tests } [Test] - public void RemoveForClient() + public void TestRemoveForClient() { TestHelpers.InMethod(); // TestHelpers.EnableLogging(); diff --git a/OpenSim/Region/CoreModules/Avatar/Chat/ChatModule.cs b/OpenSim/Region/CoreModules/Avatar/Chat/ChatModule.cs index 58f747b7d7..5229c08aa0 100644 --- a/OpenSim/Region/CoreModules/Avatar/Chat/ChatModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Chat/ChatModule.cs @@ -326,15 +326,18 @@ namespace OpenSim.Region.CoreModules.Avatar.Chat UUID fromAgentID, UUID ownerID, string fromName, ChatTypeEnum type, string message, ChatSourceType src, bool ignoreDistance) { - Vector3 fromRegionPos = fromPos + regionPos; - Vector3 toRegionPos = presence.AbsolutePosition + - new Vector3(presence.Scene.RegionInfo.RegionLocX * Constants.RegionSize, - presence.Scene.RegionInfo.RegionLocY * Constants.RegionSize, 0); - - int dis = (int)Util.GetDistanceTo(toRegionPos, fromRegionPos); + if (presence.LifecycleState != ScenePresenceState.Running) + return false; if (!ignoreDistance) { + Vector3 fromRegionPos = fromPos + regionPos; + Vector3 toRegionPos = presence.AbsolutePosition + + new Vector3(presence.Scene.RegionInfo.RegionLocX * Constants.RegionSize, + presence.Scene.RegionInfo.RegionLocY * Constants.RegionSize, 0); + + int dis = (int)Util.GetDistanceTo(toRegionPos, fromRegionPos); + if (type == ChatTypeEnum.Whisper && dis > m_whisperdistance || type == ChatTypeEnum.Say && dis > m_saydistance || type == ChatTypeEnum.Shout && dis > m_shoutdistance) diff --git a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs index 89505161c5..4219254aa4 100644 --- a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs +++ b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs @@ -920,6 +920,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer if (NeedsClosing(sp.DrawDistance, oldRegionX, newRegionX, oldRegionY, newRegionY, reg)) { + if (!sp.Scene.IncomingPreCloseAgent(sp)) + return; + // 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. @@ -1082,6 +1085,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer // Finally, let's close this previously-known-as-root agent, when the jump is outside the view zone if (NeedsClosing(sp.DrawDistance, oldRegionX, newRegionX, oldRegionY, newRegionY, reg)) { + if (!sp.Scene.IncomingPreCloseAgent(sp)) + return; + // RED ALERT!!!! // PLEASE DO NOT DECREASE THIS WAIT TIME UNDER ANY CIRCUMSTANCES. // THE VIEWERS SEEM TO NEED SOME TIME AFTER RECEIVING MoveAgentIntoRegion @@ -1095,6 +1101,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer // then this will be handled in IncomingCloseAgent under lock conditions m_log.DebugFormat( "[ENTITY TRANSFER MODULE]: Closing agent {0} in {1} after teleport", sp.Name, Scene.Name); + sp.Scene.IncomingCloseAgent(sp.UUID, false); } else diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 005c9b93f4..2a21a4c280 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -3695,10 +3695,13 @@ namespace OpenSim.Region.Framework.Scenes // We need to ensure that we are not already removing the scene presence before we ask it not to be // closed. - if (sp != null && sp.IsChildAgent && sp.LifecycleState == ScenePresenceState.Running) + if (sp != null && sp.IsChildAgent + && (sp.LifecycleState == ScenePresenceState.Running + || sp.LifecycleState == ScenePresenceState.PreRemove)) { m_log.DebugFormat( - "[SCENE]: Reusing existing child scene presence for {0} in {1}", sp.Name, Name); + "[SCENE]: Reusing existing child scene presence for {0}, state {1} in {2}", + sp.Name, sp.LifecycleState, Name); // In the case where, for example, an A B C D region layout, an avatar may // teleport from A -> D, but then -> C before A has asked B to close its old child agent. When C @@ -3720,6 +3723,8 @@ namespace OpenSim.Region.Framework.Scenes // } // else if (EntityTransferModule.IsInTransit(sp.UUID)) + sp.LifecycleState = ScenePresenceState.Running; + if (EntityTransferModule.IsInTransit(sp.UUID)) { sp.DoNotCloseAfterTeleport = true; @@ -4440,6 +4445,50 @@ namespace OpenSim.Region.Framework.Scenes return false; } + /// + /// Tell a single agent to prepare to close. + /// + /// + /// This should only be called if we may close the agent but there will be some delay in so doing. Meant for + /// internal use - other callers should almost certainly called IncomingCloseAgent(). + /// + /// + /// true if pre-close state notification was successful. false if the agent + /// was not in a state where it could transition to pre-close. + public bool IncomingPreCloseAgent(ScenePresence sp) + { + lock (m_removeClientLock) + { + // We need to avoid a race condition where in, for example, an A B C D region layout, an avatar may + // teleport from A -> D, but then -> C before A has asked B to close its old child agent. We do not + // want to obey this close since C may have renewed the child agent lease on B. + if (sp.DoNotCloseAfterTeleport) + { + m_log.DebugFormat( + "[SCENE]: Not pre-closing {0} agent {1} in {2} since another simulator has re-established the child connection", + sp.IsChildAgent ? "child" : "root", sp.Name, Name); + + // Need to reset the flag so that a subsequent close after another teleport can succeed. + sp.DoNotCloseAfterTeleport = false; + + return false; + } + + if (sp.LifecycleState != ScenePresenceState.Running) + { + m_log.DebugFormat( + "[SCENE]: Called IncomingPreCloseAgent() for {0} in {1} but presence is already in state {2}", + sp.Name, Name, sp.LifecycleState); + + return false; + } + + sp.LifecycleState = ScenePresenceState.PreRemove; + + return true; + } + } + /// /// Tell a single agent to disconnect from the region. /// @@ -4459,16 +4508,16 @@ namespace OpenSim.Region.Framework.Scenes if (sp == null) { m_log.DebugFormat( - "[SCENE]: Called RemoveClient() with agent ID {0} but no such presence is in {1}", + "[SCENE]: Called IncomingCloseAgent() with agent ID {0} but no such presence is in {1}", agentID, Name); return false; } - if (sp.LifecycleState != ScenePresenceState.Running) + if (sp.LifecycleState != ScenePresenceState.Running && sp.LifecycleState != ScenePresenceState.PreRemove) { m_log.DebugFormat( - "[SCENE]: Called RemoveClient() for {0} in {1} but presence is already in state {2}", + "[SCENE]: Called IncomingCloseAgent() for {0} in {1} but presence is already in state {2}", sp.Name, Name, sp.LifecycleState); return false; diff --git a/OpenSim/Region/Framework/Scenes/ScenePresenceStateMachine.cs b/OpenSim/Region/Framework/Scenes/ScenePresenceStateMachine.cs index dc3a212b65..cae7fe575f 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresenceStateMachine.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresenceStateMachine.cs @@ -37,7 +37,8 @@ namespace OpenSim.Region.Framework.Scenes /// This is a state machine. /// /// [Entry] => Running - /// Running => Removing + /// Running => PreRemove, Removing + /// PreRemove => Running, Removing /// Removing => Removed /// /// All other methods should only see the scene presence in running state - this is the normal operational state @@ -46,6 +47,7 @@ namespace OpenSim.Region.Framework.Scenes public enum ScenePresenceState { Running, // Normal operation state. The scene presence is available. + PreRemove, // The presence is due to be removed but can still be returning to running. Removing, // The presence is in the process of being removed from the scene via Scene.RemoveClient. Removed, // The presence has been removed from the scene and is effectively dead. // There is no exit from this state. @@ -80,8 +82,17 @@ namespace OpenSim.Region.Framework.Scenes lock (this) { - if (newState == ScenePresenceState.Removing && m_state == ScenePresenceState.Running) + if (newState == m_state) + return; + else if (newState == ScenePresenceState.Running && m_state == ScenePresenceState.PreRemove) transitionOkay = true; + else if (newState == ScenePresenceState.PreRemove && m_state == ScenePresenceState.Running) + transitionOkay = true; + else if (newState == ScenePresenceState.Removing) + { + if (m_state == ScenePresenceState.Running || m_state == ScenePresenceState.PreRemove) + transitionOkay = true; + } else if (newState == ScenePresenceState.Removed && m_state == ScenePresenceState.Removing) transitionOkay = true; }