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)0.7.4-extended
							parent
							
								
									15aef01b34
								
							
						
					
					
						commit
						5629f5141e
					
				|  | @ -57,39 +57,36 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction | |||
|         } | ||||
| 
 | ||||
|         /// <summary> | ||||
|         /// Return a xfer uploader if one does not already exist. | ||||
|         /// Return the xfer uploader for the given transaction. | ||||
|         /// </summary> | ||||
|         /// <remarks> | ||||
|         /// If an uploader does not already exist for this transaction then it is created, otherwise the existing | ||||
|         /// uploader is returned. | ||||
|         /// </remarks> | ||||
|         /// <param name="transactionID"></param> | ||||
|         /// <param name="assetID"> | ||||
|         /// We must transfer the new asset ID into the uploader on creation, otherwise | ||||
|         /// we can see race conditions with other threads which can retrieve an item before it is updated with the new | ||||
|         /// asset id. | ||||
|         /// </param> | ||||
|         /// <returns> | ||||
|         /// The xfer uploader requested.  Null if one is already in existence. | ||||
|         /// FIXME: This is a bizarre thing to do, and is probably meant to signal an error condition if multiple | ||||
|         /// transfers are made.  Needs to be corrected. | ||||
|         /// </returns> | ||||
|         public AssetXferUploader RequestXferUploader(UUID transactionID, UUID assetID) | ||||
|         /// <returns>The asset xfer uploader</returns> | ||||
|         public AssetXferUploader RequestXferUploader(UUID transactionID) | ||||
|         { | ||||
|             AssetXferUploader uploader; | ||||
| 
 | ||||
|             lock (XferUploaders) | ||||
|             { | ||||
|                 if (!XferUploaders.ContainsKey(transactionID)) | ||||
|                 { | ||||
|                     AssetXferUploader uploader = new AssetXferUploader(this, m_Scene, assetID, m_dumpAssetsToFile); | ||||
|                     uploader = new AssetXferUploader(this, m_Scene, m_dumpAssetsToFile); | ||||
| 
 | ||||
| //                    m_log.DebugFormat( | ||||
| //                        "[AGENT ASSETS TRANSACTIONS]: Adding asset xfer uploader {0} since it didn't previously exist", transactionID); | ||||
| 
 | ||||
|                     XferUploaders.Add(transactionID, uploader); | ||||
| 
 | ||||
|                     return uploader; | ||||
|                 } | ||||
|                 else | ||||
|                 { | ||||
|                     uploader = XferUploaders[transactionID]; | ||||
|                 } | ||||
|             } | ||||
| 
 | ||||
|             m_log.WarnFormat("[AGENT ASSETS TRANSACTIONS]: Ignoring request for asset xfer uploader {0} since it already exists", transactionID); | ||||
| 
 | ||||
|             return null; | ||||
|             return uploader; | ||||
|         } | ||||
| 
 | ||||
|         public void HandleXfer(ulong xferID, uint packetID, byte[] data) | ||||
|  | @ -151,23 +148,11 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction | |||
|                 string description, string name, sbyte invType, | ||||
|                sbyte type, byte wearableType, uint nextOwnerMask) | ||||
|         { | ||||
|             AssetXferUploader uploader = null; | ||||
|             AssetXferUploader uploader = RequestXferUploader(transactionID); | ||||
| 
 | ||||
|             lock (XferUploaders) | ||||
|             { | ||||
|                 if (XferUploaders.ContainsKey(transactionID)) | ||||
|                     uploader = XferUploaders[transactionID]; | ||||
|             } | ||||
| 
 | ||||
|             if (uploader != null) | ||||
|                 uploader.RequestCreateInventoryItem( | ||||
|                     remoteClient, transactionID, folderID, | ||||
|                     callbackID, description, name, invType, type, | ||||
|                     wearableType, nextOwnerMask); | ||||
|             else | ||||
|                 m_log.ErrorFormat( | ||||
|                     "[AGENT ASSET TRANSACTIONS]: Could not find uploader with transaction ID {0} when handling request to create inventory item {1} from {2}", | ||||
|                     transactionID, name, remoteClient.Name); | ||||
|             uploader.RequestCreateInventoryItem( | ||||
|                 remoteClient, transactionID, folderID, callbackID, | ||||
|                 description, name, invType, type, wearableType, nextOwnerMask); | ||||
|         } | ||||
| 
 | ||||
