When attachments are being saved and deleted for a closing root agent, delete first to avoid a hud race condition with update threads.

If delete doesn't occur first then the update thread can outrace the IsAttachment = false necessary to save attachments and send hud artifacts to other viewers.

4 files changed, 42 insertions, 29 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index 99e0153..2b0e4ab 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -152,31 +152,40 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             }
         }
 
-        public void SaveChangedAttachments(IScenePresence sp, bool saveAllScripted)
+        public void DeRezAttachments(IScenePresence sp, bool saveChanged, bool saveAllScripted)
         {
-//            m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
-
             if (!Enabled)
                 return;
 
-            foreach (SceneObjectGroup grp in sp.GetAttachments())
+//            m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
+
+            lock (sp.AttachmentsSyncLock)
             {
-                grp.IsAttachment = false;
-                grp.AbsolutePosition = grp.RootPart.AttachedPos;
-                UpdateKnownItem(sp, grp, saveAllScripted);
-                grp.IsAttachment = true;
+                foreach (SceneObjectGroup grp in sp.GetAttachments())
+                {
+                    grp.Scene.DeleteSceneObject(grp, false);
+    
+                    if (saveChanged || saveAllScripted)
+                    {
+                        grp.IsAttachment = false;
+                        grp.AbsolutePosition = grp.RootPart.AttachedPos;
+                        UpdateKnownItem(sp, grp, saveAllScripted);
+                    }
+                }
+    
+                sp.ClearAttachments();
             }
         }
 
         public void DeleteAttachmentsFromScene(IScenePresence sp, bool silent)
         {
-//            m_log.DebugFormat(
-//                "[ATTACHMENTS MODULE]: Deleting attachments from scene {0} for {1}, silent = {2}",
-//                m_scene.RegionInfo.RegionName, sp.Name, silent);
-
             if (!Enabled)
                 return;
 
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: Deleting attachments from scene {0} for {1}, silent = {2}",
+//                m_scene.RegionInfo.RegionName, sp.Name, silent);            
+
             foreach (SceneObjectGroup sop in sp.GetAttachments())
             {
                 sop.Scene.DeleteSceneObject(sop, silent);
diff --git a/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs b/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs
index eb07165..fde5de1 100644
--- a/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs
+++ b/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs
@@ -43,10 +43,15 @@ namespace OpenSim.Region.Framework.Interfaces
         void RezAttachments(IScenePresence sp);
 
         /// <summary>
-        /// Save the attachments that have change on this presence.
+        /// Derez the attachements for a scene presence that is closing.
         /// </summary>
-        /// <param name="sp"></param>
-        void SaveChangedAttachments(IScenePresence sp, bool saveAllScripted);
+        /// <remarks>
+        /// Attachment changes are saved.
+        /// </remarks>
+        /// <param name="sp">The presence closing</param>
+        /// <param name="saveChanged">Save changed attachments.</param>
+        /// <param name="saveAllScripted">Save attachments with scripts even if they haven't changed.</para>
+        void DeRezAttachments(IScenePresence sp, bool saveChanged, bool saveAllScripted);
 
         /// <summary>
         /// Delete all the presence's attachments from the scene
diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs
index 385febf..d449116 100644
--- a/OpenSim/Region/Framework/Scenes/Scene.cs
+++ b/OpenSim/Region/Framework/Scenes/Scene.cs
@@ -500,6 +500,7 @@ namespace OpenSim.Region.Framework.Scenes
         public IAttachmentsModule AttachmentsModule { get; set; }
         public IEntityTransferModule EntityTransferModule { get; private set; }
         public IAgentAssetTransactions AgentTransactionsModule { get; private set; }
+        public IUserManagement UserManagementModule { get; private set; }
 
         public IAvatarFactoryModule AvatarFactory
         {
@@ -1243,6 +1244,7 @@ namespace OpenSim.Region.Framework.Scenes
             EntityTransferModule = RequestModuleInterface<IEntityTransferModule>();
             m_groupsModule = RequestModuleInterface<IGroupsModule>();
             AgentTransactionsModule = RequestModuleInterface<IAgentAssetTransactions>();
+            UserManagementModule = RequestModuleInterface<IUserManagement>();
         }
 
         #endregion
@@ -2021,9 +2023,8 @@ namespace OpenSim.Region.Framework.Scenes
                 sceneObject.SetGroup(groupID, null);
             }
 
-            IUserManagement uman = RequestModuleInterface<IUserManagement>();
-            if (uman != null)
-                sceneObject.RootPart.CreatorIdentification = uman.GetUserUUI(ownerID);
+            if (UserManagementModule != null)
+                sceneObject.RootPart.CreatorIdentification = UserManagementModule.GetUserUUI(ownerID);
 
             sceneObject.ScheduleGroupForFullUpdate();
 
@@ -2711,14 +2712,13 @@ namespace OpenSim.Region.Framework.Scenes
         /// <param name="aCircuit"></param>
         private void CacheUserName(ScenePresence sp, AgentCircuitData aCircuit)
         {
-            IUserManagement uMan = RequestModuleInterface<IUserManagement>();
-            if (uMan != null)
+            if (UserManagementModule != null)
             {
                 string first = aCircuit.firstname, last = aCircuit.lastname;
 
                 if (sp.PresenceType == PresenceType.Npc)
                 {
-                    uMan.AddUser(aCircuit.AgentID, first, last);
+                    UserManagementModule.AddUser(aCircuit.AgentID, first, last);
                 }
                 else
                 {
@@ -2737,7 +2737,7 @@ namespace OpenSim.Region.Framework.Scenes
                         }
                     }
 
-                    uMan.AddUser(aCircuit.AgentID, first, last, homeURL);
+                    UserManagementModule.AddUser(aCircuit.AgentID, first, last, homeURL);
                 }
             }
         }
@@ -3292,17 +3292,19 @@ namespace OpenSim.Region.Framework.Scenes
 
                 if (!isChildAgent)
                 {
-                    if (AttachmentsModule != null && avatar.PresenceType != PresenceType.Npc)
+                    if (AttachmentsModule != null)
                     {
-                        IUserManagement uMan = RequestModuleInterface<IUserManagement>();
                         // Don't save attachments for HG visitors, it
                         // messes up their inventory. When a HG visitor logs
                         // out on a foreign grid, their attachments will be
                         // reloaded in the state they were in when they left
                         // the home grid. This is best anyway as the visited
                         // grid may use an incompatible script engine.
-                        if (uMan == null || uMan.IsLocalGridUser(avatar.UUID))
-                            AttachmentsModule.SaveChangedAttachments(avatar, false);
+                        bool saveChanged
+                            = avatar.PresenceType != PresenceType.Npc
+                                && (UserManagementModule == null || UserManagementModule.IsLocalGridUser(avatar.UUID));
+
+                        AttachmentsModule.DeRezAttachments(avatar, saveChanged, false);
                     }
 
                     ForEachClient(
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index 3909fd4..909c7c8 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -3416,9 +3416,6 @@ namespace OpenSim.Region.Framework.Scenes
 
         public void Close()
         {
-            if (!IsChildAgent && m_scene.AttachmentsModule != null)
-                m_scene.AttachmentsModule.DeleteAttachmentsFromScene(this, false);
-
             // Clear known regions
             KnownRegions = new Dictionary<ulong, string>();