SL-14037 BugSplat Crash #646590: Enqueue failed in AIS

4 files changed, 78 insertions, 15 deletions

diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp
index 26684a4d9e..a4fe3a2a8e 100644
--- a/indra/llmessage/llcoproceduremanager.cpp
+++ b/indra/llmessage/llcoproceduremanager.cpp
@@ -360,7 +360,7 @@ LLUUID LLCoprocedurePool::enqueueCoprocedure(const std::string &name, LLCoproced
     }
 
     // The queue should never fill up.
-    LL_ERRS("CoProcMgr") << "Enqueue failed (" << unsigned(pushed) << ")" << LL_ENDL;
+    LL_ERRS("CoProcMgr") << "Enqueue into '" << name << "' failed (" << unsigned(pushed) << ")" << LL_ENDL;
     return {};                      // never executed, pacify the compiler
 }
 
diff --git a/indra/newview/llaisapi.cpp b/indra/newview/llaisapi.cpp
index ee49125711..005259bcb8 100644
--- a/indra/newview/llaisapi.cpp
+++ b/indra/newview/llaisapi.cpp
@@ -44,6 +44,10 @@
 const std::string AISAPI::INVENTORY_CAP_NAME("InventoryAPIv3");
 const std::string AISAPI::LIBRARY_CAP_NAME("LibraryAPIv3");
 
+std::list<AISAPI::ais_query_item_t> AISAPI::sPostponedQuery;
+
+const S32 MAX_SIMULTANEOUS_COROUTINES = 2048;
+
 //-------------------------------------------------------------------------
 /*static*/
 bool AISAPI::isAvailable()
@@ -366,9 +370,51 @@ void AISAPI::UpdateItem(const LLUUID &itemId, const LLSD &updates, completion_t
 /*static*/
 void AISAPI::EnqueueAISCommand(const std::string &procName, LLCoprocedureManager::CoProcedure_t proc)
 {
+    LLCoprocedureManager &inst = LLCoprocedureManager::instance();
+    S32 pending_in_pool = inst.countPending("AIS");
     std::string procFullName = "AIS(" + procName + ")";
-    LLCoprocedureManager::instance().enqueueCoprocedure("AIS", procFullName, proc);
+    if (pending_in_pool < MAX_SIMULTANEOUS_COROUTINES)
+    {
+        inst.enqueueCoprocedure("AIS", procFullName, proc);
+    }
+    else
+    {
+        // As I understand it, coroutines have built-in 'pending' pool
+        // but unfortunately it has limited size which inventory often goes over
+        // so this is a workaround to not overfill it.
+        if (sPostponedQuery.empty())
+        {
+            sPostponedQuery.push_back(ais_query_item_t(procFullName, proc));
+            gIdleCallbacks.addFunction(onIdle, NULL);
+        }
+        else
+        {
+            sPostponedQuery.push_back(ais_query_item_t(procFullName, proc));
+        }
+    }
+}
 
+/*static*/
+void AISAPI::onIdle(void *userdata)
+{
+    if (!sPostponedQuery.empty())
+    {
+        LLCoprocedureManager &inst = LLCoprocedureManager::instance();
+        S32 pending_in_pool = inst.countPending("AIS");
+        while (pending_in_pool < MAX_SIMULTANEOUS_COROUTINES && !sPostponedQuery.empty())
+        {
+            ais_query_item_t &item = sPostponedQuery.front();
+            inst.enqueueCoprocedure("AIS", item.first, item.second);
+            sPostponedQuery.pop_front();
+            pending_in_pool++;
+        }
+    }
+    
+    if (sPostponedQuery.empty())
+    {
+        // Nothing to do anymore
+        gIdleCallbacks.deleteFunction(onIdle, NULL);
+    }
 }
 
 /*static*/
diff --git a/indra/newview/llaisapi.h b/indra/newview/llaisapi.h
index fc1a6c0871..856f3fc180 100644
--- a/indra/newview/llaisapi.h
+++ b/indra/newview/llaisapi.h
@@ -71,6 +71,7 @@ private:
         const std::string, LLSD, LLCore::HttpOptions::ptr_t, LLCore::HttpHeaders::ptr_t) > invokationFn_t;
 
     static void EnqueueAISCommand(const std::string &procName, LLCoprocedureManager::CoProcedure_t proc);
