OpenSim
/
OpenSimMirror
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Make SP.Attachments available as sp.GetAttachments() instead. 

The approach here, as in other parts of OpenSim, is to return a copy of the list rather than the attachments list itself
This prevents callers from forgetting to lock the list when they read it, as was happening in various parts of the codebase.
It also improves liveness.
This might improve attachment anomolies when performing region crossings.
remove-scene-viewer
Justin Clark-Casey (justincc) 2011-08-31 16:29:51 +01:00
parent 2acfff9f6d
commit 32444d98cb
7 changed files with 101 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
View File

@ -136,10 +136,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
public void SaveChangedAttachments(IScenePresence sp)
{
// Need to copy this list because DetachToInventoryPrep mods it
List<SceneObjectGroup> attachments = new List<SceneObjectGroup>(sp.Attachments);
foreach (SceneObjectGroup grp in attachments)
foreach (SceneObjectGroup grp in sp.GetAttachments())
{
if (grp.HasGroupChanged) // Resizer scripts?
{
@ -273,14 +270,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
// Remove any previous attachments
UUID itemID = UUID.Zero;
foreach (SceneObjectGroup grp in sp.Attachments)
{
if (grp.AttachmentPoint == attachmentPt)
{
itemID = grp.GetFromItemID();
break;
}
}
List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
// At the moment we can only deal with a single attachment
if (attachments.Count != 0)
itemID = attachments[0].GetFromItemID();
if (itemID != UUID.Zero)
DetachSingleAttachmentToInv(itemID, sp);

10
OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
View File

@ -106,7 +106,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
// Check status on scene presence
Assert.That(m_presence.HasAttachments(), Is.True);
List<SceneObjectGroup> attachments = m_presence.Attachments;
List<SceneObjectGroup> attachments = m_presence.GetAttachments();
Assert.That(attachments.Count, Is.EqualTo(1));
SceneObjectGroup attSo = attachments[0];
Assert.That(attSo.Name, Is.EqualTo(attName));
@ -140,7 +140,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
// Check scene presence status
Assert.That(m_presence.HasAttachments(), Is.True);
List<SceneObjectGroup> attachments = m_presence.Attachments;
List<SceneObjectGroup> attachments = m_presence.GetAttachments();
Assert.That(attachments.Count, Is.EqualTo(1));
SceneObjectGroup attSo = attachments[0];
Assert.That(attSo.Name, Is.EqualTo(attName));
@ -174,7 +174,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
// Check scene presence status
Assert.That(m_presence.HasAttachments(), Is.False);
List<SceneObjectGroup> attachments = m_presence.Attachments;
List<SceneObjectGroup> attachments = m_presence.GetAttachments();
Assert.That(attachments.Count, Is.EqualTo(0));
// Check appearance status
@ -208,7 +208,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
// Check status on scene presence
Assert.That(m_presence.HasAttachments(), Is.False);
List<SceneObjectGroup> attachments = m_presence.Attachments;
List<SceneObjectGroup> attachments = m_presence.GetAttachments();
Assert.That(attachments.Count, Is.EqualTo(0));
// Check item status
@ -237,7 +237,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd);
Assert.That(presence.HasAttachments(), Is.True);
List<SceneObjectGroup> attachments = presence.Attachments;
List<SceneObjectGroup> attachments = presence.GetAttachments();
Assert.That(attachments.Count, Is.EqualTo(1));
SceneObjectGroup attSo = attachments[0];

25
OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
View File

@ -208,7 +208,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
sp.ControllingClient.SendLocalTeleport(position, lookAt, teleportFlags);
sp.Teleport(position);
foreach (SceneObjectGroup grp in sp.Attachments)
foreach (SceneObjectGroup grp in sp.GetAttachments())
sp.Scene.EventManager.TriggerOnScriptChangedEvent(grp.LocalId, (uint)Changed.TELEPORT);
}
else // Another region possibly in another simulator
@ -559,11 +559,11 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
protected virtual void AgentHasMovedAway(ScenePresence sp, bool logout)
{
foreach (SceneObjectGroup sop in sp.Attachments)
foreach (SceneObjectGroup sop in sp.GetAttachments())
{
sop.Scene.DeleteSceneObject(sop, true);
}
sp.Attachments.Clear();
sp.ClearAttachments();
}
protected void KillEntity(Scene scene, uint localID)
@ -1764,9 +1764,8 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
protected bool CrossAttachmentsIntoNewRegion(GridRegion destination, ScenePresence sp, bool silent)
{
List<SceneObjectGroup> m_attachments = sp.Attachments;
lock (m_attachments)
{
List<SceneObjectGroup> m_attachments = sp.GetAttachments();
// Validate
foreach (SceneObjectGroup gobj in m_attachments)
{
@ -1788,11 +1787,11 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
CrossPrimGroupIntoNewRegion(destination, gobj, silent);
}
}
m_attachments.Clear();
sp.ClearAttachments();
return true;
}
}
#endregion
@ -1840,7 +1839,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
int i = 0;
if (sp.InTransitScriptStates.Count > 0)
{
sp.Attachments.ForEach(delegate(SceneObjectGroup sog)
List<SceneObjectGroup> attachments = sp.GetAttachments();
foreach (SceneObjectGroup sog in attachments)
{
if (i < sp.InTransitScriptStates.Count)
{
@ -1849,8 +1850,10 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
sog.ResumeScripts();
}
else
m_log.ErrorFormat("[ENTITY TRANSFER MODULE]: InTransitScriptStates.Count={0} smaller than Attachments.Count={1}", sp.InTransitScriptStates.Count, sp.Attachments.Count);
});
m_log.ErrorFormat(
"[ENTITY TRANSFER MODULE]: InTransitScriptStates.Count={0} smaller than Attachments.Count={1}",
sp.InTransitScriptStates.Count, attachments.Count);
}
sp.InTransitScriptStates.Clear();
}

