From 1b86239f791754e2c3ba7bf2641db5882efb0c80 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Mon, 27 Jan 2014 23:17:09 +0000 Subject: refactor: Remove identical part.ParentGroup.AddAvatar(UUID); calls which occur no matter which branch of the conditional is executed --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 3 +-- 1 file changed, 1 insertion(+), 2 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 85a20e9..0cc00ed 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -2909,7 +2909,6 @@ namespace OpenSim.Region.Framework.Scenes Rotation = newRot; // ParentPosition = part.AbsolutePosition; - part.ParentGroup.AddAvatar(UUID); } else { @@ -2918,13 +2917,13 @@ namespace OpenSim.Region.Framework.Scenes m_pos -= part.GroupPosition; // ParentPosition = part.AbsolutePosition; - part.ParentGroup.AddAvatar(UUID); // m_log.DebugFormat( // "[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); } + part.ParentGroup.AddAvatar(UUID); ParentPart = m_scene.GetSceneObjectPart(m_requestedSitTargetID); ParentID = m_requestedSitTargetID; m_AngularVelocity = Vector3.Zero; -- cgit v1.1 From a4017ee1eb30af8af4ac08c8003a796fcdd6f4a8 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Mon, 27 Jan 2014 23:47:43 +0000 Subject: Reinsert attachments list taking code in SP.MakeRootAgent() Locking attachments then launching script instances on a separate thread will not work, attachments will simply be unlocked and vulnerable to race conditions. --- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 44 ++++++++++++++++-------- 1 file changed, 30 insertions(+), 14 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 0cc00ed..84201cc 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -1190,22 +1190,36 @@ namespace OpenSim.Region.Framework.Scenes // and CHANGED_REGION) when the attachments have been rezzed in the new region. This cannot currently // be done in AttachmentsModule.CopyAttachments(AgentData ad, IScenePresence sp) itself since we are // not transporting the required data. - lock (m_attachments) + // + // We need to restart scripts here so that they receive the correct changed events (CHANGED_TELEPORT + // and CHANGED_REGION) when the attachments have been rezzed in the new region. This cannot currently + // be done in AttachmentsModule.CopyAttachments(AgentData ad, IScenePresence sp) itself since we are + // not transporting the required data. + // + // We must take a copy of the attachments list here (rather than locking) to avoid a deadlock where a script in one of + // the attachments may start processing an event (which locks ScriptInstance.m_Script) that then calls a method here + // which needs to lock m_attachments. ResumeScripts() needs to take a ScriptInstance.m_Script lock to try to unset the Suspend status. + // + // FIXME: In theory, this deadlock should not arise since scripts should not be processing events until ResumeScripts(). + // But XEngine starts all scripts unsuspended. Starting them suspended will not currently work because script rezzing + // is placed in an asynchronous queue in XEngine and so the ResumeScripts() call will almost certainly execute before the + // script is rezzed. This means the ResumeScripts() does absolutely nothing when using XEngine. + // + // One cannot simply iterate over attachments in a fire and forget thread because this would no longer + // be locked, allowing race conditions if other code changes the attachments list. + List attachments = GetAttachments(); + + if (attachments.Count > 0) { - if (HasAttachments()) - { - m_log.DebugFormat( - "[SCENE PRESENCE]: Restarting scripts in attachments for {0} in {1}", Name, Scene.Name); + m_log.DebugFormat( + "[SCENE PRESENCE]: Restarting scripts in attachments for {0} in {1}", Name, Scene.Name); - // Resume scripts - Util.FireAndForget(delegate(object x) { - foreach (SceneObjectGroup sog in m_attachments) - { - sog.ScheduleGroupForFullUpdate(); - sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource()); - sog.ResumeScripts(); - } - }); + // Resume scripts + foreach (SceneObjectGroup sog in attachments) + { + sog.ScheduleGroupForFullUpdate(); + sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource()); + sog.ResumeScripts(); } } } @@ -3227,6 +3241,8 @@ namespace OpenSim.Region.Framework.Scenes // again here... this comes after the cached appearance check because the avatars // appearance goes into the avatar update packet SendAvatarDataToAllAgents(); + + // This invocation always shows up in the viewer logs as an error. Is it needed? SendAppearanceToAgent(this); // If we are using the the cached appearance then send it out to everyone -- cgit v1.1