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. --- .../Avatar/Attachments/AttachmentsModule.cs | 9 ++ .../Attachments/Tests/AttachmentsModuleTests.cs | 32 ++++++- .../Region/Framework/Scenes/SceneObjectGroup.cs | 14 ++++ OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 98 ++++++++++++++++++++-- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 18 +++- OpenSim/Tests/Common/Helpers/SceneHelpers.cs | 75 +++++++++++++---- OpenSim/Tests/Common/Helpers/UserAccountHelpers.cs | 9 ++ 7 files changed, 229 insertions(+), 26 deletions(-) diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs index 9eb0e38..eccf7a6 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs @@ -251,6 +251,15 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments // m_log.DebugFormat( // "[ATTACHMENTS MODULE]: Attaching object {0} {1} to {2} point {3} from ground (silent = {4})", // group.Name, group.LocalId, sp.Name, attachmentPt, silent); + + if (group.GetSittingAvatarsCount() != 0) + { +// m_log.WarnFormat( +// "[ATTACHMENTS MODULE]: Ignoring request to attach {0} {1} to {2} on {3} since {4} avatars are still sitting on it", +// group.Name, group.LocalId, sp.Name, attachmentPt, group.GetSittingAvatarsCount()); + + return false; + } if (sp.GetAttachments(attachmentPt).Contains(group)) { diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs index 5dcbd28..3e06900 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs @@ -118,7 +118,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests Scene scene = CreateDefaultTestScene(); UserAccount ua1 = UserAccountHelpers.CreateUserWithInventory(scene, 0x1); - ScenePresence sp = SceneHelpers.AddScenePresence(scene, ua1.PrincipalID); + ScenePresence sp = SceneHelpers.AddScenePresence(scene, ua1); string attName = "att"; @@ -154,6 +154,36 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests // TestHelpers.DisableLogging(); } + /// + /// Test that we do not attempt to attach an in-world object that someone else is sitting on. + /// + [Test] + public void TestAddSatOnAttachmentFromGround() + { + TestHelpers.InMethod(); +// TestHelpers.EnableLogging(); + + Scene scene = CreateDefaultTestScene(); + UserAccount ua1 = UserAccountHelpers.CreateUserWithInventory(scene, 0x1); + ScenePresence sp = SceneHelpers.AddScenePresence(scene, ua1); + + string attName = "att"; + + SceneObjectGroup so = SceneHelpers.AddSceneObject(scene, attName, sp.UUID); + + UserAccount ua2 = UserAccountHelpers.CreateUserWithInventory(scene, 0x2); + ScenePresence sp2 = SceneHelpers.AddScenePresence(scene, ua2); + + // Put avatar within 10m of the prim so that sit doesn't fail. + sp2.AbsolutePosition = new Vector3(0, 0, 0); + sp2.HandleAgentRequestSit(sp2.ControllingClient, sp2.UUID, so.UUID, Vector3.Zero); + + scene.AttachmentsModule.AttachObject(sp, so, (uint)AttachmentPoint.Chest, false); + + Assert.That(sp.HasAttachments(), Is.False); + Assert.That(scene.GetSceneObjectGroups().Count, Is.EqualTo(1)); + } + [Test] public void TestAddAttachmentFromInventory() { diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index 4e0e183..fc04761 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -3401,6 +3401,20 @@ namespace OpenSim.Region.Framework.Scenes return count; } + /// + /// Gets the number of sitting avatars. + /// + /// This applies to all sitting avatars whether there is a sit target set or not. + /// + public int GetSittingAvatarsCount() + { + int count = 0; + + Array.ForEach(m_parts.GetArray(), p => count += p.GetSittingAvatarsCount()); + + return count; + } + public override string ToString() { return String.Format("{0} {1} ({2})", Name, UUID, AbsolutePosition); 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; + } } } diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index c7a670f..c6a2a03 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -578,6 +578,12 @@ namespace OpenSim.Region.Framework.Scenes public uint ParentID { get; set; } /// + /// Are we sitting on an object? + /// + /// A more readable way of testing presence sit status than ParentID == 0 + public bool IsSatOnObject { get { return ParentID != 0; } } + + /// /// If the avatar is sitting, the prim that it's sitting on. If not sitting then null. /// /// @@ -1808,6 +1814,8 @@ namespace OpenSim.Region.Framework.Scenes SendAvatarDataToAllAgents(); m_requestedSitTargetID = 0; + part.RemoveSittingAvatar(UUID); + if (part != null) part.ParentGroup.TriggerScriptChangedEvent(Changed.LINK); } @@ -1887,7 +1895,7 @@ namespace OpenSim.Region.Framework.Scenes ) )); -// m_log.DebugFormat("[SCENE PRESENCE]: {0} {1}", SitTargetisSet, SitTargetUnOccupied); + m_log.DebugFormat("[SCENE PRESENCE]: {0} {1}", SitTargetisSet, SitTargetUnOccupied); if (PhysicsActor != null) m_sitAvatarHeight = PhysicsActor.Size.Z; @@ -1920,6 +1928,12 @@ namespace OpenSim.Region.Framework.Scenes AbsolutePosition = pos + new Vector3(0.0f, 0.0f, m_sitAvatarHeight); canSit = true; } +// else +// { +// m_log.DebugFormat( +// "[SCENE PRESENCE]: Ignoring sit request of {0} on {1} {2} because sit target is unset and outside 10m", +// Name, part.Name, part.LocalId); +// } } if (canSit) @@ -1930,6 +1944,8 @@ namespace OpenSim.Region.Framework.Scenes RemoveFromPhysicalScene(); } + part.AddSittingAvatar(UUID); + cameraAtOffset = part.GetCameraAtOffset(); cameraEyeOffset = part.GetCameraEyeOffset(); forceMouselook = part.GetForceMouselook(); diff --git a/OpenSim/Tests/Common/Helpers/SceneHelpers.cs b/OpenSim/Tests/Common/Helpers/SceneHelpers.cs index d5354cb..769de83 100644 --- a/OpenSim/Tests/Common/Helpers/SceneHelpers.cs +++ b/OpenSim/Tests/Common/Helpers/SceneHelpers.cs @@ -412,26 +412,49 @@ namespace OpenSim.Tests.Common /// public static AgentCircuitData GenerateAgentData(UUID agentId) { - string firstName = "testfirstname"; + AgentCircuitData acd = GenerateCommonAgentData(); - AgentCircuitData agentData = new AgentCircuitData(); - agentData.AgentID = agentId; - agentData.firstname = firstName; - agentData.lastname = "testlastname"; + acd.AgentID = agentId; + acd.firstname = "testfirstname"; + acd.lastname = "testlastname"; + acd.ServiceURLs = new Dictionary(); + + return acd; + } + + /// + /// Generate some standard agent connection data. + /// + /// + /// + public static AgentCircuitData GenerateAgentData(UserAccount ua) + { + AgentCircuitData acd = GenerateCommonAgentData(); + + acd.AgentID = ua.PrincipalID; + acd.firstname = ua.FirstName; + acd.lastname = ua.LastName; + acd.ServiceURLs = ua.ServiceURLs; + + return acd; + } + + private static AgentCircuitData GenerateCommonAgentData() + { + AgentCircuitData acd = new AgentCircuitData(); // XXX: Sessions must be unique, otherwise one presence can overwrite another in NullPresenceData. - agentData.SessionID = UUID.Random(); - agentData.SecureSessionID = UUID.Random(); - - agentData.circuitcode = 123; - agentData.BaseFolder = UUID.Zero; - agentData.InventoryFolder = UUID.Zero; - agentData.startpos = Vector3.Zero; - agentData.CapsPath = "http://wibble.com"; - agentData.ServiceURLs = new Dictionary(); - agentData.Appearance = new AvatarAppearance(); - - return agentData; + acd.SessionID = UUID.Random(); + acd.SecureSessionID = UUID.Random(); + + acd.circuitcode = 123; + acd.BaseFolder = UUID.Zero; + acd.InventoryFolder = UUID.Zero; + acd.startpos = Vector3.Zero; + acd.CapsPath = "http://wibble.com"; + acd.Appearance = new AvatarAppearance(); + + return acd; } /// @@ -440,6 +463,9 @@ namespace OpenSim.Tests.Common /// /// This can be used for tests where there is only one region or where there are multiple non-neighbour regions /// and teleport doesn't take place. + /// + /// XXX: Use the version of this method that takes the UserAccount structure wherever possible - this will + /// make the agent circuit data (e.g. first, lastname) consistent with the user account data. /// /// /// @@ -452,6 +478,10 @@ namespace OpenSim.Tests.Common /// /// Add a root agent where the details of the agent connection (apart from the id) are unimportant for the test /// + /// + /// XXX: Use the version of this method that takes the UserAccount structure wherever possible - this will + /// make the agent circuit data (e.g. first, lastname) consistent with the user account data. + /// /// /// /// @@ -464,6 +494,17 @@ namespace OpenSim.Tests.Common /// /// Add a root agent. /// + /// + /// + /// + public static ScenePresence AddScenePresence(Scene scene, UserAccount ua) + { + return AddScenePresence(scene, GenerateAgentData(ua)); + } + + /// + /// Add a root agent. + /// /// /// This function /// diff --git a/OpenSim/Tests/Common/Helpers/UserAccountHelpers.cs b/OpenSim/Tests/Common/Helpers/UserAccountHelpers.cs index 3d3e65c..2fbebc4 100644 --- a/OpenSim/Tests/Common/Helpers/UserAccountHelpers.cs +++ b/OpenSim/Tests/Common/Helpers/UserAccountHelpers.cs @@ -138,6 +138,15 @@ namespace OpenSim.Tests.Common CreateUserWithInventory(scene, ua, pw); return ua; } + + public static UserAccount CreateUserWithInventory( + Scene scene, string firstName, string lastName, int userId, string pw) + { + UserAccount ua + = new UserAccount(TestHelpers.ParseTail(userId)) { FirstName = firstName, LastName = lastName }; + CreateUserWithInventory(scene, ua, pw); + return ua; + } public static void CreateUserWithInventory(Scene scene, UserAccount ua, string pw) { -- cgit v1.1