12
OpenSim/Region/CoreModules/Scripting/WorldComm/WorldCommModule.cs
View File

@ -283,7 +283,7 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
}
/// <summary>
/// Delivers the message to.
/// Delivers the message to a scene entity.
/// </summary>
/// <param name='target'>
/// Target.
@ -314,17 +314,19 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
m_scene.SimChatBroadcast(Utils.StringToBytes(msg), ChatTypeEnum.Owner, 0, pos, name, id, false);
}
List<SceneObjectGroup> attachments = sp.Attachments;
List<SceneObjectGroup> attachments = sp.GetAttachments();
// Nothing left to do
if (attachments == null)
return true;
// Get uuid of attachments
List<UUID> targets = new List<UUID>();
foreach ( SceneObjectGroup sog in attachments )
foreach (SceneObjectGroup sog in attachments)
{
targets.Add(sog.UUID);
}
// Need to check each attachment
foreach (ListenerInfo li in m_listenerManager.GetListeners(UUID.Zero, channel, name, id, msg))
{
@ -334,9 +336,10 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
if (m_scene.GetSceneObjectPart(li.GetHostID()) == null)
continue;
if ( targets.Contains(li.GetHostID()))
if (targets.Contains(li.GetHostID()))
QueueMessage(new ListenerInfo(li, name, id, msg));
}
return true;
}
@ -363,6 +366,7 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
break;
}
}
return true;
}

7
OpenSim/Region/Framework/Interfaces/IScenePresence.cs
View File

@ -58,10 +58,13 @@ namespace OpenSim.Region.Framework.Interfaces
/// <summary>
/// The scene objects attached to this avatar.
/// </summary>
/// <returns>
/// A copy of the list.
/// </returns>
/// <remarks>
/// Do not change this list directly - use methods such as
/// AddAttachment() and RemoveAttachment(). Lock this list when performing any read operations upon it.
/// AddAttachment() and RemoveAttachment().
/// </remarks>
List<SceneObjectGroup> Attachments { get; }
List<SceneObjectGroup> GetAttachments();
}
}

41
OpenSim/Region/Framework/Scenes/ScenePresence.cs
View File

@ -114,10 +114,13 @@ namespace OpenSim.Region.Framework.Scenes
}
protected ScenePresenceAnimator m_animator;
public List<SceneObjectGroup> Attachments
{
get { return m_attachments; }
}
/// <summary>
/// Attachments recorded on this avatar.
/// </summary>
/// <remarks>
/// TODO: For some reason, we effectively have a list both here and in Appearance. Need to work out if this is
/// necessary.
/// </remarks>
protected List<SceneObjectGroup> m_attachments = new List<SceneObjectGroup>();
private Dictionary<UUID, ScriptControllers> scriptedcontrols = new Dictionary<UUID, ScriptControllers>();
@ -940,15 +943,18 @@ namespace OpenSim.Region.Framework.Scenes
// and it has already rezzed the attachments and started their scripts.
// We do the following only for non-login agents, because their scripts
// haven't started yet.
if (wasChild && Attachments != null && Attachments.Count > 0)
lock (m_attachments)
{
if (wasChild && m_attachments != null && m_attachments.Count > 0)
{
m_log.DebugFormat("[SCENE PRESENCE]: Restarting scripts in attachments...");
// Resume scripts
Attachments.ForEach(delegate(SceneObjectGroup sog)
foreach (SceneObjectGroup sog in m_attachments)
{
sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource());
sog.ResumeScripts();
});
}
}
}
// send the animations of the other presences to me
@ -3474,7 +3480,17 @@ namespace OpenSim.Region.Framework.Scenes
}
/// <summary>
/// Get the scene object attached to the given point.
/// Get all the presence's attachments.
/// </summary>
/// <returns>A copy of the list which contains the attachments.</returns>
public List<SceneObjectGroup> GetAttachments()
{
lock (m_attachments)
return new List<SceneObjectGroup>(m_attachments);
}
/// <summary>
/// Get the scene objects attached to the given point.
/// </summary>
/// <param name="attachmentPoint"></param>
/// <returns>Returns an empty list if there were no attachments at the point.</returns>
@ -3521,6 +3537,15 @@ namespace OpenSim.Region.Framework.Scenes
m_attachments.Remove(gobj);
}
/// <summary>
/// Clear all attachments
/// </summary>
public void ClearAttachments()
{
lock (m_attachments)
m_attachments.Clear();
}
public bool ValidateAttachments()
{
lock (m_attachments)

8
OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
View File

@ -144,14 +144,10 @@ namespace OpenSim.Region.OptionalModules.World.NPC
return false;
// FIXME: An extremely bad bit of code that reaches directly into the attachments list and manipulates it
List<SceneObjectGroup> attachments = sp.Attachments;
lock (attachments)
{
foreach (SceneObjectGroup att in attachments)
foreach (SceneObjectGroup att in sp.GetAttachments())
scene.DeleteSceneObject(att, false);
attachments.Clear();
}
sp.ClearAttachments();
AvatarAppearance npcAppearance = new AvatarAppearance(appearance, true);
sp.Appearance = npcAppearance;