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

View File

@ -57,40 +57,37 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
} }
/// <summary> /// <summary>
/// Return a xfer uploader if one does not already exist. /// Return the xfer uploader for the given transaction.
/// </summary> /// </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="transactionID"></param>
/// <param name="assetID"> /// <returns>The asset xfer uploader</returns>
/// We must transfer the new asset ID into the uploader on creation, otherwise public AssetXferUploader RequestXferUploader(UUID transactionID)
/// 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)
{ {
AssetXferUploader uploader;
lock (XferUploaders) lock (XferUploaders)
{ {
if (!XferUploaders.ContainsKey(transactionID)) 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( // m_log.DebugFormat(
// "[AGENT ASSETS TRANSACTIONS]: Adding asset xfer uploader {0} since it didn't previously exist", transactionID); // "[AGENT ASSETS TRANSACTIONS]: Adding asset xfer uploader {0} since it didn't previously exist", transactionID);
XferUploaders.Add(transactionID, uploader); XferUploaders.Add(transactionID, uploader);
}
else
{
uploader = XferUploaders[transactionID];
}
}
return uploader; 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) 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, string description, string name, sbyte invType,
sbyte type, byte wearableType, uint nextOwnerMask) 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( uploader.RequestCreateInventoryItem(
remoteClient, transactionID, folderID, remoteClient, transactionID, folderID, callbackID,
callbackID, description, name, invType, type, description, name, invType, type, wearableType, nextOwnerMask);
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);
} }
/// <summary> /// <summary>
@ -196,16 +181,6 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
public void RequestUpdateTaskInventoryItem(IClientAPI remoteClient, public void RequestUpdateTaskInventoryItem(IClientAPI remoteClient,
SceneObjectPart part, UUID transactionID, SceneObjectPart part, UUID transactionID,
TaskInventoryItem item) TaskInventoryItem item)
{
AssetXferUploader uploader = null;
lock (XferUploaders)
{
if (XferUploaders.ContainsKey(transactionID))
uploader = XferUploaders[transactionID];
}
if (uploader != null)
{ {
AssetBase asset = GetTransactionAsset(transactionID); AssetBase asset = GetTransactionAsset(transactionID);
@ -231,35 +206,13 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
m_Scene.AssetService.Store(asset); 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, public void RequestUpdateInventoryItem(IClientAPI remoteClient,
UUID transactionID, InventoryItemBase item) 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); 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);
}
}
} }
} }

View File

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

View File

@ -40,12 +40,27 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
{ {
private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); 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> /// <summary>
/// Reference to the object that holds this uploader. Used to remove ourselves from it's list if we /// Reference to the object that holds this uploader. Used to remove ourselves from it's list if we
/// are performing a delayed update. /// are performing a delayed update.
/// </summary> /// </summary>
AgentAssetTransactions m_transactions; AgentAssetTransactions m_transactions;
private UploadState m_uploadState = UploadState.New;
private AssetBase m_asset; private AssetBase m_asset;
private UUID InventFolder = UUID.Zero; private UUID InventFolder = UUID.Zero;
private sbyte invType = 0; private sbyte invType = 0;
@ -57,7 +72,6 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
private string m_description = String.Empty; private string m_description = String.Empty;
private bool m_dumpAssetToFile; private bool m_dumpAssetToFile;
private bool m_finished = false;
private string m_name = String.Empty; private string m_name = String.Empty;
private bool m_storeLocal; private bool m_storeLocal;
private uint nextPerm = 0; private uint nextPerm = 0;
@ -68,11 +82,10 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
public ulong XferID; public ulong XferID;
private Scene m_Scene; 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_transactions = transactions;
m_Scene = scene; m_Scene = scene;
m_asset = new AssetBase() { FullID = assetID };
m_dumpAssetToFile = dumpAssetToFile; m_dumpAssetToFile = dumpAssetToFile;
} }
@ -118,20 +131,43 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
} }
/// <summary> /// <summary>
/// Initialise asset transfer from the client /// Start asset transfer from the client
/// </summary> /// </summary>
/// <param name="xferID"></param> /// <param name="remoteClient"></param>
/// <param name="packetID"></param> /// <param name="assetID"></param>
/// <param name="data"></param> /// <param name="transaction"></param>
public void Initialise(IClientAPI remoteClient, UUID assetID, /// <param name="type"></param>
UUID transaction, sbyte type, byte[] data, bool storeLocal, /// <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) bool tempFile)
{ {
// m_log.DebugFormat( // 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}", // "[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); // 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; ourClient = remoteClient;
m_asset = new AssetBase() { FullID = assetID };
m_asset.Name = "blank"; m_asset.Name = "blank";
m_asset.Description = "empty"; m_asset.Description = "empty";
m_asset.Type = type; m_asset.Type = type;
@ -166,14 +202,14 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
protected void SendCompleteMessage() 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 // 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. // message from other client UDP.
lock (this) lock (this)
{ {
m_finished = true; m_uploadState = UploadState.Complete;
ourClient.SendAssetUploadCompleteMessage(m_asset.Type, true, m_asset.FullID);
if (m_createItem) if (m_createItem)
{ {
DoCreateItem(m_createItemCallback); 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. // We must lock to avoid a race with a separate thread uploading the asset.
lock (this) lock (this)
{ {
if (m_finished) if (m_uploadState == UploadState.Complete)
{ {
DoCreateItem(callbackID); DoCreateItem(callbackID);
} }
@ -271,7 +307,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
item.AssetID = m_asset.FullID; item.AssetID = m_asset.FullID;
m_Scene.InventoryService.UpdateItem(item); m_Scene.InventoryService.UpdateItem(item);
if (m_finished) if (m_uploadState == UploadState.Complete)
{ {
StoreAssetForItemUpdate(item); StoreAssetForItemUpdate(item);
} }
@ -334,7 +370,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
/// <returns>null if the asset has not finished uploading</returns> /// <returns>null if the asset has not finished uploading</returns>
public AssetBase GetAssetData() public AssetBase GetAssetData()
{ {
if (m_finished) if (m_uploadState == UploadState.Complete)
{ {
return m_asset; return m_asset;
} }