|         /// <summary> | ||||
|  | @ -197,69 +182,37 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction | |||
|                 SceneObjectPart part, UUID transactionID, | ||||
|                 TaskInventoryItem item) | ||||
|         { | ||||
|             AssetXferUploader uploader = null; | ||||
|             AssetBase asset = GetTransactionAsset(transactionID); | ||||
| 
 | ||||
|             lock (XferUploaders) | ||||
|             // Only legacy viewers use this, and they prefer CAPS, which | ||||
|             // we have, so this really never runs. | ||||
|             // Allow it, but only for "safe" types. | ||||
|             if ((InventoryType)item.InvType != InventoryType.Notecard && | ||||
|                 (InventoryType)item.InvType != InventoryType.LSL) | ||||
|                 return; | ||||
| 
 | ||||
|             if (asset != null) | ||||
|             { | ||||
|                 if (XferUploaders.ContainsKey(transactionID)) | ||||
|                     uploader = XferUploaders[transactionID]; | ||||
|             } | ||||
| 
 | ||||
|             if (uploader != null) | ||||
|             { | ||||
|                 AssetBase asset = GetTransactionAsset(transactionID); | ||||
| 
 | ||||
|                 // Only legacy viewers use this, and they prefer CAPS, which  | ||||
|                 // we have, so this really never runs. | ||||
|                 // Allow it, but only for "safe" types. | ||||
|                 if ((InventoryType)item.InvType != InventoryType.Notecard && | ||||
|                     (InventoryType)item.InvType != InventoryType.LSL) | ||||
|                     return; | ||||
| 
 | ||||
|                 if (asset != null) | ||||
|                 { | ||||
| //                    m_log.DebugFormat( | ||||
| //                        "[AGENT ASSETS TRANSACTIONS]: Updating item {0} in {1} for transaction {2}",  | ||||
| //                        item.Name, part.Name, transactionID); | ||||
|                      | ||||
|                     asset.FullID = UUID.Random(); | ||||
|                     asset.Name = item.Name; | ||||
|                     asset.Description = item.Description; | ||||
|                     asset.Type = (sbyte)item.Type; | ||||
|                     item.AssetID = asset.FullID; | ||||
|                  | ||||
|                 asset.FullID = UUID.Random(); | ||||
|                 asset.Name = item.Name; | ||||
|                 asset.Description = item.Description; | ||||
|                 asset.Type = (sbyte)item.Type; | ||||
|                 item.AssetID = asset.FullID; | ||||
| 
 | ||||
|                     m_Scene.AssetService.Store(asset); | ||||
|                 } | ||||
|             } | ||||
|             else | ||||
|             { | ||||
|                 m_log.ErrorFormat( | ||||
|                     "[AGENT ASSET TRANSACTIONS]: Could not find uploader with transaction ID {0} when handling request to update task inventory item {1} in {2}", | ||||
|                     transactionID, item.Name, part.Name); | ||||
|                 m_Scene.AssetService.Store(asset); | ||||
|             } | ||||
|         } | ||||
| 
 | ||||
|         public void RequestUpdateInventoryItem(IClientAPI remoteClient, | ||||
|                 UUID transactionID, InventoryItemBase item) | ||||
|         { | ||||
|             AssetXferUploader uploader = null; | ||||
|             AssetXferUploader uploader = RequestXferUploader(transactionID); | ||||
| 
 | ||||
|             lock (XferUploaders) | ||||
|             { | ||||
|                 if (XferUploaders.ContainsKey(transactionID)) | ||||
|                     uploader = XferUploaders[transactionID]; | ||||
|             } | ||||
| 
 | ||||
|             if (uploader != null) | ||||
|             { | ||||
|                 uploader.RequestUpdateInventoryItem(remoteClient, transactionID, item); | ||||
|             } | ||||
|             else | ||||
|             { | ||||
|                 m_log.ErrorFormat( | ||||
|                     "[AGENT ASSET TRANSACTIONS]: Could not find uploader with transaction ID {0} when handling request to update inventory item {1} for {2}", | ||||
|                     transactionID, item.Name, remoteClient.Name); | ||||
|             } | ||||
|             uploader.RequestUpdateInventoryItem(remoteClient, transactionID, item); | ||||
|         } | ||||
|     } | ||||
| } | ||||
| } | ||||
|  | @ -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> | ||||
|  |  | |||
|  | @ -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, | ||||
|                 bool tempFile) | ||||
|         /// <param name="remoteClient"></param> | ||||
|         /// <param name="assetID"></param> | ||||
|         /// <param name="transaction"></param> | ||||
|         /// <param name="type"></param> | ||||
|         /// <param name="data"> | ||||
|         /// Optional data.  If present then the asset is created immediately with this data | ||||
|         /// rather than requesting an upload from the client.  The data must be longer than 2 bytes. | ||||
|         /// </param> | ||||
|         /// <param name="storeLocal"></param> | ||||
|         /// <param name="tempFile"></param> | ||||
|         public void StartUpload( | ||||
|             IClientAPI remoteClient, UUID assetID, UUID transaction, sbyte type, byte[] data, bool storeLocal, | ||||
|             bool tempFile) | ||||
|         { | ||||
| //            m_log.DebugFormat( | ||||
| //                "[ASSET XFER UPLOADER]: Initialised xfer from {0}, asset {1}, transaction {2}, type {3}, storeLocal {4}, tempFile {5}, already received data length {6}", | ||||
| //                remoteClient.Name, assetID, transaction, type, storeLocal, tempFile, data.Length); | ||||
| 
 | ||||
|             lock (this) | ||||
|             { | ||||
|                 if (m_uploadState != UploadState.New) | ||||
|                 { | ||||
|                     m_log.WarnFormat( | ||||
|                         "[ASSET XFER UPLOADER]: Tried to start upload of asset {0}, transaction {1} for {2} but this is already in state {3}.  Aborting.", | ||||
|                         assetID, transaction, remoteClient.Name, m_uploadState); | ||||
| 
 | ||||
|                     return; | ||||
|                 } | ||||
| 
 | ||||
|                 m_uploadState = UploadState.Uploading; | ||||
|             } | ||||
| 
 | ||||
|             ourClient = remoteClient; | ||||
| 
 | ||||
|             m_asset = new AssetBase() { FullID = assetID }; | ||||
|             m_asset.Name = "blank"; | ||||
|             m_asset.Description = "empty"; | ||||
|             m_asset.Type = type; | ||||
|  | @ -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; | ||||
|             } | ||||
|  | @ -342,4 +378,4 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction | |||
|             return null; | ||||
|         } | ||||
|     } | ||||
| } | ||||
| } | ||||
		Loading…
	
		Reference in New Issue
	
	 Justin Clark-Casey (justincc)
						Justin Clark-Casey (justincc)