From 62b3bdf0fc7a64dd9b845eb27fa8e1a2a1866c2b Mon Sep 17 00:00:00 2001 From: Oren Hurvitz Date: Thu, 24 Oct 2013 11:18:15 +0300 Subject: [PATCH] When linking two groups, and then deleting the combined group: delete *all* of the combined group's prims, including those that came from the second subgroup This fixes http://opensimulator.org/mantis/view.php?id=6175 --- OpenSim/Region/Framework/Scenes/Scene.cs | 12 +++--- .../Framework/Scenes/SceneObjectGroup.cs | 38 +++++++++++++++---- .../Scenes/Tests/SceneObjectLinkingTests.cs | 8 ++-- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index d16b73bbc0..08a230112c 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -1757,6 +1757,7 @@ namespace OpenSim.Region.Framework.Scenes { if (group != null) { + group.HasGroupChanged = true; group.ProcessBackup(SimulationDataService, true); } } @@ -2345,13 +2346,12 @@ namespace OpenSim.Region.Framework.Scenes { if (!softDelete) { - // Force a database update so that the scene object group ID is accurate. It's possible that the - // group has recently been delinked from another group but that this change has not been persisted - // to the DB. + // If the group contains prims whose SceneGroupID is incorrect then force a + // database update, because RemoveObject() works by searching on the SceneGroupID. // This is an expensive thing to do so only do it if absolutely necessary. - if (so.HasGroupChangedDueToDelink) - ForceSceneObjectBackup(so); - + if (so.GroupContainsForeignPrims) + ForceSceneObjectBackup(so); + so.DetachFromBackup(); SimulationDataService.RemoveObject(so.UUID, RegionInfo.RegionID); } diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index 6f2617602e..a78a0cb027 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -150,12 +150,27 @@ namespace OpenSim.Region.Framework.Scenes get { return m_hasGroupChanged; } } + + private bool m_groupContainsForeignPrims = false; /// - /// Has the group changed due to an unlink operation? We record this in order to optimize deletion, since - /// an unlinked group currently has to be persisted to the database before we can perform an unlink operation. + /// Whether the group contains prims that came from a different group. This happens when + /// linking or delinking groups. The implication is that until the group is persisted, + /// the prims in the database still use the old SceneGroupID. That's a problem if the group + /// is deleted, because we delete groups by searching for prims by their SceneGroupID. /// - public bool HasGroupChangedDueToDelink { get; private set; } + public bool GroupContainsForeignPrims + { + private set + { + m_groupContainsForeignPrims = value; + if (m_groupContainsForeignPrims) + HasGroupChanged = true; + } + + get { return m_groupContainsForeignPrims; } + } + private bool isTimeToPersist() { @@ -1624,7 +1639,7 @@ namespace OpenSim.Region.Framework.Scenes backup_group.RootPart.AngularVelocity = RootPart.AngularVelocity; backup_group.RootPart.ParticleSystem = RootPart.ParticleSystem; HasGroupChanged = false; - HasGroupChangedDueToDelink = false; + GroupContainsForeignPrims = false; m_scene.EventManager.TriggerOnSceneObjectPreSave(backup_group, this); datastore.StoreObject(backup_group, m_scene.RegionInfo.RegionID); @@ -2388,6 +2403,8 @@ namespace OpenSim.Region.Framework.Scenes // If linking prims with different permissions, fix them AdjustChildPrimPermissions(); + GroupContainsForeignPrims = true; + AttachToBackup(); // Here's the deal, this is ABSOLUTELY CRITICAL so the physics scene gets the update about the @@ -2531,9 +2548,16 @@ namespace OpenSim.Region.Framework.Scenes linkPart.Rezzed = RootPart.Rezzed; - // When we delete a group, we currently have to force persist to the database if the object id has changed - // (since delete works by deleting all rows which have a given object id) - objectGroup.HasGroupChangedDueToDelink = true; + // We must persist the delinked group to the database immediately, for safety. The problem + // is that although in memory the new group has a new SceneGroupID, in the database it + // still has the parent group's SceneGroupID (until the next backup). This means that if the + // parent group is deleted then the delinked group will also be deleted from the database. + // This problem will disappear if the region remains alive long enough for another backup, + // since at that time the delinked group's new SceneGroupID will be written to the database. + // But if the region crashes before that then the prims will be permanently gone, and this must + // not happen. (We can't use a just-in-time trick like GroupContainsForeignPrims in this case + // because the delinked group doesn't know when the source group is deleted.) + m_scene.ForceSceneObjectBackup(objectGroup); return objectGroup; } diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index 9378e2010d..fa8277cc7b 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -143,7 +143,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests Assert.That(grp1.Parts.Length, Is.EqualTo(1), "Group 1 still contained part2 after delink."); Assert.That(part2.AbsolutePosition == Vector3.Zero, "The absolute position should be zero"); - Assert.That(grp3.HasGroupChangedDueToDelink, Is.True); + Assert.That(grp3.GroupContainsForeignPrims, Is.True); } [Test] @@ -349,10 +349,10 @@ namespace OpenSim.Region.Framework.Scenes.Tests // These changes should occur immediately without waiting for a backup pass SceneObjectGroup groupToDelete = sog.DelinkFromGroup(linkPart, false); - - Assert.That(groupToDelete.HasGroupChangedDueToDelink, Is.True); + + Assert.That(groupToDelete.GroupContainsForeignPrims, Is.True); scene.DeleteSceneObject(groupToDelete, false); - Assert.That(groupToDelete.HasGroupChangedDueToDelink, Is.False); + Assert.That(groupToDelete.GroupContainsForeignPrims, Is.False); List storedObjects = scene.SimulationDataService.LoadObjects(scene.RegionInfo.RegionID);