In AttachmentsModule.DetachSingleAttachmentToInvInternal(), remove attachment before changing properties for correct inventory serialization.

Serialization of attachments requires IsAttachment = false so that correct positions are serialized instead of avatar position.
However, doing this when a hud is still attached allows race conditions with update threads, resulting in hud artifacts on other viewers.
This change sets SOG.IsDeleted before serialization changes take place (IsDeleted itself is not a serialized property).
LLClientView then screens out any deleted SOGs before sending updates to viewers.

3 files changed, 42 insertions, 2 deletions

diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
index 4cb7a3a..8874585 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
@@ -3808,6 +3808,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                                 && part.ParentGroup.HasPrivateAttachmentPoint
                                 && part.ParentGroup.AttachedAvatar != AgentId)
                                 continue;
+
+                            // If the part has since been deleted, then drop the update.  In the case of attachments,
+                            // this is to avoid spurious updates to other viewers since post-processing of attachments
+                            // has to change the IsAttachment flag for various reasons (which will end up in a pass
+                            // of the test above).
+                            //
+                            // Actual deletions (kills) happen in another method.
+                            if (part.ParentGroup.IsDeleted)
+                                continue;
                         }
 
                         objectUpdateBlocks.Value.Add(updateBlock);
@@ -3815,7 +3824,20 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                     }
                     else if (!canUseImproved)
                     {
-                        compressedUpdateBlocks.Value.Add(CreateCompressedUpdateBlock((SceneObjectPart)update.Entity, updateFlags));
+                        SceneObjectPart part = (SceneObjectPart)update.Entity;
+                        ObjectUpdateCompressedPacket.ObjectDataBlock compressedBlock
+                            = CreateCompressedUpdateBlock(part, updateFlags);
+
+                        // If the part has since been deleted, then drop the update.  In the case of attachments,
+                        // this is to avoid spurious updates to other viewers since post-processing of attachments
+                        // has to change the IsAttachment flag for various reasons (which will end up in a pass
+                        // of the test above).
+                        //
+                        // Actual deletions (kills) happen in another method.
+                        if (part.ParentGroup.IsDeleted)
+                            continue;
+
+                        compressedUpdateBlocks.Value.Add(compressedBlock);
                         compressedUpdates.Value.Add(update);
                     }
                     else
@@ -3842,6 +3864,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                                     && part.ParentGroup.HasPrivateAttachmentPoint
                                     && part.ParentGroup.AttachedAvatar != AgentId)
                                     continue;
+
+                                // If the part has since been deleted, then drop the update.  In the case of attachments,
+                                // this is to avoid spurious updates to other viewers since post-processing of attachments
+                                // has to change the IsAttachment flag for various reasons (which will end up in a pass
+                                // of the test above).
+                                //
+                                // Actual deletions (kills) happen in another method.
+                                if (part.ParentGroup.IsDeleted)
+                                    continue;
                             }
 
                             terseUpdateBlocks.Value.Add(terseUpdateBlock);
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index a2b95eb..99e0153 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -628,6 +628,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                         {
                             m_scene.EventManager.TriggerOnAttach(group.LocalId, itemID, UUID.Zero);
                             sp.RemoveAttachment(group);
+                            m_scene.DeleteSceneObject(group, false);
 
                             // Prepare sog for storage
                             group.AttachedAvatar = UUID.Zero;
@@ -636,7 +637,6 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                             group.AbsolutePosition = group.RootPart.AttachedPos;
 
                             UpdateKnownItem(sp, group, true);
-                            m_scene.DeleteSceneObject(group, false);
 
                             return;
                         }
diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectBasicTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectBasicTests.cs
index 453e077..81add43 100644
--- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectBasicTests.cs
+++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectBasicTests.cs
@@ -187,8 +187,13 @@ namespace OpenSim.Region.Framework.Scenes.Tests
 
             TestScene scene = new SceneHelpers().SetupScene();
             SceneObjectPart part = SceneHelpers.AddSceneObject(scene);
+
+            Assert.That(part.ParentGroup.IsDeleted, Is.False);
+
             scene.DeleteSceneObject(part.ParentGroup, false);
 
+            Assert.That(part.ParentGroup.IsDeleted, Is.True);
+
             SceneObjectPart retrievedPart = scene.GetSceneObjectPart(part.LocalId);
             Assert.That(retrievedPart, Is.Null);
         }
@@ -219,8 +224,12 @@ namespace OpenSim.Region.Framework.Scenes.Tests
 
             Assert.That(retrievedPart, Is.Not.Null);
 
+            Assert.That(part.ParentGroup.IsDeleted, Is.False);
+
             sogd.InventoryDeQueueAndDelete();
 
+            Assert.That(part.ParentGroup.IsDeleted, Is.True);            
+
             SceneObjectPart retrievedPart2 = scene.GetSceneObjectPart(part.LocalId);
             Assert.That(retrievedPart2, Is.Null);
         }