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)
connector_plugin
Justin Clark-Casey (justincc) 2012-09-25 21:35:39 +01:00
parent 020103c51e
commit 4fc0cfba3c
3 changed files with 94 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -57,40 +57,37 @@ 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);
}
else
{
uploader = XferUploaders[transactionID];
}
}
return uploader;
}
}
m_log.WarnFormat("[AGENT ASSETS TRANSACTIONS]: Ignoring request for asset xfer uploader {0} since it already exists", transactionID);
return null;
}
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);
remoteClient, transactionID, folderID, callbackID,
description, name, invType, type, wearableType, nextOwnerMask);
}
/// <summary>
@ -196,16 +181,6 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
public void RequestUpdateTaskInventoryItem(IClientAPI remoteClient,
SceneObjectPart part, UUID transactionID,
TaskInventoryItem item)
{
AssetXferUploader uploader = null;
lock (XferUploaders)
{
if (XferUploaders.ContainsKey(transactionID))
uploader = XferUploaders[transactionID];
}
if (uploader != null)
{
AssetBase asset = GetTransactionAsset(transactionID);
@ -231,35 +206,13 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
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);
}
}
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);
}
}
}
}

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>

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

@ -40,12 +40,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;
@ -57,7 +72,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;
@ -68,11 +82,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;
}
@ -118,20 +131,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,
/// <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;
@ -166,14 +202,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);
@ -243,7 +279,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);
}
@ -271,7 +307,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);
}
@ -334,7 +370,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)
{
return m_asset;
}