From 2eaa6d5ace738cf1848f82ce7a0b435928b6846f Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Mon, 9 Jul 2012 21:24:32 +0100 Subject: Do not allow a script to attach a prim if its being sat upon. This prevents a stack overflow where a get position on the avatar will refer to the attachment which will in turn refer back to the avatar. This required recording of all sitting avatars on a prim which is done separately from recording the sit target avatar. Recording HashSet is null if there are no sitting avatars in order to save memory. --- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 98 ++++++++++++++++++++-- 1 file changed, 91 insertions(+), 7 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes/SceneObjectPart.cs') diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index 3d81358..6518b84 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -374,7 +374,6 @@ namespace OpenSim.Region.Framework.Scenes private uint _category; private Int32 _creationDate; private uint _parentID = 0; - private UUID m_sitTargetAvatar = UUID.Zero; private uint _baseMask = (uint)PermissionMask.All; private uint _ownerMask = (uint)PermissionMask.All; private uint _groupMask = (uint)PermissionMask.None; @@ -1233,13 +1232,20 @@ namespace OpenSim.Region.Framework.Scenes } /// - /// ID of the avatar that is sat on us. If there is no such avatar then is UUID.Zero + /// ID of the avatar that is sat on us if we have a sit target. If there is no such avatar then is UUID.Zero /// - public UUID SitTargetAvatar - { - get { return m_sitTargetAvatar; } - set { m_sitTargetAvatar = value; } - } + public UUID SitTargetAvatar { get; set; } + + /// + /// IDs of all avatars start on this object part. + /// + /// + /// We need to track this so that we can stop sat upon prims from being attached. + /// + /// + /// null if there are no sitting avatars. This is to save us create a hashset for every prim in a scene. + /// + private HashSet m_sittingAvatars; public virtual UUID RegionID { @@ -4493,5 +4499,83 @@ namespace OpenSim.Region.Framework.Scenes Color color = Color; return new Color4(color.R, color.G, color.B, (byte)(0xFF - color.A)); } + + /// + /// Record an avatar sitting on this part. + /// + /// This is called for all the sitting avatars whether there is a sit target set or not. + /// + /// true if the avatar was not already recorded, false otherwise. + /// + /// + protected internal bool AddSittingAvatar(UUID avatarId) + { + HashSet sittingAvatars = m_sittingAvatars; + + if (sittingAvatars == null) + sittingAvatars = new HashSet(); + + lock (sittingAvatars) + { + m_sittingAvatars = sittingAvatars; + return m_sittingAvatars.Add(avatarId); + } + } + + /// + /// Remove an avatar recorded as sitting on this part. + /// + /// This applies to all sitting avatars whether there is a sit target set or not. + /// + /// true if the avatar was present and removed, false if it was not present. + /// + /// + protected internal bool RemoveSittingAvatar(UUID avatarId) + { + HashSet sittingAvatars = m_sittingAvatars; + + // This can occur under a race condition where another thread + if (sittingAvatars == null) + return false; + + lock (sittingAvatars) + { + if (sittingAvatars.Remove(avatarId)) + { + if (sittingAvatars.Count == 0) + m_sittingAvatars = null; + + return true; + } + } + + return false; + } + + /// + /// Get a copy of the list of sitting avatars. + /// + /// This applies to all sitting avatars whether there is a sit target set or not. + /// + public HashSet GetSittingAvatars() + { + return new HashSet(m_sittingAvatars); + } + + /// + /// Gets the number of sitting avatars. + /// + /// This applies to all sitting avatars whether there is a sit target set or not. + /// + public int GetSittingAvatarsCount() + { + HashSet sittingAvatars = m_sittingAvatars; + + if (sittingAvatars == null) + return 0; + + lock (sittingAvatars) + return sittingAvatars.Count; + } } } -- cgit v1.1 From 58869e5aa09a292dc2159c73bada2c487151dda0 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 10 Jul 2012 23:03:52 +0100 Subject: Fix recent SOP.GetSittingAvatars() to return null if there are no sitting avatars rather than throwing an exception. Extends sitting avatar regression tests to test new sitters information --- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes/SceneObjectPart.cs') diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index 6518b84..6677dae 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -4556,10 +4556,20 @@ namespace OpenSim.Region.Framework.Scenes /// Get a copy of the list of sitting avatars. /// /// This applies to all sitting avatars whether there is a sit target set or not. - /// + /// A hashset of the sitting avatars. Returns null if there are no sitting avatars. public HashSet GetSittingAvatars() { - return new HashSet(m_sittingAvatars); + HashSet sittingAvatars = m_sittingAvatars; + + if (sittingAvatars == null) + { + return null; + } + else + { + lock (sittingAvatars) + return new HashSet(sittingAvatars); + } } /// -- cgit v1.1 From e8347b70957273421ee567577e2d9a6139af0621 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 10 Jul 2012 23:19:52 +0100 Subject: Move common code to detect whether a part has a valid sit target into a SOP property rather than being repeated in SP. This also makes the detection in SP.FindNextAvailableSitTarget() and SendSitResponse() identical. Previously they varied slightly (SendSitResponse didn't check for an older type of invalid quaternion) but the practical effect is most probably zero. --- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'OpenSim/Region/Framework/Scenes/SceneObjectPart.cs') diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index 6677dae..dcec7e9 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -135,6 +135,21 @@ namespace OpenSim.Region.Framework.Scenes get { return ParentGroup.RootPart == this; } } + /// + /// Is an explicit sit target set for this part? + /// + public bool IsSitTargetSet + { + get + { + return + !(SitTargetPosition == Vector3.Zero + && (SitTargetOrientation == Quaternion.Identity // Valid Zero Rotation quaternion + || SitTargetOrientation.X == 0f && SitTargetOrientation.Y == 0f && SitTargetOrientation.Z == 1f && SitTargetOrientation.W == 0f // W-Z Mapping was invalid at one point + || SitTargetOrientation.X == 0f && SitTargetOrientation.Y == 0f && SitTargetOrientation.Z == 0f && SitTargetOrientation.W == 0f)); // Invalid Quaternion + } + } + #region Fields public bool AllowedDrop; -- cgit v1.1 From c8f0d476d2f775ba4d7afca12eeff527b46bb8e2 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 10 Jul 2012 23:34:40 +0100 Subject: refactor: Add SOP.IsSitTargetOccupied to improve readability --- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'OpenSim/Region/Framework/Scenes/SceneObjectPart.cs') diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index dcec7e9..937f43e 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -136,6 +136,11 @@ namespace OpenSim.Region.Framework.Scenes } /// + /// Is the sit target of this part occupied? + /// + public bool IsSitTargetOccupied { get { return SitTargetAvatar != UUID.Zero; } } + + /// /// Is an explicit sit target set for this part? /// public bool IsSitTargetSet @@ -736,7 +741,7 @@ namespace OpenSim.Region.Framework.Scenes } // TODO if we decide to do sitting in a more SL compatible way (multiple avatars per prim), this has to be fixed, too - if (SitTargetAvatar != UUID.Zero) + if (IsSitTargetOccupied) { ScenePresence avatar; if (ParentGroup.Scene.TryGetScenePresence(SitTargetAvatar, out avatar)) -- cgit v1.1 From 11e0ad6dc8663756154652465750c1ebaaf3124f Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 10 Jul 2012 23:39:05 +0100 Subject: Revert "refactor: Add SOP.IsSitTargetOccupied to improve readability" This reverts commit c8f0d476d2f775ba4d7afca12eeff527b46bb8e2. On reconsideration, I think this is less readable since immediately following code still sets SitTargetAvatar directly --- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'OpenSim/Region/Framework/Scenes/SceneObjectPart.cs') diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index 937f43e..dcec7e9 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -136,11 +136,6 @@ namespace OpenSim.Region.Framework.Scenes } /// - /// Is the sit target of this part occupied? - /// - public bool IsSitTargetOccupied { get { return SitTargetAvatar != UUID.Zero; } } - - /// /// Is an explicit sit target set for this part? /// public bool IsSitTargetSet @@ -741,7 +736,7 @@ namespace OpenSim.Region.Framework.Scenes } // TODO if we decide to do sitting in a more SL compatible way (multiple avatars per prim), this has to be fixed, too - if (IsSitTargetOccupied) + if (SitTargetAvatar != UUID.Zero) { ScenePresence avatar; if (ParentGroup.Scene.TryGetScenePresence(SitTargetAvatar, out avatar)) -- cgit v1.1 From cdea572d2ea0256423a4cd72f132346d88a93e92 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 10 Jul 2012 23:50:04 +0100 Subject: refactor: move management of SOP.SitTargetAvatar into SOP.AddSittingAvatar() and SOP.RemoveSittingAvatar() --- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'OpenSim/Region/Framework/Scenes/SceneObjectPart.cs') diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index dcec7e9..b3f11a7 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -4525,6 +4525,9 @@ namespace OpenSim.Region.Framework.Scenes /// protected internal bool AddSittingAvatar(UUID avatarId) { + if (IsSitTargetSet && SitTargetAvatar == UUID.Zero) + SitTargetAvatar = avatarId; + HashSet sittingAvatars = m_sittingAvatars; if (sittingAvatars == null) @@ -4547,6 +4550,9 @@ namespace OpenSim.Region.Framework.Scenes /// protected internal bool RemoveSittingAvatar(UUID avatarId) { + if (SitTargetAvatar == avatarId) + SitTargetAvatar = UUID.Zero; + HashSet sittingAvatars = m_sittingAvatars; // This can occur under a race condition where another thread -- cgit v1.1