Bring us up to date.

2 files changed, 314 insertions, 231 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index 218b7b8..2bf418d 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -144,20 +144,26 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
         public void SaveChangedAttachments(IScenePresence sp)
         {
+//            m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
+
             foreach (SceneObjectGroup grp in sp.GetAttachments())
             {
-                if (grp.HasGroupChanged) // Resizer scripts?
-                {
+//                if (grp.HasGroupChanged) // Resizer scripts?
+//                {
                     grp.IsAttachment = false;
                     grp.AbsolutePosition = grp.RootPart.AttachedPos;
-                    UpdateKnownItem(sp.ControllingClient, grp, grp.GetFromItemID(), grp.OwnerID);
+                    UpdateKnownItem(sp.ControllingClient, grp);
                     grp.IsAttachment = true;
-                }
+//                }
             }
         }
 
         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);
+
             foreach (SceneObjectGroup sop in sp.GetAttachments())
             {
                 sop.Scene.DeleteSceneObject(sop, silent);
@@ -220,7 +226,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                     m_scene.EventManager.TriggerOnAttach(objectLocalID, part.ParentGroup.GetFromItemID(), remoteClient.AgentId);
 
                     // Save avatar attachment information
-                    m_log.Info(
+                    m_log.Debug(
                         "[ATTACHMENTS MODULE]: Saving avatar attachment. AgentID: " + remoteClient.AgentId
                         + ", AttachmentPoint: " + AttachmentPt);
 
@@ -248,75 +254,83 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         
         private bool AttachObject(IScenePresence sp, SceneObjectGroup group, uint attachmentPt, bool silent)
         {
-//            m_log.DebugFormat(
-//                "[ATTACHMENTS MODULE]: Attaching object {0} {1} to {2} point {3} from ground (silent = {4})",
-//                group.Name, group.LocalId, sp.Name, AttachmentPt, silent);
-
-            if (sp.GetAttachments(attachmentPt).Contains(group))
-            {
-//                m_log.WarnFormat(
-//                    "[ATTACHMENTS MODULE]: Ignoring request to attach {0} {1} to {2} on {3} since it's already attached",
-//                    group.Name, group.LocalId, sp.Name, AttachmentPt);
-
-                return false;
-            }
-
-            Vector3 attachPos = group.AbsolutePosition;
-
-            // TODO: this short circuits multiple attachments functionality  in  LL viewer 2.1+ and should
-            // be removed when that functionality is implemented in opensim
-            attachmentPt &= 0x7f;
-            
-            // If the attachment point isn't the same as the one previously used
-            // set it's offset position = 0 so that it appears on the attachment point
-            // and not in a weird location somewhere unknown.
-            if (attachmentPt != 0 && attachmentPt != group.AttachmentPoint)
-            {
-                attachPos = Vector3.Zero;
-            }
-
-            // AttachmentPt 0 means the client chose to 'wear' the attachment.
-            if (attachmentPt == 0)
+            lock (sp.AttachmentsSyncLock)
             {
-                // Check object for stored attachment point
-                attachmentPt = group.AttachmentPoint;
-            }
-
-            // if we still didn't find a suitable attachment point.......
-            if (attachmentPt == 0)
-            {
-                // Stick it on left hand with Zero Offset from the attachment point.
-                attachmentPt = (uint)AttachmentPoint.LeftHand;
-                attachPos = Vector3.Zero;
-            }
-
-            group.AttachmentPoint = attachmentPt;
-            group.AbsolutePosition = attachPos;
-
-            // Remove any previous attachments
-            UUID itemID = UUID.Zero;
-
-            List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
-
-            // At the moment we can only deal with a single attachment
-            // We also don't want to do any of the inventory operations for an NPC.
-            if (sp.PresenceType != PresenceType.Npc)
-            {
-                if (attachments.Count != 0)
-                    itemID = attachments[0].GetFromItemID();
+//                m_log.DebugFormat(
+//                    "[ATTACHMENTS MODULE]: Attaching object {0} {1} to {2} point {3} from ground (silent = {4})",
+//                    group.Name, group.LocalId, sp.Name, attachmentPt, silent);
+    
+                if (sp.GetAttachments(attachmentPt).Contains(group))
+                {
+    //                m_log.WarnFormat(
+    //                    "[ATTACHMENTS MODULE]: Ignoring request to attach {0} {1} to {2} on {3} since it's already attached",
+    //                    group.Name, group.LocalId, sp.Name, AttachmentPt);
+    
+                    return false;
+                }
+    
+                Vector3 attachPos = group.AbsolutePosition;
+    
+                // TODO: this short circuits multiple attachments functionality  in  LL viewer 2.1+ and should
+                // be removed when that functionality is implemented in opensim
+                attachmentPt &= 0x7f;
+                
+                // If the attachment point isn't the same as the one previously used
+                // set it's offset position = 0 so that it appears on the attachment point
+                // and not in a weird location somewhere unknown.
+                if (attachmentPt != 0 && attachmentPt != group.AttachmentPoint)
+                {
+                    attachPos = Vector3.Zero;
+                }
+    
+                // AttachmentPt 0 means the client chose to 'wear' the attachment.
+                if (attachmentPt == 0)
+                {
+                    // Check object for stored attachment point
+                    attachmentPt = group.AttachmentPoint;
+                }
+    
+                // if we still didn't find a suitable attachment point.......
+                if (attachmentPt == 0)
+                {
+                    // Stick it on left hand with Zero Offset from the attachment point.
+                    attachmentPt = (uint)AttachmentPoint.LeftHand;
+                    attachPos = Vector3.Zero;
+                }
+    
+                group.AttachmentPoint = attachmentPt;
+                group.AbsolutePosition = attachPos;
+    
+                // We also don't want to do any of the inventory operations for an NPC.
+                if (sp.PresenceType != PresenceType.Npc)
+                {
+                    // Remove any previous attachments
+                    List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
     
-                if (itemID != UUID.Zero)
-                    DetachSingleAttachmentToInv(itemID, sp);
+                    // At the moment we can only deal with a single attachment
+                    if (attachments.Count != 0)
+                    {
+                        UUID oldAttachmentItemID = attachments[0].GetFromItemID();
+        
+                        if (oldAttachmentItemID != UUID.Zero)
+                            DetachSingleAttachmentToInv(oldAttachmentItemID, sp);
+                        else
+                            m_log.WarnFormat(
+                                "[ATTACHMENTS MODULE]: When detaching existing attachment {0} {1} at point {2} to make way for {3} {4} for {5}, couldn't find the associated item ID to adjust inventory attachment record!",
+                                attachments[0].Name, attachments[0].LocalId, attachmentPt, group.Name, group.LocalId, sp.Name);
+                    }
     
-                itemID = group.GetFromItemID();
-                if (itemID == UUID.Zero)
-                    itemID = AddSceneObjectAsAttachment(sp.ControllingClient, group).ID;
+                    // Add the new attachment to inventory if we don't already have it.
+                    UUID newAttachmentItemID = group.GetFromItemID();
+                    if (newAttachmentItemID == UUID.Zero)
+                        newAttachmentItemID = AddSceneObjectAsNewAttachmentInInv(sp.ControllingClient, group).ID;
+        
+                    ShowAttachInUserInventory(sp, attachmentPt, newAttachmentItemID, group);
+                }
     
-                ShowAttachInUserInventory(sp, attachmentPt, itemID, group);
+                AttachToAgent(sp, group, attachmentPt, attachPos, silent);
             }
 
-            AttachToAgent(sp, group, attachmentPt, attachPos, silent);
-
             return true;
         }
 
@@ -325,20 +339,28 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             RezMultipleAttachmentsFromInvPacket.HeaderDataBlock header,
             RezMultipleAttachmentsFromInvPacket.ObjectDataBlock[] objects)
         {
-            foreach (RezMultipleAttachmentsFromInvPacket.ObjectDataBlock obj in objects)
+            ScenePresence sp = m_scene.GetScenePresence(remoteClient.AgentId);
+
+            if (sp == null)
             {
-                RezSingleAttachmentFromInventory(remoteClient, obj.ItemID, obj.AttachmentPt);
+                m_log.ErrorFormat(
+                    "[ATTACHMENTS MODULE]: Could not find presence for client {0} {1} in RezMultipleAttachmentsFromInventory()",
+                    remoteClient.Name, remoteClient.AgentId);
+                return;
+            }
+
+            lock (sp.AttachmentsSyncLock)
+            {
+//                m_log.DebugFormat("[ATTACHMENTS MODULE]: Rezzing multiple attachments from inventory for {0}", sp.Name);
+
+                foreach (RezMultipleAttachmentsFromInvPacket.ObjectDataBlock obj in objects)
+                {
+                    RezSingleAttachmentFromInventory(sp, obj.ItemID, obj.AttachmentPt);
+                }
             }
         }
         
-        public ISceneEntity RezSingleAttachmentFromInventory(
-            IClientAPI remoteClient, UUID itemID, uint AttachmentPt)
-        {
-            return RezSingleAttachmentFromInventory(remoteClient, itemID, AttachmentPt, true);
-        }
-
-        public ISceneEntity RezSingleAttachmentFromInventory(
-            IClientAPI remoteClient, UUID itemID, uint AttachmentPt, bool updateInventoryStatus)
+        public ISceneEntity RezSingleAttachmentFromInventory(IClientAPI remoteClient, UUID itemID, uint AttachmentPt)
         {
             return RezSingleAttachmentFromInventory(remoteClient, itemID, AttachmentPt, true, null);
         }
@@ -347,21 +369,59 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             IClientAPI remoteClient, UUID itemID, uint AttachmentPt, bool updateInventoryStatus, XmlDocument doc)
         {
             ScenePresence sp = m_scene.GetScenePresence(remoteClient.AgentId);
-                        if (sp == null)            {                m_log.ErrorFormat(                    "[ATTACHMENTS MODULE]: Could not find presence for client {0} {1} in RezSingleAttachmentFromInventory()",                    remoteClient.Name, remoteClient.AgentId);                return null;            }            
+
+            if (sp == null)
+            {
+                m_log.ErrorFormat(
+                    "[ATTACHMENTS MODULE]: Could not find presence for client {0} {1} in RezSingleAttachmentFromInventory()",
+                    remoteClient.Name, remoteClient.AgentId);
+                return null;
+            }
+
+            return RezSingleAttachmentFromInventory(sp, itemID, AttachmentPt);
+        }
+
+        public ISceneEntity RezSingleAttachmentFromInventory(ScenePresence sp, UUID itemID, uint AttachmentPt)
+        {
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: RezSingleAttachmentFromInventory to point {0} from item {1} for {2}",
+//                (AttachmentPoint)AttachmentPt, itemID, sp.Name);
+
             // TODO: this short circuits multiple attachments functionality  in  LL viewer 2.1+ and should
             // be removed when that functionality is implemented in opensim
             AttachmentPt &= 0x7f;
 
-            SceneObjectGroup att = RezSingleAttachmentFromInventoryInternal(sp, itemID, UUID.Zero, AttachmentPt, doc);
+            // Viewer 2/3 sometimes asks to re-wear items that are already worn (and show up in it's inventory as such).
+            // This often happens during login - not sure the exact reason.
+            // For now, we will ignore the request.  Unfortunately, this means that we need to dig through all the
+            // ScenePresence attachments.  We can't use the data in AvatarAppearance because that's present at login
+            // before anything has actually been attached.
+            bool alreadyOn = false;
+            List<SceneObjectGroup> existingAttachments = sp.GetAttachments();
+            foreach (SceneObjectGroup so in existingAttachments)
+            {
+                if (so.GetFromItemID() == itemID)
+                {
+                    alreadyOn = true;
+                    break;
+                }
+            }
 
-            if (updateInventoryStatus)
+//            if (sp.Appearance.GetAttachmentForItem(itemID) != null)
+            if (alreadyOn)
             {
-                if (att == null)
-                    DetachSingleAttachmentToInv(itemID, sp.ControllingClient);
-                else
-                    ShowAttachInUserInventory(att, sp, itemID, AttachmentPt);
+//                m_log.WarnFormat(
+//                    "[ATTACHMENTS MODULE]: Ignoring request by {0} to wear item {1} at {2} since it is already worn",
+//                    sp.Name, itemID, AttachmentPt);
+
+                return null;
             }
 
+            SceneObjectGroup att = RezSingleAttachmentFromInventoryInternal(sp, itemID, UUID.Zero, AttachmentPt, null);
+
+            if (att == null)
+                DetachSingleAttachmentToInv(itemID, sp.ControllingClient);
+
             return att;
         }
 
