From 36cb6208b5f1673e707a31bd9b0880802f721719 Mon Sep 17 00:00:00 2001 From: Melanie Thielker Date: Mon, 6 Sep 2010 21:59:52 +0200 Subject: Fix yet another cause of "Ghost attachments" --- .../CoreModules/Avatar/Attachments/AttachmentsModule.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'OpenSim') diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs index 6555b54..dd7d831 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs @@ -264,8 +264,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments if (AttachmentPt != 0 && AttachmentPt != objatt.GetAttachmentPoint()) tainted = true; - AttachObject(remoteClient, objatt, AttachmentPt, false); - //objatt.ScheduleGroupForFullUpdate(); + // This will throw if the attachment fails + try + { + AttachObject(remoteClient, objatt, AttachmentPt, false); + } + catch + { + // Make sure the object doesn't stick around and bail + m_scene.DeleteSceneObject(objatt, false); + return null; + } if (tainted) objatt.HasGroupChanged = true; @@ -594,4 +603,4 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments so.HasGroupChanged = false; } } -} \ No newline at end of file +} -- cgit v1.1 From 11f4a65f42dea66091cb08423479fa6ae46c98aa Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 7 Sep 2010 00:34:06 +0100 Subject: 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 + OpenSim/Region/Framework/Scenes/Scene.Inventory.cs | 7 +- OpenSim/Region/Framework/Scenes/Scene.cs | 25 ++++--- .../Region/Framework/Scenes/SceneObjectGroup.cs | 42 +++++++----- .../Scenes/Tests/SceneObjectLinkingTests.cs | 4 +- OpenSim/Tests/Common/Mock/MockRegionDataPlugin.cs | 80 ++++++++++++++++++---- .../Tests/Common/Mock/TestInventoryDataPlugin.cs | 2 +- .../Tests/Common/Setup/UserInventoryTestUtils.cs | 2 +- 10 files changed, 129 insertions(+), 57 deletions(-) (limited to 'OpenSim') diff --git a/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs b/OpenSim/Data/MySQL/MySQLLegacyRegionData.cs index 30253c3..a39e68d 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 241cac0..916148b 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 e183f9d..6fd38e5 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 4e871d9..c49386a 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 183310d..82e7d76 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 b8b3db5..09e3e0e 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 93409fa..62761fb 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 1c139c5..2a055cc 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(); + + // 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(m_sceneObjects.Values); + 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 b70b47d..b47ad5d 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 c57363a..915af7e 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; } -- cgit v1.1 From 84ad9e4d4eb84c31d22c4cf442110d396ff8fc2c Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 7 Sep 2010 02:05:44 +0100 Subject: minor: comment out some excessive test logging --- OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'OpenSim') diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs index 62761fb..3c03ba0 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectLinkingTests.cs @@ -307,7 +307,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests public void TestDelinkPersistence() { TestHelper.InMethod(); - log4net.Config.XmlConfigurator.Configure(); +// log4net.Config.XmlConfigurator.Configure(); TestScene scene = SceneSetupHelpers.SetupScene(); -- cgit v1.1