From 11f4a65f42dea66091cb08423479fa6ae46c98aa Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 7 Sep 2010 00:34:06 +0100 Subject: [PATCH] Fix deletion persistence when freshly delinked prims are removed Previously, Scene.Inventory.DeRezObjects() forced the persistence of prims before deletion. This is necessary so that freshly delinked prims can be deleted (otherwise they remain as parts of their old group and reappear on server restart). However, DeRezObjects() deleted to user inventory, which is required by llDie() or direct region module unlink and deletion. Therefore, forced persistence has been pushed down into Scene.UnlinkSceneObject() to be more general, this is still on the DeRezObjects() path. Uncommented TestDelinkPersistence() since this now passes. Tests required considerable elaboration of MockRegionDataPlugin to reflect underlying storing of parts. --- OpenSim/Data/MySQL/MySQLLegacyRegionData.cs | 2 + .../Scenes/AsyncSceneObjectGroupDeleter.cs | 21 +++-- OpenSim/Region/Framework/Scenes/EntityBase.cs | 1 + .../Framework/Scenes/Scene.Inventory.cs | 7 +- OpenSim/Region/Framework/Scenes/Scene.cs | 25 +++--- .../Framework/Scenes/SceneObjectGroup.cs | 42 ++++++---- .../Scenes/Tests/SceneObjectLinkingTests.cs | 4 +- .../Tests/Common/Mock/MockRegionDataPlugin.cs | 80 ++++++++++++++++--- .../Common/Mock/TestInventoryDataPlugin.cs | 2 +- .../Common/Setup/UserInventoryTestUtils.cs | 2 +- 10 files changed, 129 insertions(+), 57 deletions(-) diff --git a/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs b/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs index 30253c3158..a39e68d3ad 100644 --- a/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs +++ b/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs @@ -247,6 +247,8 @@ namespace OpenSim.Data.MySQL public void RemoveObject(UUID obj, UUID regionUUID) { +// m_log.DebugFormat("[REGION DB]: Deleting scene object {0} from {1} in database", obj, regionUUID); + List uuids = new List(); // Formerly, this used to check the region UUID. diff --git a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs b/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs index 241cac0f11..916148bc6c 100644 --- a/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs +++ b/OpenSim/Region/Framework/Scenes/AsyncSceneObjectGroupDeleter.cs @@ -111,11 +111,11 @@ namespace OpenSim.Region.Framework.Scenes private void InventoryRunDeleteTimer(object sender, ElapsedEventArgs e) { - m_log.Debug("[SCENE]: Starting send to inventory loop"); + m_log.Debug("[ASYNC DELETER]: Starting send to inventory loop"); while (InventoryDeQueueAndDelete()) { - //m_log.Debug("[SCENE]: Sent item successfully to inventory, continuing..."); + //m_log.Debug("[ASYNC DELETER]: Sent item successfully to inventory, continuing..."); } } @@ -137,7 +137,7 @@ namespace OpenSim.Region.Framework.Scenes x = m_inventoryDeletes.Dequeue(); m_log.DebugFormat( - "[SCENE]: Sending object to user's inventory, {0} item(s) remaining.", left); + "[ASYNC DELETER]: Sending object to user's inventory, {0} item(s) remaining.", left); try { @@ -152,7 +152,8 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception e) { - m_log.DebugFormat("Exception background sending object: " + e); + m_log.ErrorFormat( + "[ASYNC DELETER]: Exception background sending object: {0}{1}", e.Message, e.StackTrace); } return true; @@ -164,12 +165,16 @@ namespace OpenSim.Region.Framework.Scenes // We can't put the object group details in here since the root part may have disappeared (which is where these sit). // FIXME: This needs to be fixed. m_log.ErrorFormat( - "[SCENE]: Queued sending of scene object to agent {0} {1} failed: {2}", - (x != null ? x.remoteClient.Name : "unavailable"), (x != null ? x.remoteClient.AgentId.ToString() : "unavailable"), e.ToString()); + "[ASYNC DELETER]: Queued sending of scene object to agent {0} {1} failed: {2} {3}", + (x != null ? x.remoteClient.Name : "unavailable"), + (x != null ? x.remoteClient.AgentId.ToString() : "unavailable"), + e.Message, + e.StackTrace); } - m_log.Debug("[SCENE]: No objects left in inventory send queue."); + m_log.Debug("[ASYNC DELETER]: No objects left in inventory send queue."); + return false; } } -} +} \ No newline at end of file diff --git a/OpenSim/Region/Framework/Scenes/EntityBase.cs b/OpenSim/Region/Framework/Scenes/EntityBase.cs index e183f9d1d7..6fd38e56b1 100644 --- a/OpenSim/Region/Framework/Scenes/EntityBase.cs +++ b/OpenSim/Region/Framework/Scenes/EntityBase.cs @@ -69,6 +69,7 @@ namespace OpenSim.Region.Framework.Scenes public bool IsDeleted { get { return m_isDeleted; } + set { m_isDeleted = value; } } protected bool m_isDeleted; diff --git a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs index 4e871d9edf..c49386a85b 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.Inventory.cs @@ -1720,7 +1720,7 @@ namespace OpenSim.Region.Framework.Scenes public virtual void DeRezObject(IClientAPI remoteClient, uint localID, UUID groupID, DeRezAction action, UUID destinationID) { - DeRezObjects(remoteClient, new List() { localID} , groupID, action, destinationID); + DeRezObjects(remoteClient, new List() { localID }, groupID, action, destinationID); } public virtual void DeRezObjects(IClientAPI remoteClient, List localIDs, @@ -1757,11 +1757,6 @@ namespace OpenSim.Region.Framework.Scenes deleteIDs.Add(localID); deleteGroups.Add(grp); - // Force a database backup/update on this SceneObjectGroup - // So that we know the database is upto date, - // for when deleting the object from it - ForceSceneObjectBackup(grp); - if (remoteClient == null) { // Autoreturn has a null client. Nothing else does. So diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs index 183310d7f8..82e7d763df 100644 --- a/OpenSim/Region/Framework/Scenes/Scene.cs +++ b/OpenSim/Region/Framework/Scenes/Scene.cs @@ -2092,7 +2092,7 @@ namespace OpenSim.Region.Framework.Scenes // rootPart.PhysActor = null; // } - if (UnlinkSceneObject(group.UUID, false)) + if (UnlinkSceneObject(group, false)) { EventManager.TriggerObjectBeingRemovedFromScene(group); EventManager.TriggerParcelPrimCountTainted(); @@ -2107,18 +2107,25 @@ namespace OpenSim.Region.Framework.Scenes /// Unlink the given object from the scene. Unlike delete, this just removes the record of the object - the /// object itself is not destroyed. /// - /// Id of object. + /// The scene object. + /// If true, only deletes from scene, but keeps the object in the database. /// true if the object was in the scene, false if it was not - /// If true, only deletes from scene, but keeps object in database. - public bool UnlinkSceneObject(UUID uuid, bool softDelete) + public bool UnlinkSceneObject(SceneObjectGroup so, bool softDelete) { - if (m_sceneGraph.DeleteSceneObject(uuid, softDelete)) + if (m_sceneGraph.DeleteSceneObject(so.UUID, softDelete)) { if (!softDelete) { - m_storageManager.DataStore.RemoveObject(uuid, - m_regInfo.RegionID); + // 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. + ForceSceneObjectBackup(so); + so.DetachFromBackup(); + m_storageManager.DataStore.RemoveObject(so.UUID, m_regInfo.RegionID); } + + // We need to keep track of this state in case this group is still queued for further backup. + so.IsDeleted = true; return true; } @@ -2149,7 +2156,7 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception) { - m_log.Warn("[DATABASE]: exception when trying to remove the prim that crossed the border."); + m_log.Warn("[SCENE]: exception when trying to remove the prim that crossed the border."); } return; } @@ -2166,7 +2173,7 @@ namespace OpenSim.Region.Framework.Scenes } catch (Exception) { - m_log.Warn("[DATABASE]: exception when trying to return the prim that crossed the border."); + m_log.Warn("[SCENE]: exception when trying to return the prim that crossed the border."); } return; } diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index b8b3db5486..09e3e0eb10 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -1226,16 +1226,16 @@ namespace OpenSim.Region.Framework.Scenes } /// - /// Delete this group from its scene and tell all the scene presences about that deletion. + /// Delete this group from its scene. /// - /// Broadcast deletions to all clients. + /// + /// This only handles the in-world consequences of deletion (e.g. any avatars sitting on it are forcibly stood + /// up and all avatars receive notification of its removal. Removal of the scene object from database backup + /// must be handled by the caller. + /// + /// If true then deletion is not broadcast to clients public void DeleteGroup(bool silent) { - // We need to keep track of this state in case this group is still queued for backup. - m_isDeleted = true; - - DetachFromBackup(); - lock (m_parts) { foreach (SceneObjectPart part in m_parts.Values) @@ -1381,10 +1381,18 @@ namespace OpenSim.Region.Framework.Scenes public virtual void ProcessBackup(IRegionDataStore datastore, bool forcedBackup) { if (!m_isBackedUp) + { +// m_log.DebugFormat( +// "[WATER WARS]: Ignoring backup of {0} {1} since object is not marked to be backed up", Name, UUID); return; + } if (IsDeleted || UUID == UUID.Zero) + { +// m_log.DebugFormat( +// "[WATER WARS]: Ignoring backup of {0} {1} since object is marked as already deleted", Name, UUID); return; + } // Since this is the top of the section of call stack for backing up a particular scene object, don't let // any exception propogate upwards. @@ -1420,7 +1428,7 @@ namespace OpenSim.Region.Framework.Scenes if (HasGroupChanged) { // don't backup while it's selected or you're asking for changes mid stream. - if ((isTimeToPersist()) || (forcedBackup)) + if (isTimeToPersist() || forcedBackup) { m_log.DebugFormat( "[SCENE]: Storing {0}, {1} in {2}", @@ -1443,19 +1451,19 @@ namespace OpenSim.Region.Framework.Scenes backup_group = null; } - // else - // { - // m_log.DebugFormat( - // "[SCENE]: Did not update persistence of object {0} {1}, selected = {2}", - // Name, UUID, IsSelected); - // } +// else +// { +// m_log.DebugFormat( +// "[SCENE]: Did not update persistence of object {0} {1}, selected = {2}", +// Name, UUID, IsSelected); +// } } } catch (Exception e) { m_log.ErrorFormat( - "[SCENE]: Storing of {0}, {1} in {2} failed with exception {3}\n\t{4}", - Name, UUID, m_scene.RegionInfo.RegionName, e, e.StackTrace); + "[SCENE]: Storing of {0}, {1} in {2} failed with exception {3}{4}", + Name, UUID, m_scene.RegionInfo.RegionName, e.Message, e.StackTrace); } } @@ -2245,7 +2253,7 @@ namespace OpenSim.Region.Framework.Scenes } } - m_scene.UnlinkSceneObject(objectGroup.UUID, true); + m_scene.UnlinkSceneObject(objectGroup, true); objectGroup.m_isDeleted = true; lock (objectGroup.m_parts) diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index 93409fa263..62761fb610 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -303,11 +303,11 @@ namespace OpenSim.Region.Framework.Scenes.Tests /// /// Test that a delink of a previously linked object is correctly persisted to the database /// - //[Test] + [Test] public void TestDelinkPersistence() { TestHelper.InMethod(); - //log4net.Config.XmlConfigurator.Configure(); + log4net.Config.XmlConfigurator.Configure(); TestScene scene = SceneSetupHelpers.SetupScene(); diff --git a/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs index 1c139c5250..2a055cc3c1 100644 --- a/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs +++ b/OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs @@ -44,7 +44,7 @@ namespace OpenSim.Data.Null private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); protected Dictionary m_regionSettings = new Dictionary(); - protected Dictionary m_sceneObjects = new Dictionary(); + protected Dictionary m_sceneObjectParts = new Dictionary(); protected Dictionary> m_primItems = new Dictionary>(); protected Dictionary m_terrains = new Dictionary(); @@ -85,18 +85,33 @@ namespace OpenSim.Data.Null public void StoreObject(SceneObjectGroup obj, UUID regionUUID) { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Storing object {0} {1} in {2}", obj.Name, obj.UUID, regionUUID); - m_sceneObjects[obj.UUID] = obj; + // We can't simply store groups here because on delinking, OpenSim will not update the original group + // directly. Rather, the newly delinked parts will be updated to be in their own scene object group + // Therefore, we need to store parts rather than groups. + foreach (SceneObjectPart prim in obj.Children.Values) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Storing part {0} {1} in object {2} {3} in region {4}", + prim.Name, prim.UUID, obj.Name, obj.UUID, regionUUID); + + m_sceneObjectParts[prim.UUID] = prim; + } } public void RemoveObject(UUID obj, UUID regionUUID) - { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Removing object {0} from {1}", obj, regionUUID); - - if (m_sceneObjects.ContainsKey(obj)) - m_sceneObjects.Remove(obj); + { + // All parts belonging to the object with the uuid are removed. + List parts = new List(m_sceneObjectParts.Values); + foreach (SceneObjectPart part in parts) + { + if (part.ParentGroup.UUID == obj) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Removing part {0} {1} as part of object {2} from {3}", + part.Name, part.UUID, obj, regionUUID); + m_sceneObjectParts.Remove(part.UUID); + } + } } // see IRegionDatastore @@ -107,10 +122,49 @@ namespace OpenSim.Data.Null public List LoadObjects(UUID regionUUID) { - m_log.DebugFormat( - "[MOCK REGION DATA PLUGIN]: Loading objects from {0}", regionUUID); + Dictionary objects = new Dictionary(); - return new List(m_sceneObjects.Values); + // Create all of the SOGs from the root prims first + foreach (SceneObjectPart prim in m_sceneObjectParts.Values) + { + if (prim.IsRoot) + { + m_log.DebugFormat( + "[MOCK REGION DATA PLUGIN]: Loading root part {0} {1} in {2}", prim.Name, prim.UUID, regionUUID); + objects[prim.UUID] = new SceneObjectGroup(prim); + } + } + + // Add all of the children objects to the SOGs + foreach (SceneObjectPart prim in m_sceneObjectParts.Values) + { + SceneObjectGroup sog; + if (prim.UUID != prim.ParentUUID) + { + if (objects.TryGetValue(prim.ParentUUID, out sog)) + { + int originalLinkNum = prim.LinkNum; + + sog.AddPart(prim); + + // SceneObjectGroup.AddPart() tries to be smart and automatically set the LinkNum. + // We override that here + if (originalLinkNum != 0) + prim.LinkNum = originalLinkNum; + } + else + { + m_log.WarnFormat( + "[MOCK REGION DATA PLUGIN]: Database contains an orphan child prim {0} {1} in region {2} pointing to missing parent {3}. This prim will not be loaded.", + prim.Name, prim.UUID, regionUUID, prim.ParentUUID); + } + } + } + + // TODO: Load items. This is assymetric - we store items as a separate method but don't retrieve them that + // way! + + return new List(objects.Values); } public void StoreTerrain(double[,] ter, UUID regionID) diff --git a/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs b/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs index b70b47df54..b47ad5dcb1 100644 --- a/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs +++ b/OpenSim/Tests/Common/Mock/TestInventoryDataPlugin.cs @@ -42,7 +42,7 @@ namespace OpenSim.Tests.Common.Mock /// public class TestInventoryDataPlugin : IInventoryDataPlugin { - private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); +// private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); /// /// Inventory folders diff --git a/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs b/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs index c57363aa94..915af7e58d 100644 --- a/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs +++ b/OpenSim/Tests/Common/Setup/UserInventoryTestUtils.cs @@ -52,7 +52,7 @@ namespace OpenSim.Tests.Common InventoryFolderBase objsFolder = scene.InventoryService.GetFolderForType(userId, AssetType.Object); item.Folder = objsFolder.ID; - scene.AddInventoryItem(userId, item); + scene.AddInventoryItem(item); return item; }