From 73c47f720583da40496d5c8fe94f35ed0aec640f Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 9 Mar 2012 02:22:22 +0000 Subject: Remove a race condition from SP.Set_AbsolutePosition where we assume the ParentPart is still not null if the ParentID != 0 Another thread could come in and stand the avatar between those two instructions. --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes') diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 9d5cdfa..be56fe1 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -455,12 +455,12 @@ namespace OpenSim.Region.Framework.Scenes // in the sim unless the avatar is on a sit target. While // on a sit target, m_pos will contain the desired offset // without the parent rotation applied. - if (ParentID != 0) - { - SceneObjectPart part = ParentPart; - return part.AbsolutePosition + (m_pos * part.GetWorldRotation()); - } + SceneObjectPart sitPart = ParentPart; + + if (sitPart != null) + return sitPart.AbsolutePosition + (m_pos * sitPart.GetWorldRotation()); } + return m_pos; } set -- cgit v1.1 From b454326273a03420addf4d73d308f0ca773558ad Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 9 Mar 2012 02:33:48 +0000 Subject: refactor: cleanup SP.HandleAgentSit so that everything is done within one if (part != null), rather than having unnecessary multiple checks --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes') diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index be56fe1..c9dc7fd 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -2204,23 +2204,16 @@ namespace OpenSim.Region.Framework.Scenes // "[SCENE PRESENCE]: Sitting {0} at position {1} ({2} + {3}) on part {4} {5} without sit target", // Name, part.AbsolutePosition, m_pos, ParentPosition, part.Name, part.LocalId); } - } - else - { - return; - } - - ParentPart = m_scene.GetSceneObjectPart(m_requestedSitTargetID); - if (ParentPart == null) - return; - ParentID = m_requestedSitTargetID; + ParentPart = m_scene.GetSceneObjectPart(m_requestedSitTargetID); + ParentID = m_requestedSitTargetID; - Velocity = Vector3.Zero; - RemoveFromPhysicalScene(); - - Animator.TrySetMovementAnimation(sitAnimation); - SendAvatarDataToAllAgents(); + Velocity = Vector3.Zero; + RemoveFromPhysicalScene(); + + Animator.TrySetMovementAnimation(sitAnimation); + SendAvatarDataToAllAgents(); + } } public void HandleAgentSitOnGround() -- cgit v1.1 From 94e58ff6b9368975925cea4697077a8e59162bc0 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 9 Mar 2012 02:38:11 +0000 Subject: Use SP.ParentPart instead of ParentID in places where it's more efficient (saving extra null checks, etc.) However, it looks like we should retain SP.ParentID since it's much easier to use that in places where another thread could change ParentPart to null. Otherwise one has to clumsily put ParentPart in a reference, etc. to avoid a race. --- OpenSim/Region/Framework/Scenes/SceneGraph.cs | 24 +++++----------------- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 26 ++++++++++++------------ 2 files changed, 18 insertions(+), 32 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes') diff --git a/OpenSim/Region/Framework/Scenes/SceneGraph.cs b/OpenSim/Region/Framework/Scenes/SceneGraph.cs index 66fb493..dd0ca43 100644 --- a/OpenSim/Region/Framework/Scenes/SceneGraph.cs +++ b/OpenSim/Region/Framework/Scenes/SceneGraph.cs @@ -215,27 +215,13 @@ namespace OpenSim.Region.Framework.Scenes if (sp.IsChildAgent) continue; - if (sp.ParentID != 0) - { - // sitting avatar - SceneObjectPart sop = m_parentScene.GetSceneObjectPart(sp.ParentID); - if (sop != null) - { - coarseLocations.Add(sop.AbsolutePosition + sp.OffsetPosition); - avatarUUIDs.Add(sp.UUID); - } - else - { - // we can't find the parent.. ! arg! - coarseLocations.Add(sp.AbsolutePosition); - avatarUUIDs.Add(sp.UUID); - } - } + SceneObjectPart sitPart = sp.ParentPart; + if (sitPart != null) + coarseLocations.Add(sitPart.AbsolutePosition + sp.OffsetPosition); else - { coarseLocations.Add(sp.AbsolutePosition); - avatarUUIDs.Add(sp.UUID); - } + + avatarUUIDs.Add(sp.UUID); } } diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index c9dc7fd..aab0bf0 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -432,7 +432,7 @@ namespace OpenSim.Region.Framework.Scenes { get { - if (PhysicsActor != null && m_parentID == 0) + if (PhysicsActor != null && ParentID == 0) { m_pos = PhysicsActor.Position; @@ -504,6 +504,7 @@ namespace OpenSim.Region.Framework.Scenes // There is no offset position when not seated if (ParentID == 0) return; + m_pos = value; } } @@ -562,19 +563,18 @@ namespace OpenSim.Region.Framework.Scenes public bool IsChildAgent { get; set; } - public uint ParentID - { - get { return m_parentID; } - set { m_parentID = value; } - } - private uint m_parentID; + /// + /// If the avatar is sitting, the local ID of the prim that it's sitting on. If not sitting then zero. + /// + public uint ParentID { get; set; } - public SceneObjectPart ParentPart - { - get { return m_parentPart; } - set { m_parentPart = value; } - } - private SceneObjectPart m_parentPart = null; + /// + /// If the avatar is sitting, the prim that it's sitting on. If not sitting then null. + /// + /// + /// If you use this property then you must take a reference since another thread could set it to null. + /// + public SceneObjectPart ParentPart { get; set; } public float Health { -- cgit v1.1 From 205c36d3a4bb6fe0aca5027e8e7e36fc57c6de1c Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 9 Mar 2012 02:44:08 +0000 Subject: Get rid of unnecessary ParentID == 0 check on SP.Get_AbsolutePosition since this is handled by the necessary ParentPart check --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes') diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index aab0bf0..b84660a 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -432,7 +432,7 @@ namespace OpenSim.Region.Framework.Scenes { get { - if (PhysicsActor != null && ParentID == 0) + if (PhysicsActor != null) { m_pos = PhysicsActor.Position; @@ -477,7 +477,7 @@ namespace OpenSim.Region.Framework.Scenes } } - // Don't update while sitting + // Don't update while sitting. The PhysicsActor above is null whilst sitting. if (ParentID == 0) { m_pos = value; -- cgit v1.1 From 06dda14505743bde237362b0e469d16548922f33 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 9 Mar 2012 02:50:57 +0000 Subject: Simplify minimap coarse location code by just reference SP.AbsolutePosition This is rather than checking whether the avatar is sitting and doing its own calculation. --- OpenSim/Region/Framework/Scenes/SceneGraph.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes') diff --git a/OpenSim/Region/Framework/Scenes/SceneGraph.cs b/OpenSim/Region/Framework/Scenes/SceneGraph.cs index dd0ca43..bc3400a 100644 --- a/OpenSim/Region/Framework/Scenes/SceneGraph.cs +++ b/OpenSim/Region/Framework/Scenes/SceneGraph.cs @@ -215,11 +215,7 @@ namespace OpenSim.Region.Framework.Scenes if (sp.IsChildAgent) continue; - SceneObjectPart sitPart = sp.ParentPart; - if (sitPart != null) - coarseLocations.Add(sitPart.AbsolutePosition + sp.OffsetPosition); - else - coarseLocations.Add(sp.AbsolutePosition); + coarseLocations.Add(sp.AbsolutePosition); avatarUUIDs.Add(sp.UUID); } -- cgit v1.1