3 files changed, 238 insertions, 82 deletions

diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs
index 9d8082b..eed7cd5 100644
--- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs
+++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs
@@ -41,14 +41,13 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
     /// </summary>
     public class AgentAssetTransactions
     {
-//        private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
+        private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
 
         // Fields
         private bool m_dumpAssetsToFile;
         private Scene m_Scene;
-        public UUID UserID;
-        public Dictionary<UUID, AssetXferUploader> XferUploaders =
-                new Dictionary<UUID, AssetXferUploader>();
+        private UUID UserID;
+        private Dictionary<UUID, AssetXferUploader> XferUploaders = new Dictionary<UUID, AssetXferUploader>();
 
         // Methods
         public AgentAssetTransactions(UUID agentID, Scene scene,
@@ -59,36 +58,94 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
             m_dumpAssetsToFile = dumpAssetsToFile;
         }
 
-        public AssetXferUploader RequestXferUploader(UUID transactionID)
+        /// <summary>
+        /// Return a xfer uploader if one does not already exist.
+        /// </summary>
+        /// <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)
         {
-            if (!XferUploaders.ContainsKey(transactionID))
+            lock (XferUploaders)
             {
-                AssetXferUploader uploader = new AssetXferUploader(m_Scene,
-                        m_dumpAssetsToFile);
-
-                lock (XferUploaders)
+                if (!XferUploaders.ContainsKey(transactionID))
                 {
+                    AssetXferUploader uploader = new AssetXferUploader(this, m_Scene, assetID, 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;
+                    return uploader;
+                }
             }
+
+            m_log.WarnFormat("[AGENT ASSETS TRANSACTIONS]: Ignoring request for asset xfer uploader {0} since it already exists", transactionID);
+
             return null;
         }
 
         public void HandleXfer(ulong xferID, uint packetID, byte[] data)
         {
+            AssetXferUploader foundUploader = null;
+
             lock (XferUploaders)
             {
                 foreach (AssetXferUploader uploader in XferUploaders.Values)
                 {
+//                    m_log.DebugFormat(
+//                        "[AGENT ASSETS TRANSACTIONS]: In HandleXfer, inspect xfer upload with xfer id {0}",
+//                        uploader.XferID);
+
                     if (uploader.XferID == xferID)
                     {
-                        uploader.HandleXferPacket(xferID, packetID, data);
+                        foundUploader = uploader;
                         break;
                     }
                 }
             }
+
+            if (foundUploader != null)
+            {
+//                m_log.DebugFormat(
+//                    "[AGENT ASSETS TRANSACTIONS]: Found xfer uploader for xfer id {0}, packet id {1}, data length {2}",
+//                    xferID, packetID, data.Length);
+
+                foundUploader.HandleXferPacket(xferID, packetID, data);
+            }
+            else
+            {
+                m_log.ErrorFormat(
+                    "[AGENT ASSET TRANSACTIONS]: Could not find uploader for xfer id {0}, packet id {1}, data length {2}",
+                    xferID, packetID, data.Length);
+            }
+        }
+
+        public bool RemoveXferUploader(UUID transactionID)
+        {
+            lock (XferUploaders)
+            {
+                bool removed = XferUploaders.Remove(transactionID);
+
+                if (!removed)
+                    m_log.WarnFormat(
+                        "[AGENT ASSET TRANSACTIONS]: Received request to remove xfer uploader with transaction ID {0} but none found",
+                        transactionID);
+//                else
+//                    m_log.DebugFormat(
+//                        "[AGENT ASSET TRANSACTIONS]: Removed xfer uploader with transaction ID {0}", transactionID);
+
+                return removed;
+            }
         }
 
         public void RequestCreateInventoryItem(IClientAPI remoteClient,
@@ -96,16 +153,24 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
                 string description, string name, sbyte invType,
                sbyte type, byte wearableType, uint nextOwnerMask)
         {
-            if (XferUploaders.ContainsKey(transactionID))
+            AssetXferUploader uploader = null;
+
+            lock (XferUploaders)
             {
-                XferUploaders[transactionID].RequestCreateInventoryItem(
-                        remoteClient, transactionID, folderID,
-                        callbackID, description, name, invType, type,
-                        wearableType, nextOwnerMask);
+                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);
+        }
 
         /// <summary>
         /// Get an uploaded asset. If the data is successfully retrieved,
@@ -113,19 +178,18 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
         /// </summary>
         /// <param name="transactionID"></param>
         /// <returns>The asset if the upload has completed, null if it has not.</returns>
-        public AssetBase GetTransactionAsset(UUID transactionID)
+        private AssetBase GetTransactionAsset(UUID transactionID)
         {
-            if (XferUploaders.ContainsKey(transactionID))
+            lock (XferUploaders)
             {
-                AssetXferUploader uploader = XferUploaders[transactionID];
-                AssetBase asset = uploader.GetAssetData();
-
-                lock (XferUploaders)
+                if (XferUploaders.ContainsKey(transactionID))
                 {
-                    XferUploaders.Remove(transactionID);
-                }
+                    AssetXferUploader uploader = XferUploaders[transactionID];
+                    AssetBase asset = uploader.GetAssetData();
+                    RemoveXferUploader(transactionID);
 
-                return asset;
+                    return asset;
+                }
             }
 
             return null;
@@ -135,7 +199,15 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
                 SceneObjectPart part, UUID transactionID,
                 TaskInventoryItem item)
         {
-            if (XferUploaders.ContainsKey(transactionID))
+            AssetXferUploader uploader = null;
+
+            lock (XferUploaders)
+            {
+                if (XferUploaders.ContainsKey(transactionID))
+                    uploader = XferUploaders[transactionID];
+            }
+
+            if (uploader != null)
             {
                 AssetBase asset = GetTransactionAsset(transactionID);
 
@@ -161,28 +233,34 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
                     m_Scene.AssetService.Store(asset);
                 }
             }
+            else
+            {
+                m_log.ErrorFormat(
+                    "[AGENT ASSET TRANSACTIONS]: Could not find uploader with transaction ID {0} when handling request to update task inventory item {1} in {2}",
+                    transactionID, item.Name, part.Name);
+            }
         }
 
         public void RequestUpdateInventoryItem(IClientAPI remoteClient,
                 UUID transactionID, InventoryItemBase item)
         {
-            if (XferUploaders.ContainsKey(transactionID))
-            {
-                AssetBase asset = GetTransactionAsset(transactionID);
+            AssetXferUploader uploader = null;
 
-                if (asset != null)
-                {
-                    asset.FullID = UUID.Random();
-                    asset.Name = item.Name;
-                    asset.Description = item.Description;
-                    asset.Type = (sbyte)item.AssetType;
-                    item.AssetID = asset.FullID;
-
-                    m_Scene.AssetService.Store(asset);
+            lock (XferUploaders)
+            {
+                if (XferUploaders.ContainsKey(transactionID))
+                    uploader = XferUploaders[transactionID];
+            }
 
-                    IInventoryService invService = m_Scene.InventoryService;
-                    invService.UpdateItem(item);
-                }
+            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);
             }
         }
     }
diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs
index 82558de..a28d5d7 100644
--- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs
+++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetTransactionModule.cs
@@ -172,11 +172,12 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
         /// <summary>
         /// Update an inventory item with data that has been received through a
         /// transaction.
-        ///
+        /// </summary>
+        /// <remarks>
         /// This is called when clothing or body parts are updated (for
         /// instance, with new textures or colours). It may also be called in
         /// other situations.
-        /// </summary>
+        /// </remarks>
         /// <param name="remoteClient"></param>
         /// <param name="transactionID"></param>
         /// <param name="item"></param>
@@ -184,14 +185,12 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
                 UUID transactionID, InventoryItemBase item)
         {
 //            m_log.DebugFormat(
-//                "[TRANSACTIONS MANAGER] Called HandleItemUpdateFromTransaction with item {0}",
+//                "[ASSET TRANSACTION MODULE]: Called HandleItemUpdateFromTransaction with item {0}",
 //                item.Name);
 
-            AgentAssetTransactions transactions =
-                    GetUserTransactions(remoteClient.AgentId);
+            AgentAssetTransactions transactions = GetUserTransactions(remoteClient.AgentId);
 
-            transactions.RequestUpdateInventoryItem(remoteClient,
-                    transactionID, item);
+            transactions.RequestUpdateInventoryItem(remoteClient, transactionID, item);
         }
 
         /// <summary>
@@ -255,11 +254,8 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
                 }
             }
 