@@ -371,50 +431,67 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             IInventoryAccessModule invAccess = m_scene.RequestModuleInterface<IInventoryAccessModule>();
             if (invAccess != null)
             {
-                SceneObjectGroup objatt;
-
-                if (itemID != UUID.Zero)
-                    objatt = invAccess.RezObject(sp.ControllingClient,
-                        itemID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
-                        false, false, sp.UUID, true);
-                else
-                    objatt = invAccess.RezObject(sp.ControllingClient,
-                        null, assetID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
-                        false, false, sp.UUID, true);
-
-//                m_log.DebugFormat(
-//                    "[ATTACHMENTS MODULE]: Retrieved single object {0} for attachment to {1} on point {2}",
-//                    objatt.Name, remoteClient.Name, AttachmentPt);
-                
-                if (objatt != null)
+                lock (sp.AttachmentsSyncLock)
                 {
-                    // Loading the inventory from XML will have set this, but
-                    // there is no way the object could have changed yet,
-                    // since scripts aren't running yet. So, clear it here.
-                    objatt.HasGroupChanged = false;
-                    bool tainted = false;
-                    if (attachmentPt != 0 && attachmentPt != objatt.AttachmentPoint)
-                        tainted = true;
-
-                    // This will throw if the attachment fails
-                    try
+                    SceneObjectGroup objatt;
+    
+                    if (itemID != UUID.Zero)
+                        objatt = invAccess.RezObject(sp.ControllingClient,
+                            itemID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
+                            false, false, sp.UUID, true);
+                    else
+                        objatt = invAccess.RezObject(sp.ControllingClient,
+                            null, assetID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
+                            false, false, sp.UUID, true);
+    
+    //                m_log.DebugFormat(
+    //                    "[ATTACHMENTS MODULE]: Retrieved single object {0} for attachment to {1} on point {2}",
+    //                    objatt.Name, remoteClient.Name, AttachmentPt);
+                    
+                    if (objatt != null)
                     {
-                        AttachObject(sp, objatt, attachmentPt, false);
+                        // HasGroupChanged is being set from within RezObject.  Ideally it would be set by the caller.
+                        objatt.HasGroupChanged = false;
+                        bool tainted = false;
+                        if (attachmentPt != 0 && attachmentPt != objatt.AttachmentPoint)
+                            tainted = true;
+    
+                        // This will throw if the attachment fails
+                        try
+                        {
+                            AttachObject(sp, objatt, attachmentPt, false);
+                        }
+                        catch (Exception e)
+                        {
+                            m_log.ErrorFormat(
+                                "[ATTACHMENTS MODULE]: Failed to attach {0} {1} for {2}, exception {3}{4}",
+                                objatt.Name, objatt.UUID, sp.Name, e.Message, e.StackTrace);
+    
+                            // Make sure the object doesn't stick around and bail
+                            sp.RemoveAttachment(objatt);
+                            m_scene.DeleteSceneObject(objatt, false);
+                            return null;
+                        }
+                        
+                        if (tainted)
+                            objatt.HasGroupChanged = true;
+    
+                        // Fire after attach, so we don't get messy perms dialogs
+                        // 4 == AttachedRez
+                        objatt.CreateScriptInstances(0, true, m_scene.DefaultScriptEngine, 4);
+                        objatt.ResumeScripts();
+    
+                        // Do this last so that event listeners have access to all the effects of the attachment
+                        m_scene.EventManager.TriggerOnAttach(objatt.LocalId, itemID, sp.UUID);
+
+                        return objatt;
                     }
-                    catch (Exception e)
+                    else
                     {
-                        m_log.ErrorFormat(
-                            "[ATTACHMENTS MODULE]: Failed to attach {0} {1} for {2}, exception {3}{4}",
-                            objatt.Name, objatt.UUID, sp.Name, e.Message, e.StackTrace);
-
-                        // Make sure the object doesn't stick around and bail
-                        sp.RemoveAttachment(objatt);
-                        m_scene.DeleteSceneObject(objatt, false);
-                        return null;
+                        m_log.WarnFormat(
+                            "[ATTACHMENTS MODULE]: Could not retrieve item {0} for attaching to avatar {1} at point {2}",
+                            itemID, sp.Name, attachmentPt);
                     }
-                    
-                    if (tainted)
-                        objatt.HasGroupChanged = true;
 
                     if (doc != null)
                     {
@@ -430,47 +507,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                     // Do this last so that event listeners have access to all the effects of the attachment
                     m_scene.EventManager.TriggerOnAttach(objatt.LocalId, itemID, sp.UUID);
                 }
-                else
-                {
-                    m_log.WarnFormat(
-                        "[ATTACHMENTS MODULE]: Could not retrieve item {0} for attaching to avatar {1} at point {2}", 
-                        itemID, sp.Name, attachmentPt);
-                }
-                
-                return objatt;
             }
             
             return null;
         }
