From 3ffd90496a366f2b64eb8daadf63a2b6ee05ad7a Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 16 Jan 2014 20:23:31 +0000 Subject: Prevent duplicate invocations or race dontision in SP.CompleteMovement() This can happen under poor network conditions if a viewer repeats the message send If this happens, physics actors can get orphaned, which unecessarily raises physics frame times --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 29 +++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes/ScenePresence.cs') diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 49f70c4..63cca56 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -108,6 +108,16 @@ namespace OpenSim.Region.Framework.Scenes } } + /// + /// This exists to prevent race conditions between two CompleteMovement threads if the simulator is slow and + /// the viewer fires these in quick succession. + /// + /// + /// TODO: The child -> agent transition should be folded into LifecycleState and the CompleteMovement + /// regulation done there. + /// + private object m_completeMovementLock = new object(); + // private static readonly byte[] DEFAULT_TEXTURE = AvatarAppearance.GetDefaultTexture().GetBytes(); private static readonly Array DIR_CONTROL_FLAGS = Enum.GetValues(typeof(Dir_ControlFlags)); private static readonly Vector3 HEAD_ADJUSTMENT = new Vector3(0f, 0f, 0.3f); @@ -905,6 +915,7 @@ namespace OpenSim.Region.Framework.Scenes /// /// Turns a child agent into a root agent. /// + /// /// Child agents are logged into neighbouring sims largely to observe changes. Root agents exist when the /// avatar is actual in the sim. They can perform all actions. /// This change is made whenever an avatar enters a region, whether by crossing over from a neighbouring sim, @@ -912,8 +923,8 @@ namespace OpenSim.Region.Framework.Scenes /// /// This method is on the critical path for transferring an avatar from one region to another. Delay here /// delays that crossing. - /// - private void MakeRootAgent(Vector3 pos, bool isFlying) + /// + private bool MakeRootAgent(Vector3 pos, bool isFlying) { // m_log.InfoFormat( // "[SCENE]: Upgrading child to root agent for {0} in {1}", @@ -921,6 +932,10 @@ namespace OpenSim.Region.Framework.Scenes //m_log.DebugFormat("[SCENE]: known regions in {0}: {1}", Scene.RegionInfo.RegionName, KnownChildRegionHandles.Count); + lock (m_completeMovementLock) + if (!IsChildAgent) + return false; + IsChildAgent = false; // Must reset this here so that a teleport to a region next to an existing region does not keep the flag @@ -1070,6 +1085,7 @@ namespace OpenSim.Region.Framework.Scenes m_scene.EventManager.TriggerOnMakeRootAgent(this); + return true; } public int GetStateSource() @@ -1443,7 +1459,14 @@ namespace OpenSim.Region.Framework.Scenes } bool flying = ((m_AgentControlFlags & AgentManager.ControlFlags.AGENT_CONTROL_FLY) != 0); - MakeRootAgent(AbsolutePosition, flying); + if (!MakeRootAgent(AbsolutePosition, flying)) + { + m_log.DebugFormat( + "[SCENE PRESENCE]: Aborting CompleteMovement call for {0} in {1} as they are already root", + Name, Scene.Name); + + return; + } // Tell the client that we're totally ready ControllingClient.MoveAgentIntoRegion(m_scene.RegionInfo, AbsolutePosition, look); -- cgit v1.1 From 3bc669ffc7638b56d5ab5aac038c33106ba9a95b Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 16 Jan 2014 23:31:50 +0000 Subject: Actually put IsChildAgent = true inside the lock, otherwise there is still a small window for race conditions on duplicate CompleteMovement calls --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'OpenSim/Region/Framework/Scenes/ScenePresence.cs') diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 63cca56..3290da1 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -933,10 +933,12 @@ namespace OpenSim.Region.Framework.Scenes //m_log.DebugFormat("[SCENE]: known regions in {0}: {1}", Scene.RegionInfo.RegionName, KnownChildRegionHandles.Count); lock (m_completeMovementLock) + { if (!IsChildAgent) return false; - IsChildAgent = false; + IsChildAgent = false; + } // Must reset this here so that a teleport to a region next to an existing region does not keep the flag // set and prevent the close of the connection on a subsequent re-teleport. -- cgit v1.1 From 4fa843ff19441c9daa4e7dae0a4d705f912fca54 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 16 Jan 2014 23:44:17 +0000 Subject: Reorder checks in SP.CompleteMovement() to fix test failures --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 69 ++++++++++++------------ 1 file changed, 33 insertions(+), 36 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes/ScenePresence.cs') diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 18d84a2..85a20e9 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -1003,48 +1003,45 @@ namespace OpenSim.Region.Framework.Scenes /// private bool MakeRootAgent(Vector3 pos, bool isFlying) { -// m_log.InfoFormat( -// "[SCENE]: Upgrading child to root agent for {0} in {1}", -// Name, m_scene.RegionInfo.RegionName); - - if (ParentUUID != UUID.Zero) + lock (m_completeMovementLock) { - m_log.DebugFormat("[SCENE PRESENCE]: Sitting avatar back on prim {0}", ParentUUID); - SceneObjectPart part = m_scene.GetSceneObjectPart(ParentUUID); - if (part == null) + if (!IsChildAgent) + return false; + + //m_log.DebugFormat("[SCENE]: known regions in {0}: {1}", Scene.RegionInfo.RegionName, KnownChildRegionHandles.Count); + + // m_log.InfoFormat( + // "[SCENE]: Upgrading child to root agent for {0} in {1}", + // Name, m_scene.RegionInfo.RegionName); + + if (ParentUUID != UUID.Zero) { - m_log.ErrorFormat("[SCENE PRESENCE]: Can't find prim {0} to sit on", ParentUUID); + m_log.DebugFormat("[SCENE PRESENCE]: Sitting avatar back on prim {0}", ParentUUID); + SceneObjectPart part = m_scene.GetSceneObjectPart(ParentUUID); + if (part == null) + { + m_log.ErrorFormat("[SCENE PRESENCE]: Can't find prim {0} to sit on", ParentUUID); + } + else + { + part.ParentGroup.AddAvatar(UUID); + if (part.SitTargetPosition != Vector3.Zero) + part.SitTargetAvatar = UUID; + // ParentPosition = part.GetWorldPosition(); + ParentID = part.LocalId; + ParentPart = part; + m_pos = PrevSitOffset; + // pos = ParentPosition; + pos = part.GetWorldPosition(); + } + ParentUUID = UUID.Zero; + + // Animator.TrySetMovementAnimation("SIT"); } else { - part.ParentGroup.AddAvatar(UUID); - if (part.SitTargetPosition != Vector3.Zero) - part.SitTargetAvatar = UUID; -// ParentPosition = part.GetWorldPosition(); - ParentID = part.LocalId; - ParentPart = part; - m_pos = PrevSitOffset; -// pos = ParentPosition; - pos = part.GetWorldPosition(); + IsLoggingIn = false; } - ParentUUID = UUID.Zero; - - IsChildAgent = false; - -// Animator.TrySetMovementAnimation("SIT"); - } - else - { - IsChildAgent = false; - IsLoggingIn = false; - } - - //m_log.DebugFormat("[SCENE]: known regions in {0}: {1}", Scene.RegionInfo.RegionName, KnownChildRegionHandles.Count); - - lock (m_completeMovementLock) - { - if (!IsChildAgent) - return false; IsChildAgent = false; } -- cgit v1.1