From 4fc0cfba3ce1e6545e334f8e34a0e5b45274081e Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Tue, 25 Sep 2012 21:35:39 +0100
Subject: Fix occasional race condition failure when creating new clothing/body
parts in the viewer or updating existing assets.
On creating these items, the viewer sends a UDP AssetUploadRequest followed by a CreateInventoryItem.
It was possible for the CreateInventoryItem/UpdateInventoryItem to occasionally outrace the AssetUploadRequest and fail to find an initialized Xfer object, at which point the item create would fail.
So instead we always set up a Xfer object on either the asset or inventory item update request.
This does not introduce a new race because code already exists to delay the item operation until the asset is uploaded if necessary (but this only worked if the xfer object already existed)
---
.../AssetTransaction/AgentAssetsTransactions.cs | 123 +++++++--------------
1 file changed, 38 insertions(+), 85 deletions(-)
(limited to 'OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs')
diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs
index b557ffe..bba7b9c 100644
--- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs
+++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs
@@ -57,39 +57,36 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
}
///
- /// Return a xfer uploader if one does not already exist.
+ /// Return the xfer uploader for the given transaction.
///
+ ///
+ /// If an uploader does not already exist for this transaction then it is created, otherwise the existing
+ /// uploader is returned.
+ ///
///
- ///
- /// We must transfer the new asset ID into the uploader on creation, otherwise
- /// we can see race conditions with other threads which can retrieve an item before it is updated with the new
- /// asset id.
- ///
- ///
- /// The xfer uploader requested. Null if one is already in existence.
- /// FIXME: This is a bizarre thing to do, and is probably meant to signal an error condition if multiple
- /// transfers are made. Needs to be corrected.
- ///
- public AssetXferUploader RequestXferUploader(UUID transactionID, UUID assetID)
+ /// The asset xfer uploader
+ public AssetXferUploader RequestXferUploader(UUID transactionID)
{
+ AssetXferUploader uploader;
+
lock (XferUploaders)
{
if (!XferUploaders.ContainsKey(transactionID))
{
- AssetXferUploader uploader = new AssetXferUploader(this, m_Scene, assetID, m_dumpAssetsToFile);
+ uploader = new AssetXferUploader(this, m_Scene, m_dumpAssetsToFile);
// m_log.DebugFormat(
// "[AGENT ASSETS TRANSACTIONS]: Adding asset xfer uploader {0} since it didn't previously exist", transactionID);
XferUploaders.Add(transactionID, uploader);
-
- return uploader;
+ }
+ else
+ {
+ uploader = XferUploaders[transactionID];
}
}
- m_log.WarnFormat("[AGENT ASSETS TRANSACTIONS]: Ignoring request for asset xfer uploader {0} since it already exists", transactionID);
-
- return null;
+ return uploader;
}
public void HandleXfer(ulong xferID, uint packetID, byte[] data)
@@ -151,23 +148,11 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
string description, string name, sbyte invType,
sbyte type, byte wearableType, uint nextOwnerMask)
{
- AssetXferUploader uploader = null;
+ AssetXferUploader uploader = RequestXferUploader(transactionID);
- lock (XferUploaders)
- {
- if (XferUploaders.ContainsKey(transactionID))
- uploader = XferUploaders[transactionID];
- }
-
- if (uploader != null)
- uploader.RequestCreateInventoryItem(
- remoteClient, transactionID, folderID,
- callbackID, description, name, invType, type,
- wearableType, nextOwnerMask);
- else
- m_log.ErrorFormat(
- "[AGENT ASSET TRANSACTIONS]: Could not find uploader with transaction ID {0} when handling request to create inventory item {1} from {2}",
- transactionID, name, remoteClient.Name);
+ uploader.RequestCreateInventoryItem(
+ remoteClient, transactionID, folderID, callbackID,
+ description, name, invType, type, wearableType, nextOwnerMask);
}
///
@@ -197,69 +182,37 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
SceneObjectPart part, UUID transactionID,
TaskInventoryItem item)
{
- AssetXferUploader uploader = null;
+ AssetBase asset = GetTransactionAsset(transactionID);
- lock (XferUploaders)
- {
- if (XferUploaders.ContainsKey(transactionID))
- uploader = XferUploaders[transactionID];
- }
+ // Only legacy viewers use this, and they prefer CAPS, which
+ // we have, so this really never runs.
+ // Allow it, but only for "safe" types.
+ if ((InventoryType)item.InvType != InventoryType.Notecard &&
+ (InventoryType)item.InvType != InventoryType.LSL)
+ return;
- if (uploader != null)
+ if (asset != null)
{
- AssetBase asset = GetTransactionAsset(transactionID);
-
- // Only legacy viewers use this, and they prefer CAPS, which
- // we have, so this really never runs.
- // Allow it, but only for "safe" types.
- if ((InventoryType)item.InvType != InventoryType.Notecard &&
- (InventoryType)item.InvType != InventoryType.LSL)
- return;
-
- if (asset != null)
- {
// m_log.DebugFormat(
// "[AGENT ASSETS TRANSACTIONS]: Updating item {0} in {1} for transaction {2}",
// item.Name, part.Name, transactionID);
-
- asset.FullID = UUID.Random();
- asset.Name = item.Name;
- asset.Description = item.Description;
- asset.Type = (sbyte)item.Type;
- item.AssetID = asset.FullID;
-
- m_Scene.AssetService.Store(asset);
- }
- }
- else
- {
- m_log.ErrorFormat(
- "[AGENT ASSET TRANSACTIONS]: Could not find uploader with transaction ID {0} when handling request to update task inventory item {1} in {2}",
- transactionID, item.Name, part.Name);
+
+ asset.FullID = UUID.Random();
+ asset.Name = item.Name;
+ asset.Description = item.Description;
+ asset.Type = (sbyte)item.Type;
+ item.AssetID = asset.FullID;
+
+ m_Scene.AssetService.Store(asset);
}
}
public void RequestUpdateInventoryItem(IClientAPI remoteClient,
UUID transactionID, InventoryItemBase item)
{
- AssetXferUploader uploader = null;
-
- lock (XferUploaders)
- {
- if (XferUploaders.ContainsKey(transactionID))
- uploader = XferUploaders[transactionID];
- }
+ AssetXferUploader uploader = RequestXferUploader(transactionID);
- if (uploader != null)
- {
- uploader.RequestUpdateInventoryItem(remoteClient, transactionID, item);
- }
- else
- {
- m_log.ErrorFormat(
- "[AGENT ASSET TRANSACTIONS]: Could not find uploader with transaction ID {0} when handling request to update inventory item {1} for {2}",
- transactionID, item.Name, remoteClient.Name);
- }
+ uploader.RequestUpdateInventoryItem(remoteClient, transactionID, item);
}
}
-}
+}
\ No newline at end of file
--
cgit v1.1