-        
-        /// <summary>
-        /// Update the user inventory to the attachment of an item
-        /// </summary>
-        /// <param name="att"></param>
-        /// <param name="sp"></param>
-        /// <param name="itemID"></param>
-        /// <param name="attachmentPoint"></param>
-        /// <returns></returns>
-        private UUID ShowAttachInUserInventory(
-            SceneObjectGroup att, IScenePresence sp, UUID itemID, uint attachmentPoint)
-        {
-//            m_log.DebugFormat(
-//                "[ATTACHMENTS MODULE]: Updating inventory of {0} to show attachment of {1} {2} (item ID {3}) at {4}",
-//                sp.Name, att.Name, att.LocalId, itemID, AttachmentPt);
-            
-            if (!att.IsDeleted)
-                attachmentPoint = att.AttachmentPoint;
-
-            InventoryItemBase item = new InventoryItemBase(itemID, sp.UUID);
-                if (m_scene.InventoryService != null)
-                    item = m_scene.InventoryService.GetItem(item);
-
-            bool changed = sp.Appearance.SetAttachment((int)attachmentPoint, itemID, item.AssetID);
-            if (changed && m_scene.AvatarFactory != null)
-                m_scene.AvatarFactory.QueueAppearanceSave(sp.UUID);
-            
-            return att.UUID;
-        }
 
         /// <summary>
         /// Update the user inventory to reflect an attachment
@@ -483,8 +523,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             IScenePresence sp, uint AttachmentPt, UUID itemID, SceneObjectGroup att)
         {
 //            m_log.DebugFormat(
-//                "[USER INVENTORY]: Updating attachment {0} for {1} at {2} using item ID {3}", 
-//                att.Name, remoteClient.Name, AttachmentPt, itemID);
+//                "[USER INVENTORY]: Updating attachment {0} for {1} at {2} using item ID {3}",
+//                att.Name, sp.Name, AttachmentPt, itemID);
             
             if (UUID.Zero == itemID)
             {
@@ -518,6 +558,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
         public void DetachObject(uint objectLocalID, IClientAPI remoteClient)
         {
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: DetachObject() for object {0} on {1}", objectLocalID, remoteClient.Name);
+
             SceneObjectGroup group = m_scene.GetGroupByPrim(objectLocalID);
             if (group != null)
             {
@@ -530,14 +573,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             ScenePresence presence;
             if (m_scene.TryGetScenePresence(remoteClient.AgentId, out presence))
             {
-                // Save avatar attachment information
-                m_log.Debug("[ATTACHMENTS MODULE]: Detaching from UserID: " + remoteClient.AgentId + ", ItemID: " + itemID);
-
-                bool changed = presence.Appearance.DetachAttachment(itemID);
-                if (changed && m_scene.AvatarFactory != null)
-                    m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
+                lock (presence.AttachmentsSyncLock)
+                {
+                    // Save avatar attachment information
+                    m_log.Debug("[ATTACHMENTS MODULE]: Detaching from UserID: " + remoteClient.AgentId + ", ItemID: " + itemID);
 
-                DetachSingleAttachmentToInv(itemID, presence);
+                    bool changed = presence.Appearance.DetachAttachment(itemID);
+                    if (changed && m_scene.AvatarFactory != null)
+                        m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
+    
+                    DetachSingleAttachmentToInv(itemID, presence);
+                }
             }
         }
 
@@ -545,7 +591,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         {
 //            m_log.DebugFormat(
 //                "[ATTACHMENTS MODULE]: DetachSingleAttachmentToGround() for {0}, object {1}",
-//                remoteClient.Name, sceneObjectID);
+//                remoteClient.Name, soLocalId);
 
             SceneObjectGroup so = m_scene.GetGroupByPrim(soLocalId);
 
@@ -564,24 +610,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             ScenePresence presence;
             if (m_scene.TryGetScenePresence(remoteClient.AgentId, out presence))
             {
-                if (!m_scene.Permissions.CanRezObject(
-                    so.PrimCount, remoteClient.AgentId, presence.AbsolutePosition))
-                    return;
+                lock (presence.AttachmentsSyncLock)
+                {
+                    if (!m_scene.Permissions.CanRezObject(
+                        so.PrimCount, remoteClient.AgentId, presence.AbsolutePosition))
+                        return;
 
-                bool changed = presence.Appearance.DetachAttachment(inventoryID);
-                if (changed && m_scene.AvatarFactory != null)
-                    m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
+                    bool changed = presence.Appearance.DetachAttachment(inventoryID);
+                    if (changed && m_scene.AvatarFactory != null)
+                        m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
 
-                presence.RemoveAttachment(so);
-                DetachSceneObjectToGround(so, presence);
+                    presence.RemoveAttachment(so);
+                    DetachSceneObjectToGround(so, presence);
 
-                List<UUID> uuids = new List<UUID>();
-                uuids.Add(inventoryID);
-                m_scene.InventoryService.DeleteItems(remoteClient.AgentId, uuids);
-                remoteClient.SendRemoveInventoryItem(inventoryID);
-            }
+                    List<UUID> uuids = new List<UUID>();
+                    uuids.Add(inventoryID);
+                    m_scene.InventoryService.DeleteItems(remoteClient.AgentId, uuids);
+                    remoteClient.SendRemoveInventoryItem(inventoryID);
+                }
 
-            m_scene.EventManager.TriggerOnAttach(so.LocalId, so.UUID, UUID.Zero);
+                m_scene.EventManager.TriggerOnAttach(so.LocalId, so.UUID, UUID.Zero);
+            }
         }
 
         /// <summary>
@@ -615,6 +664,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         // To LocalId or UUID, *THAT* is the question. How now Brown UUID??
         private void DetachSingleAttachmentToInv(UUID itemID, IScenePresence sp)
         {
+//            m_log.DebugFormat("[ATTACHMENTS MODULE]: Detaching item {0} to inventory for {1}", itemID, sp.Name);
+
             if (itemID == UUID.Zero) // If this happened, someone made a mistake....
                 return;
 
@@ -623,40 +674,29 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             EntityBase[] detachEntities = m_scene.GetEntities();
             SceneObjectGroup group;
 
-            foreach (EntityBase entity in detachEntities)
+            lock (sp.AttachmentsSyncLock)
             {
-                if (entity is SceneObjectGroup)
+                foreach (EntityBase entity in detachEntities)
                 {
-                    group = (SceneObjectGroup)entity;
-                    if (group.GetFromItemID() == itemID)
+                    if (entity is SceneObjectGroup)
                     {
-                        m_scene.EventManager.TriggerOnAttach(group.LocalId, itemID, UUID.Zero);
+                                                group = (SceneObjectGroup)entity;                        if (group.GetFromItemID() == itemID)                        {                        m_scene.EventManager.TriggerOnAttach(group.LocalId, itemID, UUID.Zero);
                         // CM / XMREngine!!!! Needed to conclude attach event
                         //SceneObjectSerializer.ToOriginalXmlFormat(group);
                         group.DetachToInventoryPrep();
                         m_log.Debug("[ATTACHMENTS MODULE]: Saving attachpoint: " + ((uint)group.GetAttachmentPoint()).ToString());
 
-                        // Prepare sog for storage
-                        group.AttachedAvatar = UUID.Zero;
-
-                        group.ForEachPart(
-                            delegate(SceneObjectPart part)
-                            {
-                                // If there are any scripts,
-                                // then always trigger a new object and state persistence in UpdateKnownItem()
-                                if (part.Inventory.ContainsScripts())
-                                    group.HasGroupChanged = true;
-                            }
-                        );
-
-                        group.RootPart.SetParentLocalId(0);
-                        group.IsAttachment = false;
-                        group.AbsolutePosition = group.RootPart.AttachedPos;
+                            // Prepare sog for storage
+                            group.AttachedAvatar = UUID.Zero;
+                            group.RootPart.SetParentLocalId(0);
+                            group.IsAttachment = false;
+                            group.AbsolutePosition = group.RootPart.AttachedPos;
 
-                        UpdateKnownItem(sp.ControllingClient, group, group.GetFromItemID(), group.OwnerID);
-                        m_scene.DeleteSceneObject(group, false);
+                            UpdateKnownItem(sp.ControllingClient, group);
+                            m_scene.DeleteSceneObject(group, false);
 
-                        return;
+                            return;
+                        }
                     }
                 }
             }
@@ -687,28 +727,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         /// </remarks>
         /// <param name="remoteClient"></param>
         /// <param name="grp"></param>
-        /// <param name="itemID"></param>
-        /// <param name="agentID"></param>
-        public void UpdateKnownItem(IClientAPI remoteClient, SceneObjectGroup grp, UUID itemID, UUID agentID)
+        private void UpdateKnownItem(IClientAPI remoteClient, SceneObjectGroup grp)
         {
-            if (grp != null)
+            if (grp.HasGroupChanged || grp.ContainsScripts())
             {
-                if (!grp.HasGroupChanged)
-                {
-                    m_log.DebugFormat(
-                        "[ATTACHMENTS MODULE]: Don't need to update asset for unchanged attachment {0}, attachpoint {1}",
-                        grp.UUID, grp.AttachmentPoint);
-
-                    return;
-                }
-
                 m_log.DebugFormat(
                     "[ATTACHMENTS MODULE]: Updating asset for attachment {0}, attachpoint {1}",
                     grp.UUID, grp.AttachmentPoint);
 
                 string sceneObjectXml = SceneObjectSerializer.ToOriginalXmlFormat(grp);
 
-                InventoryItemBase item = new InventoryItemBase(itemID, remoteClient.AgentId);
+                InventoryItemBase item = new InventoryItemBase(grp.GetFromItemID(), remoteClient.AgentId);
                 item = m_scene.InventoryService.GetItem(item);
 
                 if (item != null)
@@ -734,6 +763,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                         remoteClient.SendInventoryItemCreateUpdate(item, 0);
                 }
             }
+            else
+            {
+                m_log.DebugFormat(
+                    "[ATTACHMENTS MODULE]: Don't need to update asset for unchanged attachment {0}, attachpoint {1}",
+                    grp.UUID, grp.AttachmentPoint);
+            }
         } 
         
         /// <summary>
@@ -751,7 +786,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         private void AttachToAgent(
             IScenePresence avatar, SceneObjectGroup so, uint attachmentpoint, Vector3 attachOffset, bool silent)
         {
-//            m_log.DebugFormat("[ATTACHMENTS MODULE]: Adding attachment {0} to avatar {1} in pt {2} pos {3} {4}",
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: Adding attachment {0} to avatar {1} in pt {2} pos {3} {4}",
 //                so.Name, avatar.Name, attachmentpoint, attachOffset, so.RootPart.AttachedPos);
                               
             so.DetachFromBackup();
@@ -788,14 +824,16 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         }
 
         /// <summary>
-        /// Add a scene object that was previously free in the scene as an attachment to an avatar.
+        /// Add a scene object as a new attachment in the user inventory.
         /// </summary>
         /// <param name="remoteClient"></param>
         /// <param name="grp"></param>
         /// <returns>The user inventory item created that holds the attachment.</returns>
-        private InventoryItemBase AddSceneObjectAsAttachment(IClientAPI remoteClient, SceneObjectGroup grp)
+        private InventoryItemBase AddSceneObjectAsNewAttachmentInInv(IClientAPI remoteClient, SceneObjectGroup grp)
         {
-//            m_log.DebugFormat("[SCENE]: Called AddSceneObjectAsAttachment for object {0} {1} for {2} {3} {4}", grp.Name, grp.LocalId, remoteClient.Name, remoteClient.AgentId, AgentId);
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: Called AddSceneObjectAsAttachment for object {0} {1} for {2}",
+//                grp.Name, grp.LocalId, remoteClient.Name);
 
             Vector3 inventoryStoredPosition = new Vector3
                    (((grp.AbsolutePosition.X > (int)Constants.RegionSize)
@@ -879,4 +917,4 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             return item;
         }
     }
-}
+}
\ No newline at end of file
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index 363e258..ff3358f 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -58,12 +58,16 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
         private AttachmentsModule m_attMod;
         private ScenePresence m_presence;
 
-        [SetUp]
-        public void Init()
+        [TestFixtureSetUp]
+        public void FixtureInit()
         {
             // Don't allow tests to be bamboozled by asynchronous events.  Execute everything on the same thread.
             Util.FireAndForgetMethod = FireAndForgetMethod.None;
+        }
 
+        [SetUp]
+        public void Init()
+        {
             IConfigSource config = new IniConfigSource();
             config.AddConfig("Modules");
             config.Configs["Modules"].Set("InventoryAccessModule", "BasicInventoryAccessModule");
@@ -73,7 +77,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
             SceneHelpers.SetupSceneModules(scene, config, m_attMod, new BasicInventoryAccessModule());
         }
 
-        [TearDown]
+        [TestFixtureTearDown]
         public void TearDown()
         {
             // We must set this back afterwards, otherwise later tests will fail since they're expecting multiple
@@ -150,6 +154,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
             Assert.That(attSo.IsTemporary, Is.False);
 
             // Check appearance status
+            Assert.That(m_presence.Appearance.GetAttachments().Count, Is.EqualTo(1));
             Assert.That(m_presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest));
         }
 
