From 4fc0cfba3ce1e6545e334f8e34a0e5b45274081e Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 25 Sep 2012 21:35:39 +0100 Subject: [PATCH 1/6] 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) --- .../AgentAssetsTransactions.cs | 123 ++++++------------ .../AssetTransactionModule.cs | 9 +- .../AssetTransaction/AssetXferUploader.cs | 72 +++++++--- 3 files changed, 94 insertions(+), 110 deletions(-) diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs index b557ffe5f2..bba7b9ca27 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) + // 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); } } -} +} \ 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 708198923a..10a0794c23 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 ec4dfd0bbb..9f05120798 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs @@ -40,12 +40,27 @@ 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 From 2f795e4fa6433269748f4e062d4bba7197e46ab1 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 25 Sep 2012 22:08:11 +0100 Subject: [PATCH 2/6] 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 --- .../AgentAssetsTransactions.cs | 47 +------------ .../AssetTransactionModule.cs | 2 +- .../AssetTransaction/AssetXferUploader.cs | 67 ++++++++++++++----- 3 files changed, 52 insertions(+), 64 deletions(-) diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs index bba7b9ca27..59d00755eb 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); + AssetXferUploader uploader = RequestXferUploader(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); - } + 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 10a0794c23..73d1f72b2a 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 9f05120798..08c31346bd 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 From eb5bec96e49466ede4ece376cb15cd68477ce983 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 25 Sep 2012 22:54:20 +0100 Subject: [PATCH 3/6] 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. --- .../AgentAssetsTransactions.cs | 8 +- .../AssetTransaction/AssetXferUploader.cs | 77 +++++++++++-------- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs index 59d00755eb..0271738aee 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 08c31346bd..52d7d57986 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; + 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) + // We must lock to avoid a race with a separate thread uploading the asset. + lock (this) + { + if (m_uploadState == UploadState.Complete) { - 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; - } + DoCreateItem(callbackID); + } + else + { + 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) From 3c2fb771339193e17e4fdfbd9f3fc93a4b62b8b7 Mon Sep 17 00:00:00 2001 From: BlueWall Date: Tue, 25 Sep 2012 18:09:34 -0400 Subject: [PATCH 4/6] Format OpenSim.ini.example Make lines fint in 80x24 terminal for easier reading --- bin/OpenSim.ini.example | 100 ++++++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 35 deletions(-) diff --git a/bin/OpenSim.ini.example b/bin/OpenSim.ini.example index 3ec4bab23c..b21a214339 100644 --- a/bin/OpenSim.ini.example +++ b/bin/OpenSim.ini.example @@ -1,19 +1,23 @@ -;; This is the main configuration file for OpenSimulator. If it's named OpenSim.ini -;; then it will be loaded by OpenSimulator. If it's named OpenSim.ini.example then -;; you will need to copy it to OpenSim.ini first (if that file does not already exist) +;; This is the main configuration file for OpenSimulator. +;; If it's named OpenSim.ini then it will be loaded by OpenSimulator. +;; If it's named OpenSim.ini.example then you will need to copy it to +;; OpenSim.ini first (if that file does not already exist) ;; -;; If you are copying, then once you have copied OpenSim.ini.example to OpenSim.ini you will -;; need to pick an architecture in the [Architecture] section at the end of this file. +;; If you are copying, then once you have copied OpenSim.ini.example to +;; OpenSim.ini you will need to pick an architecture in the [Architecture] +;; section at the end of this file. ;; -;; The settings in this file are in the form " = ". For example, save_crashes = false -;; in the [Startup] section below. +;; The settings in this file are in the form " = ". For example, +;; save_crashes = false in the [Startup] section below. ;; -;; All settings are initially commented out and the default value used, as found in -;; OpenSimDefaults.ini. To change a setting, first uncomment it by deleting the initial semicolon (;) -;; and then change the value. This will override the value in OpenSimDefaults.ini +;; All settings are initially commented out and the default value used, as +;; found in OpenSimDefaults.ini. To change a setting, first uncomment it by +;; deleting the initial semicolon (;) and then change the value. This will +;; override the value in OpenSimDefaults.ini ;; -;; If you want to find out what configuration OpenSimulator has finished with once all the configuration -;; files are loaded then type "config show" on the region console command line. +;; If you want to find out what configuration OpenSimulator has finished with +;; once all the configuration files are loaded then type "config show" on the +;; region console command line. ;; ;; ;; NOTES FOR DEVELOPERS REGARDING THE FORMAT OF THIS FILE @@ -26,13 +30,16 @@ ;; formatted as: ;; {option} {depends on} {question to ask} {choices} default value ;; Any text comments following the declaration, up to the next blank line. -;; will be copied to the generated file (NOTE: generation is not yet implemented) +;; will be copied to the generated file (NOTE: generation is not yet +;; implemented) +;; ;; A * in the choices list will allow an empty entry. ;; An empty question will set the default if the dependencies are ;; satisfied. ;; ;; ; denotes a commented out option. -;; Any options added to OpenSim.ini.example should be initially commented out. +;; Any options added to OpenSim.ini.example should be initially commented +;; out. [Startup] @@ -47,9 +54,12 @@ ;# {save_crashes} {} {Save crashes to disk?} {true false} false ;; Set this to true if you want to log crashes to disk ;; this can be useful when submitting bug reports. - ;; However, this will only log crashes within OpenSimulator that cause the entire program to exit - ;; It will not log crashes caused by virtual machine failures, which includes mono and ODE failures. - ;; You will need to capture these native stack traces by recording the session log itself. + ;; However, this will only log crashes within OpenSimulator that cause the + ;; entire program to exit + ;; It will not log crashes caused by virtual machine failures, which + ;; includes mono and ODE failures. + ;; You will need to capture these native stack traces by recording the + ;; session log itself. ; save_crashes = false ;# {crash_dir} {save_crashes:true} {Directory to save crashes to?} {} crashes @@ -88,35 +98,46 @@ ; allow_regionless = false ;# {NonPhysicalPrimMin} {} {Minimum size of nonphysical prims?} {} 0.001 - ;; Minimum size for non-physical prims. Affects resizing of existing prims. This can be overriden in the region config file (as NonPhysicalPrimMin!). + ;; Minimum size for non-physical prims. Affects resizing of existing + ;; prims. This can be overriden in the region config file (as + ;; NonPhysicalPrimMin!). ; NonPhysicalPrimMin = 0.001 ;# {NonPhysicalPrimMax} {} {Maximum size of nonphysical prims?} {} 256 - ;; Maximum size for non-physical prims. Affects resizing of existing prims. This can be overriden in the region config file (as NonPhysicalPrimMax!). + ;; Maximum size for non-physical prims. Affects resizing of existing + ;; prims. This can be overriden in the region config file (as + ;; NonPhysicalPrimMax!). ; NonPhysicalPrimMax = 256 ;# {PhysicalPrimMin} {} {Minimum size of physical prims?} {} 10 - ;; Maximum size where a prim can be physical. Affects resizing of existing prims. This can be overriden in the region config file. + ;; Maximum size where a prim can be physical. Affects resizing of + ;; existing prims. This can be overriden in the region config file. ; PhysicalPrimMin = 0.01 ;# {PhysicalPrimMax} {} {Maximum size of physical prims?} {} 10 - ;; Maximum size where a prim can be physical. Affects resizing of existing prims. This can be overriden in the region config file. + ;; Maximum size where a prim can be physical. Affects resizing of + ;; existing prims. This can be overriden in the region config file. ; PhysicalPrimMax = 10 ;# {ClampPrimSize} {} {Clamp viewer rezzed prims to max sizes?} {true false} false - ;; If a viewer attempts to rez a prim larger than the non-physical or physical prim max, clamp the dimensions to the appropriate maximum + ;; If a viewer attempts to rez a prim larger than the non-physical or + ;; physical prim max, clamp the dimensions to the appropriate maximum ;; This can be overriden in the region config file. ; ClampPrimSize = false ;# {LinksetPrims} {} {Max prims an object will hold?} {} 0 - ;; Maximum number of prims allowable in a linkset. Affects creating new linksets. Ignored if less than or equal to zero. + ;; Maximum number of prims allowable in a linkset. Affects creating new + ;; linksets. Ignored if less than or equal to zero. ;; This can be overriden in the region config file. ; LinksetPrims = 0 ;# {AllowScriptCrossing} {} {Allow scripts to cross into this region} {true false} true - ;; Allow scripts to keep running when they cross region boundaries, rather than being restarted. State is reloaded on the destination region. - ;; This only applies when crossing to a region running in a different simulator. - ;; For crossings where the regions are on the same simulator the script is always kept running. + ;; Allow scripts to keep running when they cross region boundaries, rather + ;; than being restarted. State is reloaded on the destination region. + ;; This only applies when crossing to a region running in a different + ;; simulator. + ;; For crossings where the regions are on the same simulator the script is + ;; always kept running. ; AllowScriptCrossing = true ;# {TrustBinaries} {AllowScriptCrossing:true} {Accept compiled binary script code? (DANGEROUS!)} {true false} false @@ -186,7 +207,8 @@ ;# {physics} {} {Select physics engine} {OpenDynamicsEngine BulletSim basicphysics POS} OpenDynamicsEngine ;; OpenDynamicsEngine is by some distance the most developed physics engine ;; BulletSim is incomplete and experimental but in active development - ;; basicphysics effectively does not model physics at all, making all objects phantom + ;; basicphysics effectively does not model physics at all, making all + ;; objects phantom ;; Default is OpenDynamicsEngine ; physics = OpenDynamicsEngine ; physics = BulletSim @@ -229,8 +251,9 @@ ;# {simple_build_permissions} {} {Allow building in parcel by access list (no groups)} {true false} false ;; More control over permissions ;; This is definitely not SL! - ;; Provides a simple control for land owners to give build rights to specific avatars - ;; in publicly accessible parcels that disallow object creation in general. + ;; Provides a simple control for land owners to give build rights to + ;; specific avatars in publicly accessible parcels that disallow object + ;; creation in general. ;; Owners specific avatars by adding them to the Access List of the parcel ;; without having to use the Groups feature ; simple_build_permissions = false @@ -266,11 +289,14 @@ ; DrawPrimOnMapTile = true ;# {HttpProxy} {} {Proxy URL for llHTTPRequest and dynamic texture loading} {} http://proxy.com:8080 - ;; Http proxy setting for llHTTPRequest and dynamic texture loading, if required + ;; Http proxy setting for llHTTPRequest and dynamic texture loading, if + ;; required ; HttpProxy = "http://proxy.com:8080" ;# {HttpProxyExceptions} {HttpProxy} {Set of regular expressions defining URL that should not be proxied} {} - ;; If you're using HttpProxy, then you can set HttpProxyExceptions to a list of regular expressions for URLs that you don't want to go through the proxy + ;; If you're using HttpProxy, then you can set HttpProxyExceptions to a + ;; list of regular expressions for URLs that you don't want to go through + ;; the proxy. ;; For example, servers inside your firewall. ;; Separate patterns with a ';' ; HttpProxyExceptions = ".mydomain.com;localhost" @@ -289,13 +315,15 @@ ; SpawnPointRouting = closest ;# {TelehubAllowLandmark} {} {Allow users with landmarks to override telehub routing} {true false} false - ;; TelehubAllowLandmark allows users with landmarks to override telehub routing and land at the landmark coordinates when set to true + ;; TelehubAllowLandmark allows users with landmarks to override telehub + ;; routing and land at the landmark coordinates when set to true ;; default is false ; TelehubAllowLandmark = false ;# {AllowedClients} {} {Bar (|) separated list of allowed clients} {} ;; Bar (|) separated list of viewers which may gain access to the regions. - ;; One can use a substring of the viewer name to enable only certain versions + ;; One can use a substring of the viewer name to enable only certain + ;; versions ;; Example: Agent uses the viewer "Imprudence 1.3.2.0" ;; - "Imprudence" has access ;; - "Imprudence 1.3" has access @@ -304,7 +332,8 @@ ;# {BannedClients} {} {Bar (|) separated list of banned clients} {} ;# Bar (|) separated list of viewers which may not gain access to the regions. - ;; One can use a Substring of the viewer name to disable only certain versions + ;; One can use a Substring of the viewer name to disable only certain + ;; versions ;; Example: Agent uses the viewer "Imprudence 1.3.2.0" ;; - "Imprudence" has no access ;; - "Imprudence 1.3" has no access @@ -450,7 +479,8 @@ [SimulatorFeatures] ;# {MapImageServerURI} {} {URL for the map server} {} - ; Experimental new information sent in SimulatorFeatures cap for Kokua viewers + ; Experimental new information sent in SimulatorFeatures cap for Kokua + ; viewers ; meant to override the MapImage and search server url given at login, and varying ; on a sim-basis. ; Viewers that don't understand it, will ignore it From b96a53962b86083476b78498278868ce8152ddd8 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 25 Sep 2012 23:12:28 +0100 Subject: [PATCH 5/6] 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. --- .../Agent/AssetTransaction/AssetXferUploader.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs index 52d7d57986..8d21202541 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( From ddd9384b3901f532243c1e8018334385b84290d1 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 25 Sep 2012 23:30:25 +0100 Subject: [PATCH 6/6] 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. --- .../AssetTransaction/AssetXferUploader.cs | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs index 8d21202541..8add4bb073 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