Avoid a deadlock where a script can attempt to take a ScriptInstance.m_Scripts lock then a lock on SP.m_attachments whilst SP.MakeRootAgent() attempts to take in the opposite order.
This is because scripts (at least on XEngine) start unsuspended - deceptively the ResumeScripts() calls in various places in the code are actually completely redundant (and useless). The solution chosen here is to use a copy of the SP attachments and not have the list locked whilst creating the scripts when an avatar enters the region. This looks to address http://opensimulator.org/mantis/view.php?id=6557cpu-performance
							parent
							
								
									b5d0ac4c42
								
							
						
					
					
						commit
						f41fc4eb25
					
				| 
						 | 
				
			
			@ -121,6 +121,8 @@ namespace OpenSim.Region.Framework.Scenes
 | 
			
		|||
        /// <remarks>
 | 
			
		||||
        /// TODO: For some reason, we effectively have a list both here and in Appearance.  Need to work out if this is
 | 
			
		||||
        /// necessary.
 | 
			
		||||
        /// NOTE: To avoid deadlocks, do not lock m_attachments and then perform other tasks under that lock.  Take a copy
 | 
			
		||||
        /// of the list and act on that instead.
 | 
			
		||||
        /// </remarks>
 | 
			
		||||
        private List<SceneObjectGroup> m_attachments = new List<SceneObjectGroup>();
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -971,22 +973,30 @@ 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)
 | 
			
		||||
                {
 | 
			
		||||
                    if (HasAttachments())
 | 
			
		||||
                //
 | 
			
		||||
                // 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)
 | 
			
		||||
                {
 | 
			
		||||
                    m_log.DebugFormat(
 | 
			
		||||
                        "[SCENE PRESENCE]: Restarting scripts in attachments for {0} in {1}", Name, Scene.Name);
 | 
			
		||||
 | 
			
		||||
                    // Resume scripts
 | 
			
		||||
                        foreach (SceneObjectGroup sog in m_attachments)
 | 
			
		||||
                    foreach (SceneObjectGroup sog in attachments)
 | 
			
		||||
                    {
 | 
			
		||||
                        sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource());
 | 
			
		||||
                        sog.ResumeScripts();
 | 
			
		||||
                    }
 | 
			
		||||
                }
 | 
			
		||||
            }
 | 
			
		||||
            }
 | 
			
		||||
 | 
			
		||||
            // send the animations of the other presences to me
 | 
			
		||||
            m_scene.ForEachRootScenePresence(delegate(ScenePresence presence)
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue