Fix recent regression where a race condition meant SP.MakeRootAgent() would sometimes look to start attachment scripts before ETM.HandleIncomingSceneObject() had added them.

Probably a regression since ghosts branch merge on Nov 26 2014
mb-throttle-test
Justin Clark-Casey (justincc) 2014-12-17 00:21:58 +00:00
parent e50aac020f
commit e901253b49
2 changed files with 39 additions and 36 deletions

View File

@ -341,11 +341,13 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
if (!Enabled) if (!Enabled)
return; return;
if (DebugLevel > 0)
m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
List<SceneObjectGroup> attachments = sp.GetAttachments(); List<SceneObjectGroup> attachments = sp.GetAttachments();
if (DebugLevel > 0)
m_log.DebugFormat(
"[ATTACHMENTS MODULE]: Saving for {0} attachments for {1} in {2}",
attachments.Count, sp.Name, m_scene.Name);
if (attachments.Count <= 0) if (attachments.Count <= 0)
return; return;
@ -359,6 +361,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
// This must be done outside the sp.AttachmentSyncLock so that there is no risk of a deadlock from // This must be done outside the sp.AttachmentSyncLock so that there is no risk of a deadlock from
// scripts performing attachment operations at the same time. Getting object states stops the scripts. // scripts performing attachment operations at the same time. Getting object states stops the scripts.
scriptStates[so] = PrepareScriptInstanceForSave(so, false); scriptStates[so] = PrepareScriptInstanceForSave(so, false);
// m_log.DebugFormat(
// "[ATTACHMENTS MODULE]: For object {0} for {1} in {2} got saved state {3}",
// so.Name, sp.Name, m_scene.Name, scriptStates[so]);
} }
lock (sp.AttachmentsSyncLock) lock (sp.AttachmentsSyncLock)
@ -826,8 +832,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
{ {
if (DebugLevel > 0) if (DebugLevel > 0)
m_log.DebugFormat( m_log.DebugFormat(
"[ATTACHMENTS MODULE]: Adding attachment {0} to avatar {1} in pt {2} pos {3} {4}", "[ATTACHMENTS MODULE]: Adding attachment {0} to avatar {1} at pt {2} pos {3} {4} in {5}",
so.Name, sp.Name, attachmentpoint, attachOffset, so.RootPart.AttachedPos); so.Name, sp.Name, attachmentpoint, attachOffset, so.RootPart.AttachedPos, m_scene.Name);
// Remove from database and parcel prim count // Remove from database and parcel prim count
m_scene.DeleteFromStorage(so.UUID); m_scene.DeleteFromStorage(so.UUID);

View File

@ -1228,45 +1228,27 @@ namespace OpenSim.Region.Framework.Scenes
m_scene.SwapRootAgentCount(false); m_scene.SwapRootAgentCount(false);
// The initial login scene presence is already root when it gets here if (Scene.AttachmentsModule != null)
// 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 (PresenceType == PresenceType.Npc || IsRealLogin(m_teleportFlags))
{ {
// Viewers which have a current outfit folder will actually rez their own attachments. However, // The initial login scene presence is already root when it gets here
// viewers without (e.g. v1 viewers) will not, so we still need to make this call. // and it has already rezzed the attachments and started their scripts.
if (Scene.AttachmentsModule != null) // We do the following only for non-login agents, because their scripts
// haven't started yet.
if (PresenceType == PresenceType.Npc || IsRealLogin(m_teleportFlags))
{ {
// Viewers which have a current outfit folder will actually rez their own attachments. However,
// viewers without (e.g. v1 viewers) will not, so we still need to make this call.
WorkManager.RunJob( WorkManager.RunJob(
"RezAttachments", "RezAttachments",
o => Scene.AttachmentsModule.RezAttachments(this), o => Scene.AttachmentsModule.RezAttachments(this),
null, null,
string.Format("Rez attachments for {0} in {1}", Name, Scene.Name)); string.Format("Rez attachments for {0} in {1}", Name, Scene.Name));
} }
} else
else
{
// 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.
List<SceneObjectGroup> attachments = GetAttachments();
if (attachments.Count > 0)
{ {
WorkManager.RunJob( WorkManager.RunJob(
"StartAttachmentScripts", "StartAttachmentScripts",
o => RestartAttachmentScripts(attachments), o => RestartAttachmentScripts(),
null, null,
string.Format("Start attachment scripts for {0} in {1}", Name, Scene.Name), string.Format("Start attachment scripts for {0} in {1}", Name, Scene.Name),
true); true);
@ -1292,10 +1274,25 @@ namespace OpenSim.Region.Framework.Scenes
return true; return true;
} }
private void RestartAttachmentScripts(List<SceneObjectGroup> attachments) private void RestartAttachmentScripts()
{ {
// 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.
List<SceneObjectGroup> attachments = GetAttachments();
m_log.DebugFormat( m_log.DebugFormat(
"[SCENE PRESENCE]: Restarting scripts in attachments for {0} in {1}", Name, Scene.Name); "[SCENE PRESENCE]: Restarting scripts in {0} attachments for {1} in {2}", attachments.Count, Name, Scene.Name);
// Resume scripts // Resume scripts
foreach (SceneObjectGroup sog in attachments) foreach (SceneObjectGroup sog in attachments)