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.
master-beforevarregion
Justin Clark-Casey (justincc) 2014-01-27 23:47:43 +00:00
parent 1b86239f79
commit a4017ee1eb
1 changed files with 31 additions and 15 deletions

View File

@ -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 // 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 // be done in AttachmentsModule.CopyAttachments(AgentData ad, IScenePresence sp) itself since we are
// not transporting the required data. // not transporting the required data.
lock (m_attachments) //
{ // We need to restart scripts here so that they receive the correct changed events (CHANGED_TELEPORT
if (HasAttachments()) // 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
m_log.DebugFormat( // not transporting the required data.
"[SCENE PRESENCE]: Restarting scripts in attachments for {0} in {1}", Name, Scene.Name); //
// 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<SceneObjectGroup> attachments = GetAttachments();
// Resume scripts if (attachments.Count > 0)
Util.FireAndForget(delegate(object x) { {
foreach (SceneObjectGroup sog in m_attachments) m_log.DebugFormat(
{ "[SCENE PRESENCE]: Restarting scripts in attachments for {0} in {1}", Name, Scene.Name);
sog.ScheduleGroupForFullUpdate();
sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource()); // Resume scripts
sog.ResumeScripts(); 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 // again here... this comes after the cached appearance check because the avatars
// appearance goes into the avatar update packet // appearance goes into the avatar update packet
SendAvatarDataToAllAgents(); SendAvatarDataToAllAgents();
// This invocation always shows up in the viewer logs as an error. Is it needed?
SendAppearanceToAgent(this); SendAppearanceToAgent(this);
// If we are using the the cached appearance then send it out to everyone // If we are using the the cached appearance then send it out to everyone