From 8e08dd20dc9fe785874943c7986f82af60ed1879 Mon Sep 17 00:00:00 2001 From: diva Date: Sat, 18 Apr 2009 16:37:05 +0000 Subject: [PATCH] Thank you dslake for diagnosing and fixing a race condition in OGS1SecureInventoryServer (mantis #3483). The provided patch was slightly modified to narrow the locking scope to smaller portions of the functions. Applied the same locking to HGInventoryService, which suffered from the same race condition. --- .../Hypergrid/HGInventoryService.cs | 170 +++++++++--------- .../OGS1/OGS1SecureInventoryService.cs | 140 ++++++++------- 2 files changed, 162 insertions(+), 148 deletions(-) diff --git a/OpenSim/Region/Communications/Hypergrid/HGInventoryService.cs b/OpenSim/Region/Communications/Hypergrid/HGInventoryService.cs index 156137e12b..98f5f688d3 100644 --- a/OpenSim/Region/Communications/Hypergrid/HGInventoryService.cs +++ b/OpenSim/Region/Communications/Hypergrid/HGInventoryService.cs @@ -77,52 +77,54 @@ namespace OpenSim.Region.Communications.Hypergrid } // grid/hypergrid mode - if (!m_RequestingInventory.ContainsKey(userID)) + lock (m_RequestingInventory) { - m_RequestingInventory.Add(userID, callback); - - string invServer = GetUserInventoryURI(userID); - m_log.InfoFormat( - "[HGrid INVENTORY SERVICE]: Requesting inventory from {0}/GetInventory/ for user {1} ({2})", - /*_inventoryServerUrl*/ invServer, userID, userID.Guid); - - try + if (!m_RequestingInventory.ContainsKey(userID)) { - - //RestSessionObjectPosterResponse requester - // = new RestSessionObjectPosterResponse(); - //requester.ResponseCallback = InventoryResponse; - - //requester.BeginPostObject(invServer + "/GetInventory/", userID.Guid, session_id.ToString(), userID.ToString()); - GetInventoryDelegate d = GetInventoryAsync; - d.BeginInvoke(invServer, userID, session_id, GetInventoryCompleted, d); - + m_RequestingInventory.Add(userID, callback); } - catch (WebException e) + else { - if (StatsManager.SimExtraStats != null) - StatsManager.SimExtraStats.AddInventoryServiceRetrievalFailure(); - - m_log.ErrorFormat("[HGrid INVENTORY SERVICE]: Request inventory operation failed, {0} {1}", - e.Source, e.Message); - - // Well, let's synthesize one - InventoryCollection icol = new InventoryCollection(); - icol.UserID = userID; - icol.Items = new List(); - icol.Folders = new List(); - InventoryFolderBase rootFolder = new InventoryFolderBase(); - rootFolder.ID = UUID.Random(); - rootFolder.Owner = userID; - icol.Folders.Add(rootFolder); - InventoryResponse(icol); + m_log.ErrorFormat("[HGrid INVENTORY SERVICE]: RequestInventoryForUser() - could not find user profile for {0}", userID); + return; } } - else + string invServer = GetUserInventoryURI(userID); + m_log.InfoFormat( + "[HGrid INVENTORY SERVICE]: Requesting inventory from {0}/GetInventory/ for user {1} ({2})", + /*_inventoryServerUrl*/ invServer, userID, userID.Guid); + + try { - m_log.ErrorFormat("[HGrid INVENTORY SERVICE]: RequestInventoryForUser() - could not find user profile for {0}", userID); + + //RestSessionObjectPosterResponse requester + // = new RestSessionObjectPosterResponse(); + //requester.ResponseCallback = InventoryResponse; + + //requester.BeginPostObject(invServer + "/GetInventory/", userID.Guid, session_id.ToString(), userID.ToString()); + GetInventoryDelegate d = GetInventoryAsync; + d.BeginInvoke(invServer, userID, session_id, GetInventoryCompleted, d); + } - + catch (WebException e) + { + if (StatsManager.SimExtraStats != null) + StatsManager.SimExtraStats.AddInventoryServiceRetrievalFailure(); + + m_log.ErrorFormat("[HGrid INVENTORY SERVICE]: Request inventory operation failed, {0} {1}", + e.Source, e.Message); + + // Well, let's synthesize one + InventoryCollection icol = new InventoryCollection(); + icol.UserID = userID; + icol.Items = new List(); + icol.Folders = new List(); + InventoryFolderBase rootFolder = new InventoryFolderBase(); + rootFolder.ID = UUID.Random(); + rootFolder.Owner = userID; + icol.Folders.Add(rootFolder); + InventoryResponse(icol); + } } private delegate InventoryCollection GetInventoryDelegate(string url, UUID userID, UUID sessionID); @@ -447,62 +449,66 @@ namespace OpenSim.Region.Communications.Hypergrid private void InventoryResponse(InventoryCollection response) { UUID userID = response.UserID; - if (m_RequestingInventory.ContainsKey(userID)) + InventoryReceiptCallback callback = null; + lock (m_RequestingInventory) { - m_log.InfoFormat("[HGrid INVENTORY SERVICE]: " + - "Received inventory response for user {0} containing {1} folders and {2} items", - userID, response.Folders.Count, response.Items.Count); - - InventoryFolderImpl rootFolder = null; - InventoryReceiptCallback callback = m_RequestingInventory[userID]; - - ICollection folders = new List(); - ICollection items = new List(); - - foreach (InventoryFolderBase folder in response.Folders) + if (m_RequestingInventory.ContainsKey(userID)) { - if (folder.ParentID == UUID.Zero) - { - rootFolder = new InventoryFolderImpl(folder); - folders.Add(rootFolder); - - break; - } - } - - if (rootFolder != null) - { - foreach (InventoryFolderBase folder in response.Folders) - { - if (folder.ID != rootFolder.ID) - { - folders.Add(new InventoryFolderImpl(folder)); - } - } - - foreach (InventoryItemBase item in response.Items) - { - items.Add(item); - } + m_log.InfoFormat("[HGrid INVENTORY SERVICE]: " + + "Received inventory response for user {0} containing {1} folders and {2} items", + userID, response.Folders.Count, response.Items.Count); + callback = m_RequestingInventory[userID]; + m_RequestingInventory.Remove(userID); } else { - m_log.ErrorFormat("[HGrid INVENTORY SERVICE]: Did not get back an inventory containing a root folder for user {0}", userID); + m_log.WarnFormat( + "[HGrid INVENTORY SERVICE]: " + + "Received inventory response for {0} for which we do not have a record of requesting!", + userID); + return; + } + } + + InventoryFolderImpl rootFolder = null; + + ICollection folders = new List(); + ICollection items = new List(); + + foreach (InventoryFolderBase folder in response.Folders) + { + if (folder.ParentID == UUID.Zero) + { + rootFolder = new InventoryFolderImpl(folder); + folders.Add(rootFolder); + + break; + } + } + + if (rootFolder != null) + { + foreach (InventoryFolderBase folder in response.Folders) + { + if (folder.ID != rootFolder.ID) + { + folders.Add(new InventoryFolderImpl(folder)); + } } - m_RequestingInventory.Remove(userID); - callback(folders, items); - + foreach (InventoryItemBase item in response.Items) + { + items.Add(item); + } } else { - m_log.WarnFormat( - "[HGrid INVENTORY SERVICE]: " + - "Received inventory response for {0} for which we do not have a record of requesting!", - userID); + m_log.ErrorFormat("[HGrid INVENTORY SERVICE]: Did not get back an inventory containing a root folder for user {0}", userID); } - } + callback(folders, items); + + } private bool IsLocalStandaloneUser(UUID userID) { diff --git a/OpenSim/Region/Communications/OGS1/OGS1SecureInventoryService.cs b/OpenSim/Region/Communications/OGS1/OGS1SecureInventoryService.cs index 03fb0d5676..e24f820e3f 100644 --- a/OpenSim/Region/Communications/OGS1/OGS1SecureInventoryService.cs +++ b/OpenSim/Region/Communications/OGS1/OGS1SecureInventoryService.cs @@ -69,34 +69,36 @@ namespace OpenSim.Region.Communications.OGS1 /// public void RequestInventoryForUser(UUID userID, UUID session_id, InventoryReceiptCallback callback) { - if (!m_RequestingInventory.ContainsKey(userID)) + lock (m_RequestingInventory) { - m_RequestingInventory.Add(userID, callback); - - try + if (!m_RequestingInventory.ContainsKey(userID)) + m_RequestingInventory.Add(userID, callback); + else { - m_log.InfoFormat( - "[OGS1 INVENTORY SERVICE]: Requesting inventory from {0}/GetInventory/ for user {1}", - _inventoryServerUrl, userID); - - RestSessionObjectPosterResponse requester - = new RestSessionObjectPosterResponse(); - requester.ResponseCallback = InventoryResponse; - - requester.BeginPostObject(_inventoryServerUrl + "/GetInventory/", userID.Guid, session_id.ToString(), userID.ToString()); - } - catch (WebException e) - { - if (StatsManager.SimExtraStats != null) - StatsManager.SimExtraStats.AddInventoryServiceRetrievalFailure(); - - m_log.ErrorFormat("[OGS1 INVENTORY SERVICE]: Request inventory operation failed, {0} {1}", - e.Source, e.Message); + m_log.ErrorFormat("[OGS1 INVENTORY SERVICE]: RequestInventoryForUser() - could you not find user profile for {0}", userID); + return; } } - else + + try { - m_log.ErrorFormat("[OGS1 INVENTORY SERVICE]: RequestInventoryForUser() - could you not find user profile for {0}", userID); + m_log.InfoFormat( + "[OGS1 INVENTORY SERVICE]: Requesting inventory from {0}/GetInventory/ for user {1}", + _inventoryServerUrl, userID); + + RestSessionObjectPosterResponse requester + = new RestSessionObjectPosterResponse(); + requester.ResponseCallback = InventoryResponse; + + requester.BeginPostObject(_inventoryServerUrl + "/GetInventory/", userID.Guid, session_id.ToString(), userID.ToString()); + } + catch (WebException e) + { + if (StatsManager.SimExtraStats != null) + StatsManager.SimExtraStats.AddInventoryServiceRetrievalFailure(); + + m_log.ErrorFormat("[OGS1 INVENTORY SERVICE]: Request inventory operation failed, {0} {1}", + e.Source, e.Message); } } @@ -107,60 +109,66 @@ namespace OpenSim.Region.Communications.OGS1 private void InventoryResponse(InventoryCollection response) { UUID userID = response.UserID; - if (m_RequestingInventory.ContainsKey(userID)) + InventoryReceiptCallback callback = null; + lock (m_RequestingInventory) { - m_log.InfoFormat("[OGS1 INVENTORY SERVICE]: " + - "Received inventory response for user {0} containing {1} folders and {2} items", - userID, response.Folders.Count, response.Items.Count); - - InventoryFolderImpl rootFolder = null; - InventoryReceiptCallback callback = m_RequestingInventory[userID]; - - ICollection folders = new List(); - ICollection items = new List(); - - foreach (InventoryFolderBase folder in response.Folders) + if (m_RequestingInventory.ContainsKey(userID)) { - if (folder.ParentID == UUID.Zero) - { - rootFolder = new InventoryFolderImpl(folder); - folders.Add(rootFolder); - - break; - } - } - - if (rootFolder != null) - { - foreach (InventoryFolderBase folder in response.Folders) - { - if (folder.ID != rootFolder.ID) - { - folders.Add(new InventoryFolderImpl(folder)); - } - } - - foreach (InventoryItemBase item in response.Items) - { - items.Add(item); - } + callback = m_RequestingInventory[userID]; + m_RequestingInventory.Remove(userID); } else { - m_log.ErrorFormat("[OGS1 INVENTORY SERVICE]: Did not get back an inventory containing a root folder for user {0}", userID); + m_log.WarnFormat( + "[OGS1 INVENTORY SERVICE]: " + + "Received inventory response for {0} for which we do not have a record of requesting!", + userID); + return; + } + } + + m_log.InfoFormat("[OGS1 INVENTORY SERVICE]: " + + "Received inventory response for user {0} containing {1} folders and {2} items", + userID, response.Folders.Count, response.Items.Count); + + InventoryFolderImpl rootFolder = null; + + ICollection folders = new List(); + ICollection items = new List(); + + foreach (InventoryFolderBase folder in response.Folders) + { + if (folder.ParentID == UUID.Zero) + { + rootFolder = new InventoryFolderImpl(folder); + folders.Add(rootFolder); + + break; + } + } + + if (rootFolder != null) + { + foreach (InventoryFolderBase folder in response.Folders) + { + if (folder.ID != rootFolder.ID) + { + folders.Add(new InventoryFolderImpl(folder)); + } } - callback(folders, items); - - m_RequestingInventory.Remove(userID); + foreach (InventoryItemBase item in response.Items) + { + items.Add(item); + } } else { - m_log.WarnFormat( - "[OGS1 INVENTORY SERVICE]: " + - "Received inventory response for {0} for which we do not have a record of requesting!", - userID); + m_log.ErrorFormat("[OGS1 INVENTORY SERVICE]: Did not get back an inventory containing a root folder for user {0}", userID); } + + callback(folders, items); + } ///