+    static void onIdle(void *userdata); // launches postponed AIS commands
 
     static std::string getInvCap();
     static std::string getLibCap();
@@ -79,6 +80,8 @@ private:
         invokationFn_t invoke, std::string url, LLUUID targetId, LLSD body, 
         completion_t callback, COMMAND_TYPE type);
 
+    typedef std::pair<std::string, LLCoprocedureManager::CoProcedure_t> ais_query_item_t;
+    static std::list<ais_query_item_t> sPostponedQuery;
 };
 
 class AISUpdate
diff --git a/indra/newview/lllandmarklist.cpp b/indra/newview/lllandmarklist.cpp
index 1fc70cd6d6..b4236c406b 100644
--- a/indra/newview/lllandmarklist.cpp
+++ b/indra/newview/lllandmarklist.cpp
@@ -40,8 +40,8 @@
 LLLandmarkList gLandmarkList;
 
 // number is mostly arbitrary, but it should be below DEFAULT_QUEUE_SIZE pool size,
-// which is 4096, to not overfill the pool if user has more than 4K of landmarks,
-// and low number helps with not flooding server with requests
+// which is 4096, to not overfill the pool if user has more than 4K of landmarks
+// and it should leave some space for other potential simultaneous asset request
 const S32 MAX_SIMULTANEOUS_REQUESTS = 512;
 
 
@@ -98,7 +98,11 @@ LLLandmark* LLLandmarkList::getAsset(const LLUUID& asset_uuid, loaded_callback_t
 
         if (mRequestedList.size() > MAX_SIMULTANEOUS_REQUESTS)
         {
-            // Postpone download till queu is emptier
+            // Workarounds for corutines pending list size limit:
+            // Postpone download till queue is emptier.
+            // Coroutines have own built in 'pending' list, but unfortunately
+            // it is too small compared to potential amount of landmarks
+            // or assets.
             mWaitList.insert(asset_uuid);
             return NULL;
         }
@@ -176,17 +180,27 @@ void LLLandmarkList::processGetAssetReply(
         // todo: this should clean mLoadedCallbackMap!
 	}
 
-    if (!gLandmarkList.mWaitList.empty())
+    // getAssetData can fire callback immediately, causing
+    // a recursion which is suboptimal for very large wait list.
+    // 'scheduling' indicates that we are inside request and
+    // shouldn't be launching more requests.
+    static bool scheduling = false;
+    if (!scheduling && !gLandmarkList.mWaitList.empty())
     {
-        // start new download from wait list
-        landmark_uuid_list_t::iterator iter = gLandmarkList.mWaitList.begin();
-        LLUUID asset_uuid = *iter;
-        gLandmarkList.mWaitList.erase(iter);
-        gAssetStorage->getAssetData(asset_uuid,
-            LLAssetType::AT_LANDMARK,
-            LLLandmarkList::processGetAssetReply,
-            NULL);
-        gLandmarkList.mRequestedList[asset_uuid] = gFrameTimeSeconds;
+        scheduling = true;
+        while (!gLandmarkList.mWaitList.empty() && gLandmarkList.mRequestedList.size() < MAX_SIMULTANEOUS_REQUESTS)
+        {
+            // start new download from wait list
+            landmark_uuid_list_t::iterator iter = gLandmarkList.mWaitList.begin();
+            LLUUID asset_uuid = *iter;
+            gLandmarkList.mWaitList.erase(iter);
+            gAssetStorage->getAssetData(asset_uuid,
+                LLAssetType::AT_LANDMARK,
+                LLLandmarkList::processGetAssetReply,
+                NULL);
+            gLandmarkList.mRequestedList[asset_uuid] = gFrameTimeSeconds;
+        }
+        scheduling = false;
     }
 }