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 +++++++-------------- .../AssetTransaction/AssetTransactionModule.cs | 9 +- .../Agent/AssetTransaction/AssetXferUploader.cs | 72 +++++++++--- 3 files changed, 94 insertions(+), 110 deletions(-) (limited to 'OpenSim/Region') 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 diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs index 7081989..10a0794 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs @@ -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); } /// diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs index ec4dfd0..9f05120 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs @@ -41,11 +41,26 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); /// + /// Upload state. + /// + /// + /// New -> Uploading -> Complete + /// + private enum UploadState + { + New, + Uploading, + Complete + } + + /// /// Reference to the object that holds this uploader. Used to remove ourselves from it's list if we /// are performing a delayed update. /// 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 } /// - /// Initialise asset transfer from the client + /// Start asset transfer from the client /// - /// - /// - /// - public void Initialise(IClientAPI remoteClient, UUID assetID, - UUID transaction, sbyte type, byte[] data, bool storeLocal, - bool tempFile) + /// + /// + /// + /// + /// + /// 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. + /// + /// + /// + 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 /// null if the asset has not finished uploading public AssetBase GetAssetData() { - if (m_finished) + if (m_uploadState == UploadState.Complete) { return m_asset; } @@ -342,4 +378,4 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction return null; } } -} +} \ No newline at end of file -- cgit v1.1 From 2f795e4fa6433269748f4e062d4bba7197e46ab1 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 25 Sep 2012 22:08:11 +0100 Subject: Move UDP update task item code to AssetXferUploader to match existing create user item and update user item mechanisms This is done for consistency and to allow removal or some access methods that increase code complexity. However, this path has not been used for a long time, not even by LL 1.23 - viewers use caps http upload for this instead --- .../AssetTransaction/AgentAssetsTransactions.cs | 47 +-------------- .../AssetTransaction/AssetTransactionModule.cs | 2 +- .../Agent/AssetTransaction/AssetXferUploader.cs | 67 ++++++++++++++++------ 3 files changed, 52 insertions(+), 64 deletions(-) (limited to 'OpenSim/Region') diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs index bba7b9c..59d0075 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs @@ -155,56 +155,13 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction description, name, invType, type, wearableType, nextOwnerMask); } - /// - /// Get an uploaded asset. If the data is successfully retrieved, - /// the transaction will be removed. - /// - /// - /// The asset if the upload has completed, null if it has not. - private AssetBase GetTransactionAsset(UUID transactionID) - { - lock (XferUploaders) - { - if (XferUploaders.ContainsKey(transactionID)) - { - AssetXferUploader uploader = XferUploaders[transactionID]; - AssetBase asset = uploader.GetAssetData(); - RemoveXferUploader(transactionID); - - return asset; - } - } - - return null; - } - public void RequestUpdateTaskInventoryItem(IClientAPI remoteClient, SceneObjectPart part, UUID transactionID, TaskInventoryItem item) { - 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; + AssetXferUploader uploader = RequestXferUploader(transactionID); - 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); - } + uploader.RequestUpdateTaskInventoryItem(remoteClient, transactionID, item); } public void RequestUpdateInventoryItem(IClientAPI remoteClient, diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs index 10a0794..73d1f72 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs @@ -215,7 +215,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction IClientAPI remoteClient, SceneObjectPart part, UUID transactionID, TaskInventoryItem item) { m_log.DebugFormat( - "[TRANSACTIONS MANAGER] Called HandleTaskItemUpdateFromTransaction with item {0} in {1} for {2} in {3}", + "[ASSET TRANSACTION MODULE] Called HandleTaskItemUpdateFromTransaction with item {0} in {1} for {2} in {3}", item.Name, part.Name, remoteClient.Name, m_Scene.RegionInfo.RegionName); AgentAssetTransactions transactions = diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs index 9f05120..08c3134 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs @@ -65,11 +65,15 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction private UUID InventFolder = UUID.Zero; private sbyte invType = 0; - private bool m_createItem = false; - private uint m_createItemCallback = 0; - private bool m_updateItem = false; + private bool m_createItem; + private uint m_createItemCallback; + + private bool m_updateItem; private InventoryItemBase m_updateItemData; + private bool m_updateTaskItem; + private TaskInventoryItem m_updateTaskItemData; + private string m_description = String.Empty; private bool m_dumpAssetToFile; private string m_name = String.Empty; @@ -223,6 +227,12 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction // TODO: Should probably do the same for create item. m_transactions.RemoveXferUploader(TransactionID); } + else if (m_updateTaskItem) + { + StoreAssetForTaskItemUpdate(m_updateTaskItemData); + + m_transactions.RemoveXferUploader(TransactionID); + } else if (m_storeLocal) { m_Scene.AssetService.Store(m_asset); @@ -323,8 +333,30 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction } } + public void RequestUpdateTaskInventoryItem(IClientAPI remoteClient, UUID transactionID, TaskInventoryItem taskItem) + { + // We must lock to avoid a race with a separate thread uploading the asset. + lock (this) + { + m_asset.Name = taskItem.Name; + m_asset.Description = taskItem.Description; + m_asset.Type = (sbyte)taskItem.Type; + taskItem.AssetID = m_asset.FullID; + + if (m_uploadState == UploadState.Complete) + { + StoreAssetForTaskItemUpdate(taskItem); + } + else + { + m_updateTaskItem = true; + m_updateTaskItemData = taskItem; + } + } + } + /// - /// Store the asset for the given item. + /// Store the asset for the given item when it has been uploaded. /// /// private void StoreAssetForItemUpdate(InventoryItemBase item) @@ -336,6 +368,19 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction m_Scene.AssetService.Store(m_asset); } + /// + /// Store the asset for the given task item when it has been uploaded. + /// + /// + private void StoreAssetForTaskItemUpdate(TaskInventoryItem taskItem) + { +// m_log.DebugFormat( +// "[ASSET XFER UPLOADER]: Storing asset {0} for earlier task item update for {1} for {2}", +// m_asset.FullID, taskItem.Name, ourClient.Name); + + m_Scene.AssetService.Store(m_asset); + } + private void DoCreateItem(uint callbackID) { m_Scene.AssetService.Store(m_asset); @@ -363,19 +408,5 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction else ourClient.SendAlertMessage("Unable to create inventory item"); } - - /// - /// Get the asset data uploaded in this transfer. - /// - /// null if the asset has not finished uploading - public AssetBase GetAssetData() - { - if (m_uploadState == UploadState.Complete) - { - return m_asset; - } - - return null; - } } } \ No newline at end of file -- cgit v1.1 From eb5bec96e49466ede4ece376cb15cd68477ce983 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 25 Sep 2012 22:54:20 +0100 Subject: Insert transaction ID into AssetXferUploader constructor rather than at UploadAsset() to prevent item creation failure when NewInventoryItem thread reachs the object first. This was preventing the previous race condition fix in 4fc0cfb from actually working. This commit also removes some of the pointless transaction id checks - these conditions are already being enforced in AgentAssetsTransactions. --- .../AssetTransaction/AgentAssetsTransactions.cs | 8 +-- .../Agent/AssetTransaction/AssetXferUploader.cs | 77 +++++++++++++--------- 2 files changed, 49 insertions(+), 36 deletions(-) (limited to 'OpenSim/Region') diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs index 59d0075..0271738 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs @@ -73,7 +73,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction { if (!XferUploaders.ContainsKey(transactionID)) { - uploader = new AssetXferUploader(this, m_Scene, m_dumpAssetsToFile); + uploader = new AssetXferUploader(this, m_Scene, transactionID, m_dumpAssetsToFile); // m_log.DebugFormat( // "[AGENT ASSETS TRANSACTIONS]: Adding asset xfer uploader {0} since it didn't previously exist", transactionID); @@ -151,7 +151,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction AssetXferUploader uploader = RequestXferUploader(transactionID); uploader.RequestCreateInventoryItem( - remoteClient, transactionID, folderID, callbackID, + remoteClient, folderID, callbackID, description, name, invType, type, wearableType, nextOwnerMask); } @@ -161,7 +161,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction { AssetXferUploader uploader = RequestXferUploader(transactionID); - uploader.RequestUpdateTaskInventoryItem(remoteClient, transactionID, item); + uploader.RequestUpdateTaskInventoryItem(remoteClient, item); } public void RequestUpdateInventoryItem(IClientAPI remoteClient, @@ -169,7 +169,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction { AssetXferUploader uploader = RequestXferUploader(transactionID); - uploader.RequestUpdateInventoryItem(remoteClient, transactionID, item); + uploader.RequestUpdateInventoryItem(remoteClient, item); } } } \ No newline at end of file diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs index 08c3134..52d7d57 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs @@ -80,15 +80,32 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction private bool m_storeLocal; private uint nextPerm = 0; private IClientAPI ourClient; - private UUID TransactionID = UUID.Zero; + + private UUID m_transactionID; + private sbyte type = 0; private byte wearableType = 0; public ulong XferID; private Scene m_Scene; - public AssetXferUploader(AgentAssetTransactions transactions, Scene scene, bool dumpAssetToFile) + /// + /// AssetXferUploader constructor + /// + /// /param> + /// + /// + /// + /// If true then when the asset is uploaded it is dumped to a file with the format + /// String.Format("{6}_{7}_{0:d2}{1:d2}{2:d2}_{3:d2}{4:d2}{5:d2}.dat", + /// now.Year, now.Month, now.Day, now.Hour, now.Minute, + /// now.Second, m_asset.Name, m_asset.Type); + /// for debugging purposes. + /// + public AssetXferUploader( + AgentAssetTransactions transactions, Scene scene, UUID transactionID, bool dumpAssetToFile) { m_transactions = transactions; + m_transactionID = transactionID; m_Scene = scene; m_dumpAssetToFile = dumpAssetToFile; } @@ -180,7 +197,6 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction m_asset.Local = storeLocal; m_asset.Temporary = tempFile; - TransactionID = transaction; m_storeLocal = storeLocal; if (m_asset.Data.Length > 2) @@ -225,13 +241,13 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction // Remove ourselves from the list of transactions if completion was delayed until the transaction // was complete. // TODO: Should probably do the same for create item. - m_transactions.RemoveXferUploader(TransactionID); + m_transactions.RemoveXferUploader(m_transactionID); } else if (m_updateTaskItem) { StoreAssetForTaskItemUpdate(m_updateTaskItemData); - m_transactions.RemoveXferUploader(TransactionID); + m_transactions.RemoveXferUploader(m_transactionID); } else if (m_storeLocal) { @@ -241,7 +257,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction m_log.DebugFormat( "[ASSET XFER UPLOADER]: Uploaded asset {0} for transaction {1}", - m_asset.FullID, TransactionID); + m_asset.FullID, m_transactionID); if (m_dumpAssetToFile) { @@ -269,40 +285,37 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction } public void RequestCreateInventoryItem(IClientAPI remoteClient, - UUID transactionID, UUID folderID, uint callbackID, + UUID folderID, uint callbackID, string description, string name, sbyte invType, sbyte type, byte wearableType, uint nextOwnerMask) { - if (TransactionID == transactionID) + InventFolder = folderID; + m_name = name; + m_description = description; + this.type = type; + this.invType = invType; + this.wearableType = wearableType; + nextPerm = nextOwnerMask; + m_asset.Name = name; + m_asset.Description = description; + m_asset.Type = type; + + // We must lock to avoid a race with a separate thread uploading the asset. + lock (this) { - InventFolder = folderID; - m_name = name; - m_description = description; - this.type = type; - this.invType = invType; - this.wearableType = wearableType; - nextPerm = nextOwnerMask; - m_asset.Name = name; - m_asset.Description = description; - m_asset.Type = type; - - // We must lock to avoid a race with a separate thread uploading the asset. - lock (this) + if (m_uploadState == UploadState.Complete) + { + DoCreateItem(callbackID); + } + else { - if (m_uploadState == UploadState.Complete) - { - DoCreateItem(callbackID); - } - else - { - m_createItem = true; //set flag so the inventory item is created when upload is complete - m_createItemCallback = callbackID; - } + m_createItem = true; //set flag so the inventory item is created when upload is complete + m_createItemCallback = callbackID; } } } - public void RequestUpdateInventoryItem(IClientAPI remoteClient, UUID transactionID, InventoryItemBase item) + public void RequestUpdateInventoryItem(IClientAPI remoteClient, InventoryItemBase item) { // We must lock to avoid a race with a separate thread uploading the asset. lock (this) @@ -333,7 +346,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction } } - public void RequestUpdateTaskInventoryItem(IClientAPI remoteClient, UUID transactionID, TaskInventoryItem taskItem) + public void RequestUpdateTaskInventoryItem(IClientAPI remoteClient, TaskInventoryItem taskItem) { // We must lock to avoid a race with a separate thread uploading the asset. lock (this) -- cgit v1.1 From b96a53962b86083476b78498278868ce8152ddd8 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 25 Sep 2012 23:12:28 +0100 Subject: Comment out old m_storeLocal from AssetXferUploader. This was only used if none of new item, update item or update task item had been set. But since all transactions go through these paths this old code is redundant. --- .../CoreModules/Agent/AssetTransaction/AssetXferUploader.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'OpenSim/Region') diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs index 52d7d57..8d21202 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs @@ -77,7 +77,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction private string m_description = String.Empty; private bool m_dumpAssetToFile; private string m_name = String.Empty; - private bool m_storeLocal; +// private bool m_storeLocal; private uint nextPerm = 0; private IClientAPI ourClient; @@ -197,7 +197,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction m_asset.Local = storeLocal; m_asset.Temporary = tempFile; - m_storeLocal = storeLocal; +// m_storeLocal = storeLocal; if (m_asset.Data.Length > 2) { @@ -249,10 +249,10 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction m_transactions.RemoveXferUploader(m_transactionID); } - else if (m_storeLocal) - { - m_Scene.AssetService.Store(m_asset); - } +// else if (m_storeLocal) +// { +// m_Scene.AssetService.Store(m_asset); +// } } m_log.DebugFormat( -- cgit v1.1 From ddd9384b3901f532243c1e8018334385b84290d1 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Tue, 25 Sep 2012 23:30:25 +0100 Subject: Fix very recently introduced race condition where a CreateNewItem outracing an UploadAsset request could throw an exception because m_asset did not yet exist. This was accidentally introduced in 4fc0cfb This commit also consistently removes the AssetXferUploader when the transaction completes, no matter if it completed on asset upload or item operation. The amount of data being retained was small, since this was clothing/bodypart metadata in the asset rather than textures themselves. --- .../Agent/AssetTransaction/AssetXferUploader.cs | 37 +++++++++++----------- 1 file changed, 18 insertions(+), 19 deletions(-) (limited to 'OpenSim/Region') diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs index 8d21202..8add4bb 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs @@ -104,6 +104,8 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction public AssetXferUploader( AgentAssetTransactions transactions, Scene scene, UUID transactionID, bool dumpAssetToFile) { + m_asset = new AssetBase(); + m_transactions = transactions; m_transactionID = transactionID; m_Scene = scene; @@ -188,9 +190,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction ourClient = remoteClient; - m_asset = new AssetBase() { FullID = assetID }; - m_asset.Name = "blank"; - m_asset.Description = "empty"; + m_asset.FullID = assetID; m_asset.Type = type; m_asset.CreatorID = remoteClient.AgentId.ToString(); m_asset.Data = data; @@ -232,22 +232,15 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction if (m_createItem) { - DoCreateItem(m_createItemCallback); + CompleteCreateItem(m_createItemCallback); } else if (m_updateItem) { - StoreAssetForItemUpdate(m_updateItemData); - - // Remove ourselves from the list of transactions if completion was delayed until the transaction - // was complete. - // TODO: Should probably do the same for create item. - m_transactions.RemoveXferUploader(m_transactionID); + CompleteItemUpdate(m_updateItemData); } else if (m_updateTaskItem) { - StoreAssetForTaskItemUpdate(m_updateTaskItemData); - - m_transactions.RemoveXferUploader(m_transactionID); + CompleteTaskItemUpdate(m_updateTaskItemData); } // else if (m_storeLocal) // { @@ -305,7 +298,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction { if (m_uploadState == UploadState.Complete) { - DoCreateItem(callbackID); + CompleteCreateItem(callbackID); } else { @@ -332,7 +325,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction if (m_uploadState == UploadState.Complete) { - StoreAssetForItemUpdate(item); + CompleteItemUpdate(item); } else { @@ -358,7 +351,7 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction if (m_uploadState == UploadState.Complete) { - StoreAssetForTaskItemUpdate(taskItem); + CompleteTaskItemUpdate(taskItem); } else { @@ -372,29 +365,33 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction /// Store the asset for the given item when it has been uploaded. /// /// - private void StoreAssetForItemUpdate(InventoryItemBase item) + private void CompleteItemUpdate(InventoryItemBase item) { // m_log.DebugFormat( // "[ASSET XFER UPLOADER]: Storing asset {0} for earlier item update for {1} for {2}", // m_asset.FullID, item.Name, ourClient.Name); m_Scene.AssetService.Store(m_asset); + + m_transactions.RemoveXferUploader(m_transactionID); } /// /// Store the asset for the given task item when it has been uploaded. /// /// - private void StoreAssetForTaskItemUpdate(TaskInventoryItem taskItem) + private void CompleteTaskItemUpdate(TaskInventoryItem taskItem) { // m_log.DebugFormat( // "[ASSET XFER UPLOADER]: Storing asset {0} for earlier task item update for {1} for {2}", // m_asset.FullID, taskItem.Name, ourClient.Name); m_Scene.AssetService.Store(m_asset); + + m_transactions.RemoveXferUploader(m_transactionID); } - private void DoCreateItem(uint callbackID) + private void CompleteCreateItem(uint callbackID) { m_Scene.AssetService.Store(m_asset); @@ -420,6 +417,8 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction ourClient.SendInventoryItemCreateUpdate(item, callbackID); else ourClient.SendAlertMessage("Unable to create inventory item"); + + m_transactions.RemoveXferUploader(m_transactionID); } } } \ No newline at end of file -- cgit v1.1