Merge branch 'master' of melanie@opensimulator.org:/var/git/opensim

4 files changed, 84 insertions, 37 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs
index 5b228ee..13f0167 100644
--- a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs
@@ -62,6 +62,7 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles
         // count. The entries are removed when the interest count reaches 0.
         Dictionary<UUID,UUID> classifiedCache = new Dictionary<UUID, UUID>();
         Dictionary<UUID,int> classifiedInterest = new Dictionary<UUID, int>();
+        Object classifiedLock;
 
         public Scene Scene
         {
@@ -326,14 +327,17 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles
                 string name = m["name"].AsString();
 
                 classifieds[cid] = name;
-                
+
                 if(!classifiedCache.ContainsKey(cid))
                 {
-                    classifiedCache.Add(cid,creatorId);
-                    classifiedInterest.Add(cid, 0);
+                    lock(classifiedCache)
+                        classifiedCache.Add(cid,creatorId);
+                    lock(classifiedInterest)
+                        classifiedInterest.Add(cid, 0);
                 }
 
-                classifiedInterest[cid] ++;
+                lock(classifiedInterest)
+                    classifiedInterest[cid] ++;
             }
 
             remoteClient.SendAvatarClassifiedReply(new UUID(args[0]), classifieds);
@@ -346,22 +350,20 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles
             ad.ClassifiedId = queryClassifiedID;
 
             if(classifiedCache.ContainsKey(queryClassifiedID))
-            {
+            {   
                 target = classifiedCache[queryClassifiedID];
 
-                if(classifiedInterest[queryClassifiedID] -- == 0)
+                lock(classifiedInterest)
+                    classifiedInterest[queryClassifiedID] --;
+
+                if(classifiedInterest[queryClassifiedID] == 0)
                 {
+                    lock(classifiedInterest)
+                        classifiedInterest.Remove(queryClassifiedID);
                     lock(classifiedCache)
-                    {
-                        lock(classifiedInterest)
-                        {
-                            classifiedInterest.Remove(queryClassifiedID);
-                        }
                         classifiedCache.Remove(queryClassifiedID);
-                    }
                 }
-            }            
-
+            }
             
             string serverURI = string.Empty;
             bool foreign = GetUserProfileServerURI(target, out serverURI);
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index b8ff7f7..bab14dd 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -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,19 +973,27 @@ 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 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)
                 {
-                    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
-                        foreach (SceneObjectGroup sog in m_attachments)
-                        {
-                            sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource());
-                            sog.ResumeScripts();
-                        }
+                    // Resume scripts
+                    foreach (SceneObjectGroup sog in attachments)
+                    {
+                        sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource());
+                        sog.ResumeScripts();
                     }
                 }
             }
diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSParam.cs b/OpenSim/Region/Physics/BulletSPlugin/BSParam.cs
index afd547a..aad1108 100755
--- a/OpenSim/Region/Physics/BulletSPlugin/BSParam.cs
+++ b/OpenSim/Region/Physics/BulletSPlugin/BSParam.cs
@@ -360,7 +360,7 @@ public static class BSParam
         new ParameterDefn<bool>("UseSeparatePhysicsThread", "If 'true', the physics engine runs independent from the simulator heartbeat",
             false ),
         new ParameterDefn<float>("PhysicsTimeStep", "If separate thread, seconds to simulate each interval",
-            0.1f ),
+            0.089f ),
 
         new ParameterDefn<bool>("MeshSculptedPrim", "Whether to create meshes for sculpties",
             true,
diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSShapes.cs b/OpenSim/Region/Physics/BulletSPlugin/BSShapes.cs
index 81edc12..867d2ab 100755
--- a/OpenSim/Region/Physics/BulletSPlugin/BSShapes.cs
+++ b/OpenSim/Region/Physics/BulletSPlugin/BSShapes.cs
@@ -389,9 +389,21 @@ public class BSShapeMesh : BSShape
     }
     public override BSShape GetReference(BSScene pPhysicsScene, BSPhysObject pPrim)
     {
-        // Another reference to this shape is just counted.
-        IncrementReference();
-        return this;
+        BSShape ret = null;
+        // If the underlying shape is native, the actual shape has not been build (waiting for asset)
+        //     and we must create a copy of the native shape since they are never shared.
+        if (physShapeInfo.HasPhysicalShape && physShapeInfo.isNativeShape)
+        {
+            // TODO: decide when the native shapes should be freed. Check in Dereference?
+            ret = BSShapeNative.GetReference(pPhysicsScene, pPrim, BSPhysicsShapeType.SHAPE_BOX, FixedShapeKey.KEY_BOX);
+        }
+        else
+        {
+            // Another reference to this shape is just counted.
+            IncrementReference();
+            ret = this;
+        }
+        return ret;
     }
     public override void Dereference(BSScene physicsScene)
     {
@@ -560,9 +572,21 @@ public class BSShapeHull : BSShape
     }
     public override BSShape GetReference(BSScene pPhysicsScene, BSPhysObject pPrim)
     {
-        // Another reference to this shape is just counted.
-        IncrementReference();
-        return this;
+        BSShape ret = null;
+        // If the underlying shape is native, the actual shape has not been build (waiting for asset)
+        //     and we must create a copy of the native shape since they are never shared.
+        if (physShapeInfo.HasPhysicalShape && physShapeInfo.isNativeShape)
+        {
+            // TODO: decide when the native shapes should be freed. Check in Dereference?
+            ret = BSShapeNative.GetReference(pPhysicsScene, pPrim, BSPhysicsShapeType.SHAPE_BOX, FixedShapeKey.KEY_BOX);
+        }
+        else
+        {
+            // Another reference to this shape is just counted.
+            IncrementReference();
+            ret = this;
+        }
+        return ret;
     }
     public override void Dereference(BSScene physicsScene)
     {
@@ -1075,12 +1099,23 @@ public class BSShapeGImpact : BSShape
                             (w, iC, i, vC, v) => physicsScene.PE.CreateGImpactShape(w, iC, i, vC, v) );
     }
 
-    public override BSShape GetReference(BSScene physicsScene, BSPhysObject prim)
+    public override BSShape GetReference(BSScene pPhysicsScene, BSPhysObject pPrim)
     {
-        // Calling this reference means we want another handle to an existing shape
-        //     (usually linksets) so return this copy.
-        IncrementReference();
-        return this;
+        BSShape ret = null;
+        // If the underlying shape is native, the actual shape has not been build (waiting for asset)
+        //     and we must create a copy of the native shape since they are never shared.
+        if (physShapeInfo.HasPhysicalShape && physShapeInfo.isNativeShape)
+        {
+            // TODO: decide when the native shapes should be freed. Check in Dereference?
+            ret = BSShapeNative.GetReference(pPhysicsScene, pPrim, BSPhysicsShapeType.SHAPE_BOX, FixedShapeKey.KEY_BOX);
+        }
+        else
+        {
+            // Another reference to this shape is just counted.
+            IncrementReference();
+            ret = this;
+        }
+        return ret;
     }
     // Dereferencing a compound shape releases the hold on all the child shapes.
     public override void Dereference(BSScene physicsScene)