Get attachment script state before taking sp.AttachmentsSyncLock() to avoid race conditions between closing agents and scripts that may be doing attachment manipulation.
This is in an effort to resolve http://opensimulator.org/mantis/view.php?id=6557user_profiles
Justin Clark-Casey (justincc)
parent
fa9f4ef1ba
commit
ccd6f443e1
|
|
@ -241,12 +241,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
|
||||
// m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
|
||||
|
||||
List<SceneObjectGroup> attachments = sp.GetAttachments();
|
||||
|
||||
if (attachments.Count <= 0)
|
||||
return;
|
||||
|
||||
Dictionary<SceneObjectGroup, string> scriptStates = new Dictionary<SceneObjectGroup, string>();
|
||||
|
||||
foreach (SceneObjectGroup so in attachments)
|
||||
{
|
||||
// Scripts MUST be snapshotted before the object is
|
||||
// removed from the scene because doing otherwise will
|
||||
// clobber the run flag
|
||||
// This must be done outside the sp.AttachmentSyncLock so that there is no risk of a deadlock from
|
||||
// scripts performing attachment operations at the same time. Getting object states stops the scripts.
|
||||
scriptStates[so] = PrepareScriptInstanceForSave(so, false);
|
||||
}
|
||||
|
||||
lock (sp.AttachmentsSyncLock)
|
||||
{
|
||||
foreach (SceneObjectGroup so in sp.GetAttachments())
|
||||
{
|
||||
UpdateDetachedObject(sp, so);
|
||||
}
|
||||
foreach (SceneObjectGroup so in attachments)
|
||||
UpdateDetachedObject(sp, so, scriptStates[so]);
|
||||
|
||||
sp.ClearAttachments();
|
||||
}
|
||||
|
|
@ -285,30 +300,38 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
|
||||
private bool AttachObjectInternal(IScenePresence sp, SceneObjectGroup group, uint attachmentPt, bool silent, bool temp)
|
||||
{
|
||||
lock (sp.AttachmentsSyncLock)
|
||||
{
|
||||
// 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 (group.GetSittingAvatarsCount() != 0)
|
||||
{
|
||||
if (group.GetSittingAvatarsCount() != 0)
|
||||
{
|
||||
// m_log.WarnFormat(
|
||||
// "[ATTACHMENTS MODULE]: Ignoring request to attach {0} {1} to {2} on {3} since {4} avatars are still sitting on it",
|
||||
// group.Name, group.LocalId, sp.Name, attachmentPt, group.GetSittingAvatarsCount());
|
||||
|
||||
return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
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);
|
||||
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;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Remove any previous attachments
|
||||
List<SceneObjectGroup> existingAttachments = sp.GetAttachments(attachmentPt);
|
||||
string existingAttachmentScriptState = null;
|
||||
|
||||
// At the moment we can only deal with a single attachment
|
||||
if (existingAttachments.Count != 0 && existingAttachments[0].FromItemID != UUID.Zero)
|
||||
DetachSingleAttachmentToInv(sp, group);
|
||||
|
||||
lock (sp.AttachmentsSyncLock)
|
||||
{
|
||||
Vector3 attachPos = group.AbsolutePosition;
|
||||
|
||||
// TODO: this short circuits multiple attachments functionality in LL viewer 2.1+ and should
|
||||
|
|
@ -352,21 +375,6 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
|
||||
private void UpdateUserInventoryWithAttachment(IScenePresence sp, SceneObjectGroup group, uint attachmentPt, bool temp)
|
||||
{
|
||||
// Remove any previous attachments
|
||||
List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
|
||||
|
||||
// At the moment we can only deal with a single attachment
|
||||
if (attachments.Count != 0)
|
||||
{
|
||||
if (attachments[0].FromItemID != UUID.Zero)
|
||||
DetachSingleAttachmentToInvInternal(sp, attachments[0]);
|
||||
// Error logging commented because UUID.Zero now means temp attachment
|
||||
// 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);
|
||||
}
|
||||
|
||||
// Add the new attachment to inventory if we don't already have it.
|
||||
if (!temp)
|
||||
{
|
||||
|
|
@ -426,12 +434,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
return;
|
||||
|
||||
// m_log.DebugFormat("[ATTACHMENTS MODULE]: Rezzing multiple attachments from inventory for {0}", sp.Name);
|
||||
lock (sp.AttachmentsSyncLock)
|
||||
|
||||
foreach (KeyValuePair<UUID, uint> rez in rezlist)
|
||||
{
|
||||
foreach (KeyValuePair<UUID, uint> rez in rezlist)
|
||||
{
|
||||
RezSingleAttachmentFromInventory(sp, rez.Key, rez.Value);
|
||||
}
|
||||
RezSingleAttachmentFromInventory(sp, rez.Key, rez.Value);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -511,25 +517,33 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
|
||||
public void DetachSingleAttachmentToInv(IScenePresence sp, SceneObjectGroup so)
|
||||
{
|
||||
if (so.AttachedAvatar != sp.UUID)
|
||||
{
|
||||
m_log.WarnFormat(
|
||||
"[ATTACHMENTS MODULE]: Tried to detach object {0} from {1} {2} but attached avatar id was {3} in {4}",
|
||||
so.Name, sp.Name, sp.UUID, so.AttachedAvatar, m_scene.RegionInfo.RegionName);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
// Scripts MUST be snapshotted before the object is
|
||||
// removed from the scene because doing otherwise will
|
||||
// clobber the run flag
|
||||
// This must be done outside the sp.AttachmentSyncLock so that there is no risk of a deadlock from
|
||||
// scripts performing attachment operations at the same time. Getting object states stops the scripts.
|
||||
string scriptedState = PrepareScriptInstanceForSave(so, true);
|
||||
|
||||
lock (sp.AttachmentsSyncLock)
|
||||
{
|
||||
// Save avatar attachment information
|
||||
// m_log.Debug("[ATTACHMENTS MODULE]: Detaching from UserID: " + sp.UUID + ", ItemID: " + itemID);
|
||||
|
||||
if (so.AttachedAvatar != sp.UUID)
|
||||
{
|
||||
m_log.WarnFormat(
|
||||
"[ATTACHMENTS MODULE]: Tried to detach object {0} from {1} {2} but attached avatar id was {3} in {4}",
|
||||
so.Name, sp.Name, sp.UUID, so.AttachedAvatar, m_scene.RegionInfo.RegionName);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
bool changed = sp.Appearance.DetachAttachment(so.FromItemID);
|
||||
if (changed && m_scene.AvatarFactory != null)
|
||||
m_scene.AvatarFactory.QueueAppearanceSave(sp.UUID);
|
||||
|
||||
DetachSingleAttachmentToInvInternal(sp, so);
|
||||
sp.RemoveAttachment(so);
|
||||
UpdateDetachedObject(sp, so, scriptedState);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -739,8 +753,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
return newItem;
|
||||
}
|
||||
|
||||
private string GetObjectScriptStates(SceneObjectGroup grp)
|
||||
/// <summary>
|
||||
/// Prepares the script instance for save.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// This involves triggering the detach event and getting the script state (which also stops the script)
|
||||
/// This MUST be done outside sp.AttachmentsSyncLock, since otherwise there is a chance of deadlock if a
|
||||
/// running script is performing attachment operations.
|
||||
/// </remarks>
|
||||
/// <returns>
|
||||
/// The script state ready for persistence.
|
||||
/// </returns>
|
||||
/// <param name='grp'>
|
||||
/// </param>
|
||||
/// <param name='fireDetachEvent'>
|
||||
/// If true, then fire the script event before we save its state.
|
||||
/// </param>
|
||||
private string PrepareScriptInstanceForSave(SceneObjectGroup grp, bool fireDetachEvent)
|
||||
{
|
||||
if (fireDetachEvent)
|
||||
m_scene.EventManager.TriggerOnAttach(grp.LocalId, grp.FromItemID, UUID.Zero);
|
||||
|
||||
using (StringWriter sw = new StringWriter())
|
||||
{
|
||||
using (XmlTextWriter writer = new XmlTextWriter(sw))
|
||||
|
|
@ -752,7 +785,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
}
|
||||
}
|
||||
|
||||
private void UpdateDetachedObject(IScenePresence sp, SceneObjectGroup so)
|
||||
private void UpdateDetachedObject(IScenePresence sp, SceneObjectGroup so, string scriptedState)
|
||||
{
|
||||
// Don't save attachments for HG visitors, it
|
||||
// messes up their inventory. When a HG visitor logs
|
||||
|
|
@ -765,11 +798,6 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
&& (m_scene.UserManagementModule == null
|
||||
|| m_scene.UserManagementModule.IsLocalGridUser(sp.UUID));
|
||||
|
||||
// Scripts MUST be snapshotted before the object is
|
||||
// removed from the scene because doing otherwise will
|
||||
// clobber the run flag
|
||||
string scriptedState = GetObjectScriptStates(so);
|
||||
|
||||
// Remove the object from the scene so no more updates
|
||||
// are sent. Doing this before the below changes will ensure
|
||||
// updates can't cause "HUD artefacts"
|
||||
|
|
@ -793,91 +821,87 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
so.RemoveScriptInstances(true);
|
||||
}
|
||||
|
||||
private void DetachSingleAttachmentToInvInternal(IScenePresence sp, SceneObjectGroup so)
|
||||
{
|
||||
// m_log.DebugFormat("[ATTACHMENTS MODULE]: Detaching item {0} to inventory for {1}", itemID, sp.Name);
|
||||
|
||||
m_scene.EventManager.TriggerOnAttach(so.LocalId, so.FromItemID, UUID.Zero);
|
||||
sp.RemoveAttachment(so);
|
||||
|
||||
UpdateDetachedObject(sp, so);
|
||||
}
|
||||
|
||||
private SceneObjectGroup RezSingleAttachmentFromInventoryInternal(
|
||||
IScenePresence sp, UUID itemID, UUID assetID, uint attachmentPt)
|
||||
{
|
||||
if (m_invAccessModule == null)
|
||||
return null;
|
||||
|
||||
SceneObjectGroup objatt;
|
||||
|
||||
if (itemID != UUID.Zero)
|
||||
objatt = m_invAccessModule.RezObject(sp.ControllingClient,
|
||||
itemID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
|
||||
false, false, sp.UUID, true);
|
||||
else
|
||||
objatt = m_invAccessModule.RezObject(sp.ControllingClient,
|
||||
null, assetID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
|
||||
false, false, sp.UUID, true);
|
||||
|
||||
if (objatt == null)
|
||||
{
|
||||
m_log.WarnFormat(
|
||||
"[ATTACHMENTS MODULE]: Could not retrieve item {0} for attaching to avatar {1} at point {2}",
|
||||
itemID, sp.Name, attachmentPt);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
// Remove any previous attachments
|
||||
List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
|
||||
string previousAttachmentScriptedState = null;
|
||||
|
||||
// At the moment we can only deal with a single attachment
|
||||
if (attachments.Count != 0)
|
||||
DetachSingleAttachmentToInv(sp, attachments[0]);
|
||||
|
||||
lock (sp.AttachmentsSyncLock)
|
||||
{
|
||||
SceneObjectGroup objatt;
|
||||
|
||||
if (itemID != UUID.Zero)
|
||||
objatt = m_invAccessModule.RezObject(sp.ControllingClient,
|
||||
itemID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
|
||||
false, false, sp.UUID, true);
|
||||
else
|
||||
objatt = m_invAccessModule.RezObject(sp.ControllingClient,
|
||||
null, assetID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
|
||||
false, false, sp.UUID, true);
|
||||
|
||||
if (objatt != null)
|
||||
{
|
||||
// m_log.DebugFormat(
|
||||
// "[ATTACHMENTS MODULE]: Rezzed single object {0} for attachment to {1} on point {2} in {3}",
|
||||
// objatt.Name, sp.Name, attachmentPt, m_scene.Name);
|
||||
|
||||
// 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;
|
||||
// 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;
|
||||
|
||||
// FIXME: Detect whether it's really likely for AttachObject to throw an exception in the normal
|
||||
// course of events. If not, then it's probably not worth trying to recover the situation
|
||||
// since this is more likely to trigger further exceptions and confuse later debugging. If
|
||||
// exceptions can be thrown in expected error conditions (not NREs) then make this consistent
|
||||
// since other normal error conditions will simply return false instead.
|
||||
// This will throw if the attachment fails
|
||||
try
|
||||
{
|
||||
AttachObjectInternal(sp, objatt, attachmentPt, false, 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;
|
||||
}
|
||||
else
|
||||
// FIXME: Detect whether it's really likely for AttachObject to throw an exception in the normal
|
||||
// course of events. If not, then it's probably not worth trying to recover the situation
|
||||
// since this is more likely to trigger further exceptions and confuse later debugging. If
|
||||
// exceptions can be thrown in expected error conditions (not NREs) then make this consistent
|
||||
// since other normal error conditions will simply return false instead.
|
||||
// This will throw if the attachment fails
|
||||
try
|
||||
{
|
||||
m_log.WarnFormat(
|
||||
"[ATTACHMENTS MODULE]: Could not retrieve item {0} for attaching to avatar {1} at point {2}",
|
||||
itemID, sp.Name, attachmentPt);
|
||||
AttachObjectInternal(sp, objatt, attachmentPt, false, 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);
|
||||
|
||||
return null;
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -1027,17 +1051,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
|
|||
ScenePresence sp = m_scene.GetScenePresence(remoteClient.AgentId);
|
||||
if (sp != null)
|
||||
{
|
||||
lock (sp.AttachmentsSyncLock)
|
||||
{
|
||||
List<SceneObjectGroup> attachments = sp.GetAttachments();
|
||||
List<SceneObjectGroup> attachments = sp.GetAttachments();
|
||||
|
||||
foreach (SceneObjectGroup group in attachments)
|
||||
foreach (SceneObjectGroup group in attachments)
|
||||
{
|
||||
if (group.FromItemID == itemID && group.FromItemID != UUID.Zero)
|
||||
{
|
||||
if (group.FromItemID == itemID && group.FromItemID != UUID.Zero)
|
||||
{
|
||||
DetachSingleAttachmentToInv(sp, group);
|
||||
return;
|
||||
}
|
||||
DetachSingleAttachmentToInv(sp, group);
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue