OpenSim
/
OpenSimMirror
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

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)
avinationmerge
Justin Clark-Casey (justincc) 2012-09-25 21:35:39 +01:00 committed by Melanie
parent daf84d7385
commit 1a902fceb5
3 changed files with 93 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

123
OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs
View File

@ -57,39 +57,36 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
}
/// <summary>
/// Return a xfer uploader if one does not already exist.
/// Return the xfer uploader for the given transaction.
/// </summary>
/// <remarks>
/// If an uploader does not already exist for this transaction then it is created, otherwise the existing
/// uploader is returned.
/// </remarks>
/// <param name="transactionID"></param>
/// <param name="assetID">
/// 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.
/// </param>
/// <returns>
/// 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.
/// </returns>
public AssetXferUploader RequestXferUploader(UUID transactionID, UUID assetID)
/// <returns>The asset xfer uploader</returns>
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);
}
/// <summary>
@ -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)
// 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)
{
if (XferUploaders.ContainsKey(transactionID))
uploader = XferUploaders[transactionID];
}
if (uploader != 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;
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);
m_Scene.AssetService.Store(asset);
}
}
public void RequestUpdateInventoryItem(IClientAPI remoteClient,
UUID transactionID, InventoryItemBase item)
{
AssetXferUploader uploader = null;
AssetXferUploader uploader = RequestXferUploader(transactionID);
lock (XferUploaders)
{
if (XferUploaders.ContainsKey(transactionID))
uploader = XferUploaders[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);
}
}
}
}

9
OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs
View File

@ -274,13 +274,8 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
}
AgentAssetTransactions transactions = GetUserTransactions(remoteClient.AgentId);
AssetXferUploader uploader = transactions.RequestXferUploader(transaction, assetID);
if (uploader != null)
{
uploader.Initialise(remoteClient, assetID, transaction, type,
data, storeLocal, tempFile);
}
AssetXferUploader uploader = transactions.RequestXferUploader(transaction);
uploader.StartUpload(remoteClient, assetID, transaction, type, data, storeLocal, tempFile);
}
/// <summary>

71
OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs
View File

@ -48,12 +48,27 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
};
private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
/// <summary>
/// Upload state.
/// </summary>
/// <remarks>
/// New -> Uploading -> Complete
/// </remarks>
private enum UploadState
{
New,
Uploading,
Complete
}
/// <summary>
/// Reference to the object that holds this uploader. Used to remove ourselves from it's list if we
/// are performing a delayed update.
/// </summary>
AgentAssetTransactions m_transactions;
private UploadState m_uploadState = UploadState.New;
private AssetBase m_asset;
private UUID InventFolder = UUID.Zero;
private sbyte invType = 0;
@ -65,7 +80,6 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
private string m_description = String.Empty;
private bool m_dumpAssetToFile;
private bool m_finished = false;
private string m_name = String.Empty;
private bool m_storeLocal;
private uint nextPerm = 0;
@ -77,11 +91,10 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
public ulong XferID;
private Scene m_Scene;
public AssetXferUploader(AgentAssetTransactions transactions, Scene scene, UUID assetID, bool dumpAssetToFile)
public AssetXferUploader(AgentAssetTransactions transactions, Scene scene, bool dumpAssetToFile)
{
m_transactions = transactions;
m_Scene = scene;
m_asset = new AssetBase() { FullID = assetID };
m_dumpAssetToFile = dumpAssetToFile;
}
@ -127,20 +140,43 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
}
/// <summary>
/// Initialise asset transfer from the client
/// Start asset transfer from the client
/// </summary>
/// <param name="xferID"></param>
/// <param name="packetID"></param>
/// <param name="data"></param>
public void Initialise(IClientAPI remoteClient, UUID assetID,
UUID transaction, sbyte type, byte[] data, bool storeLocal,
bool tempFile)
/// <param name="remoteClient"></param>
/// <param name="assetID"></param>
/// <param name="transaction"></param>
/// <param name="type"></param>
/// <param name="data">
/// Optional data. If present then the asset is created immediately with this data
/// rather than requesting an upload from the client. The data must be longer than 2 bytes.
/// </param>
/// <param name="storeLocal"></param>
/// <param name="tempFile"></param>
public void StartUpload(
IClientAPI remoteClient, UUID assetID, UUID transaction, sbyte type, byte[] data, bool storeLocal,
bool tempFile)
{
// m_log.DebugFormat(
// "[ASSET XFER UPLOADER]: Initialised xfer from {0}, asset {1}, transaction {2}, type {3}, storeLocal {4}, tempFile {5}, already received data length {6}",
// remoteClient.Name, assetID, transaction, type, storeLocal, tempFile, data.Length);
lock (this)
{
if (m_uploadState != UploadState.New)
{
m_log.WarnFormat(
"[ASSET XFER UPLOADER]: Tried to start upload of asset {0}, transaction {1} for {2} but this is already in state {3}. Aborting.",
assetID, transaction, remoteClient.Name, m_uploadState);
return;
}
m_uploadState = UploadState.Uploading;
}
ourClient = remoteClient;
m_asset = new AssetBase() { FullID = assetID };
m_asset.Name = "blank";
m_asset.Description = "empty";
m_asset.Type = type;
@ -175,14 +211,14 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
protected void SendCompleteMessage()
{
ourClient.SendAssetUploadCompleteMessage(m_asset.Type, true,
m_asset.FullID);
// We must lock in order to avoid a race with a separate thread dealing with an inventory item or create
// message from other client UDP.
lock (this)
{
m_finished = true;
m_uploadState = UploadState.Complete;
ourClient.SendAssetUploadCompleteMessage(m_asset.Type, true, m_asset.FullID);
if (m_createItem)
{
DoCreateItem(m_createItemCallback);
@ -252,7 +288,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
// We must lock to avoid a race with a separate thread uploading the asset.
lock (this)
{
if (m_finished)
if (m_uploadState == UploadState.Complete)
{
DoCreateItem(callbackID);
}
@ -280,7 +316,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
item.AssetID = m_asset.FullID;
m_Scene.InventoryService.UpdateItem(item);
if (m_finished)
if (m_uploadState == UploadState.Complete)
{
StoreAssetForItemUpdate(item);
}
@ -416,7 +452,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
/// <returns>null if the asset has not finished uploading</returns>
public AssetBase GetAssetData()
{
if (m_finished)
if (m_uploadState == UploadState.Complete)
{
ValidateAssets();
return m_asset;
@ -469,4 +505,3 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
}
}
}