-            AgentAssetTransactions transactions =
-                    GetUserTransactions(remoteClient.AgentId);
-
-            AssetXferUploader uploader =
-                    transactions.RequestXferUploader(transaction);
+            AgentAssetTransactions transactions = GetUserTransactions(remoteClient.AgentId);
+            AssetXferUploader uploader = transactions.RequestXferUploader(transaction, assetID);
 
             if (uploader != null)
             {
@@ -279,9 +275,8 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
         public void HandleXfer(IClientAPI remoteClient, ulong xferID,
                 uint packetID, byte[] data)
         {
-            //m_log.Debug("xferID: " + xferID + "  packetID: " + packetID + "  data!");
-            AgentAssetTransactions transactions =
-                    GetUserTransactions(remoteClient.AgentId);
+//            m_log.Debug("xferID: " + xferID + "  packetID: " + packetID + "  data length " + data.Length);
+            AgentAssetTransactions transactions = GetUserTransactions(remoteClient.AgentId);
 
             transactions.HandleXfer(xferID, packetID, data);
         }
diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs
index a7929ba..ec4dfd0 100644
--- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs
+++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AssetXferUploader.cs
@@ -40,11 +40,21 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
     {
         private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
 
+        /// <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 AssetBase m_asset;
         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 InventoryItemBase m_updateItemData;
+
         private string m_description = String.Empty;
         private bool m_dumpAssetToFile;
         private bool m_finished = false;
@@ -58,9 +68,11 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
         public ulong XferID;
         private Scene m_Scene;
 
-        public AssetXferUploader(Scene scene, bool dumpAssetToFile)
+        public AssetXferUploader(AgentAssetTransactions transactions, Scene scene, UUID assetID, bool dumpAssetToFile)
         {
+            m_transactions = transactions;
             m_Scene = scene;
+            m_asset = new AssetBase() { FullID = assetID };
             m_dumpAssetToFile = dumpAssetToFile;
         }
 
@@ -73,6 +85,10 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
         /// <returns>True if the transfer is complete, false otherwise or if the xferID was not valid</returns>
         public bool HandleXferPacket(ulong xferID, uint packetID, byte[] data)
         {
+//            m_log.DebugFormat(
+//                "[ASSET XFER UPLOADER]: Received packet {0} for xfer {1} (data length {2})",
+//                packetID, xferID, data.Length);
+
             if (XferID == xferID)
             {
                 if (m_asset.Data.Length > 1)
@@ -107,16 +123,20 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
         /// <param name="xferID"></param>
         /// <param name="packetID"></param>
         /// <param name="data"></param>
-        /// <returns>True if the transfer is complete, false otherwise</returns>
-        public bool Initialise(IClientAPI remoteClient, UUID assetID,
+        public void Initialise(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);
+
             ourClient = remoteClient;
-            m_asset = new AssetBase(assetID, "blank", type,
-                    remoteClient.AgentId.ToString());
-            m_asset.Data = data;
+            m_asset.Name = "blank";
             m_asset.Description = "empty";
+            m_asset.Type = type;
+            m_asset.CreatorID = remoteClient.AgentId.ToString();
+            m_asset.Data = data;
             m_asset.Local = storeLocal;
             m_asset.Temporary = tempFile;
 
@@ -126,21 +146,22 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
             if (m_asset.Data.Length > 2)
             {
                 SendCompleteMessage();
-                return true;
             }
             else
             {
                 RequestStartXfer();
             }
-
-            return false;
         }
 
         protected void RequestStartXfer()
         {
             XferID = Util.GetNextXferID();
-            ourClient.SendXferRequest(XferID, m_asset.Type, m_asset.FullID,
-                    0, new byte[0]);
+
+//            m_log.DebugFormat(
+//                "[ASSET XFER UPLOADER]: Requesting Xfer of asset {0}, type {1}, transfer id {2} from {3}",
+//                m_asset.FullID, m_asset.Type, XferID, ourClient.Name);
+
+            ourClient.SendXferRequest(XferID, m_asset.Type, m_asset.FullID, 0, new byte[0]);
         }
 
         protected void SendCompleteMessage()
@@ -148,18 +169,32 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
             ourClient.SendAssetUploadCompleteMessage(m_asset.Type, true,
                     m_asset.FullID);
 
-            m_finished = true;
-            if (m_createItem)
-            {
-                DoCreateItem(m_createItemCallback);
-            }
-            else if (m_storeLocal)
+            // 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_Scene.AssetService.Store(m_asset);
+                m_finished = true;
+                if (m_createItem)
+                {
+                    DoCreateItem(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(TransactionID);
+                }
+                else if (m_storeLocal)
+                {
+                    m_Scene.AssetService.Store(m_asset);
+                }
             }
 
             m_log.DebugFormat(
-                "[ASSET TRANSACTIONS]: Uploaded asset {0} for transaction {1}",
+                "[ASSET XFER UPLOADER]: Uploaded asset {0} for transaction {1}",
                 m_asset.FullID, TransactionID);
 
             if (m_dumpAssetToFile)
@@ -205,18 +240,66 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction
                 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_finished)
+                    {
+                        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)
+        {
+            // We must lock to avoid a race with a separate thread uploading the asset.
+            lock (this)
+            {
+                m_asset.Name = item.Name;
+                m_asset.Description = item.Description;
+                m_asset.Type = (sbyte)item.AssetType;
+
+                // We must always store the item at this point even if the asset hasn't finished uploading, in order
+                // to avoid a race condition when the appearance module retrieves the item to set the asset id in
+                // the AvatarAppearance structure.
+                item.AssetID = m_asset.FullID;
+                m_Scene.InventoryService.UpdateItem(item);
+
                 if (m_finished)
                 {
-                    DoCreateItem(callbackID);
+                    StoreAssetForItemUpdate(item);
                 }
                 else
                 {
-                    m_createItem = true; //set flag so the inventory item is created when upload is complete
-                    m_createItemCallback = callbackID;
+//                    m_log.DebugFormat(
+//                        "[ASSET XFER UPLOADER]: Holding update inventory item request {0} for {1} pending completion of asset xfer for transaction {2}",
+//                        item.Name, remoteClient.Name, transactionID);
+    
+                    m_updateItem = true;
+                    m_updateItemData = item;
                 }
             }
         }
 
+        /// <summary>
+        /// Store the asset for the given item.
+        /// </summary>
+        /// <param name="item"></param>
+        private void StoreAssetForItemUpdate(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);
+        }
+
         private void DoCreateItem(uint callbackID)
         {
             m_Scene.AssetService.Store(m_asset);