@@ -215,6 +220,38 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
             Assert.That(m_presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo(0));
         }
 
+        /// <summary>
+        /// Test that attachments don't hang about in the scene when the agent is closed
+        /// </summary>
+        [Test]
+        public void TestRemoveAttachmentsOnAvatarExit()
+        {
+            TestHelpers.InMethod();
+//            log4net.Config.XmlConfigurator.Configure();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+            UUID attItemId = TestHelpers.ParseTail(0x2);
+            UUID attAssetId = TestHelpers.ParseTail(0x3);
+            string attName = "att";
+
+            UserAccountHelpers.CreateUserWithInventory(scene, userId);
+            InventoryItemBase attItem
+                = UserInventoryHelpers.CreateInventoryItem(
+                    scene, attName, attItemId, attAssetId, userId, InventoryType.Object);
+
+            AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+            acd.Appearance = new AvatarAppearance();
+            acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID);
+            ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd);
+
+            SceneObjectGroup rezzedAtt = presence.GetAttachments()[0];
+
+            scene.IncomingCloseAgent(presence.UUID);
+
+            // Check that we can't retrieve this attachment from the scene.
+            Assert.That(scene.GetSceneObjectGroup(rezzedAtt.UUID), Is.Null);
+        }
+
         [Test]
         public void TestRezAttachmentsOnAvatarEntrance()
         {
@@ -246,6 +283,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
             Assert.That(attSo.IsAttachment);
             Assert.That(attSo.UsesPhysics, Is.False);
             Assert.That(attSo.IsTemporary, Is.False);
+
+            // Check appearance status
+            List<AvatarAttachment> retreivedAttachments = presence.Appearance.GetAttachments();
+            Assert.That(retreivedAttachments.Count, Is.EqualTo(1));
+            Assert.That(retreivedAttachments[0].AttachPoint, Is.EqualTo((int)AttachmentPoint.Chest));
+            Assert.That(retreivedAttachments[0].ItemID, Is.EqualTo(attItemId));
+            Assert.That(retreivedAttachments[0].AssetID, Is.EqualTo(attAssetId));
+            Assert.That(presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest));
         }
 
         // I'm commenting this test because scene setup NEEDS InventoryService to