SL-18701 llsd is not thread safe, parse it before using

1 files changed, 76 insertions, 85 deletions

diff --git a/indra/newview/llgltfmateriallist.cpp b/indra/newview/llgltfmateriallist.cpp
index a604930715..45de0b71ae 100644
--- a/indra/newview/llgltfmateriallist.cpp
+++ b/indra/newview/llgltfmateriallist.cpp
@@ -42,6 +42,7 @@
 #include "llviewerstats.h"
 #include "llcorehttputil.h"
 #include "llagent.h"
+#include "llvocache.h"
 #include "llworld.h"
 
 #include "tinygltf/tiny_gltf.h"
@@ -159,14 +160,14 @@ public:
         LLSD message;
 
         sparam_t::const_iterator it = strings.begin();
-        if (it != strings.end()) {
+        if (it != strings.end())
+        {
             const std::string& llsdRaw = *it++;
             std::istringstream llsdData(llsdRaw);
             if (!LLSDSerialize::deserialize(message, llsdData, llsdRaw.length()))
             {
                 LL_WARNS() << "LLGLTFMaterialOverrideDispatchHandler: Attempted to read parameter data into LLSD but failed:" << llsdRaw << LL_ENDL;
             }
-            LLGLTFMaterialList::writeCacheOverrides(message, llsdRaw);
         }
         else
         {
@@ -175,99 +176,118 @@ public:
             return false;
         }
 
-        if (!message.has("object_id"))
+        LLGLTFOverrideCacheEntry object_override;
+        if (!object_override.fromLLSD(message))
         {
             // malformed message, nothing we can do to handle it
             LL_DEBUGS("GLTF") << "Message without id:" << message << LL_ENDL;
             return false;
         }
 
+        // Cache the data
+        {
+            LL_DEBUGS("GLTF") << "material overrides cache" << LL_ENDL;
+
+            // default to main region if message doesn't specify
+            LLViewerRegion * region = gAgent.getRegion();;
+
+            if (object_override.mHasRegionHandle)
+            {
+                // TODO start requiring this once server sends this for all messages
+                region = LLWorld::instance().getRegionFromHandle(object_override.mRegionHandle);
+            }
+
+            if (region)
+            {
+                region->cacheFullUpdateGLTFOverride(object_override);
+            }
+            else
+            {
+                LL_WARNS("GLTF") << "could not access region for material overrides message cache, region_handle: " << LL_ENDL;
+            }
+        }
+        applyData(object_override);
+        return true;
+    }
+
+    static void applyData(const LLGLTFOverrideCacheEntry &object_override)
+    {
+        // Parse the data
+
         LL::WorkQueue::ptr_t main_queue = LL::WorkQueue::getInstance("mainloop");
         LL::WorkQueue::ptr_t general_queue = LL::WorkQueue::getInstance("General");
 
         struct ReturnData
         {
         public:
-            std::vector<LLPointer<LLGLTFMaterial> > mMaterialVector;
-            std::vector<bool> mResults;
+            LLPointer<LLGLTFMaterial> mMaterial;
+            S32 mSide;
+            bool mSuccess;
         };
 
         // fromJson() is performance heavy offload to a thread.
         main_queue->postTo(
             general_queue,
-            [message]() // Work done on general queue
+            [object_override]() // Work done on general queue
         {
-            ReturnData result;
+            std::vector<ReturnData> results;
 
-            if (message.has("sides") && message.has("gltf_json"))
+            if (!object_override.mSides.empty())
             {
-                LLSD const& sides = message.get("sides");
-                LLSD const& gltf_json = message.get("gltf_json");
-
-                if (sides.isArray() && gltf_json.isArray() &&
-                    sides.size() != 0 &&
-                    sides.size() == gltf_json.size())
+                results.reserve(object_override.mSides.size());
+                // parse json
+                std::map<S32, std::string>::const_iterator iter = object_override.mSides.begin();
+                std::map<S32, std::string>::const_iterator end = object_override.mSides.end();
+                while (iter != end)
                 {
-                    // message should be interpreted thusly:
-                    ///  sides is a list of face indices
-                    //   gltf_json is a list of corresponding json
-                    //   any side not represented in "sides" has no override
-                    result.mResults.resize(sides.size());
-                    result.mMaterialVector.resize(sides.size());
-
-                    // parse json
-                    for (int i = 0; i < sides.size(); ++i)
-                    {
-                        LLPointer<LLGLTFMaterial> override_data = new LLGLTFMaterial();
-
-                        std::string gltf_json_str = gltf_json[i].asString();
+                    LLPointer<LLGLTFMaterial> override_data = new LLGLTFMaterial();
+                    std::string warn_msg, error_msg;
 
-                        std::string warn_msg, error_msg;
+                    bool success = override_data->fromJSON(iter->second, warn_msg, error_msg);
 
-                        bool success = override_data->fromJSON(gltf_json_str, warn_msg, error_msg);
+                    ReturnData result;
+                    result.mSuccess = success;
+                    result.mSide = iter->first;
 
-                        result.mResults[i] = success;
-
-                        if (success)
-                        {
-                            result.mMaterialVector[i] = override_data;
-                        }
-                        else
-                        {
-                            LL_WARNS("GLTF") << "failed to parse GLTF override data.  errors: " << error_msg << " | warnings: " << warn_msg << LL_ENDL;
-                        }
+                    if (success)
+                    {
+                        result.mMaterial = override_data;
                     }
+                    else
+                    {
+                        LL_WARNS("GLTF") << "failed to parse GLTF override data.  errors: " << error_msg << " | warnings: " << warn_msg << LL_ENDL;
+                    }
+
+                    results.push_back(result);
+                    iter++;
                 }
             }
-            return result;
+            return results;
         },
-            [message](ReturnData result) // Callback to main thread
+            [object_override](std::vector<ReturnData> results) // Callback to main thread
             {
 
-            LLUUID object_id = message.get("object_id").asUUID();
-            LLViewerObject * obj = gObjectList.findObject(object_id);
+            LLViewerObject * obj = gObjectList.findObject(object_override.mObjectId);
 
-            if (result.mResults.size() > 0 )
+            if (results.size() > 0 )
             {
-                LLSD const& sides = message.get("sides");
                 std::unordered_set<S32> side_set;
 
-                for (int i = 0; i < result.mResults.size(); ++i)
+                for (int i = 0; i < results.size(); ++i)
                 {
-                    if (result.mResults[i])
+                    if (results[i].mSuccess)
                     {
-                        S32 side = sides[i].asInteger();
                         // flag this side to not be nulled out later
-                        side_set.insert(sides[i]);
+                        side_set.insert(results[i].mSide);
 
-                        if (!obj || !obj->setTEGLTFMaterialOverride(side, result.mMaterialVector[i]))
+                        if (!obj || !obj->setTEGLTFMaterialOverride(results[i].mSide, results[i].mMaterial))
                         {
                             // object not ready to receive override data, queue for later
-                            gGLTFMaterialList.queueOverrideUpdate(object_id, side, result.mMaterialVector[i]);
+                            gGLTFMaterialList.queueOverrideUpdate(object_override.mObjectId, results[i].mSide, results[i].mMaterial);
                         }
                         else if (obj && obj->isAnySelected())
                         {
-                            LLMaterialEditor::updateLive(object_id, side);
+                            LLMaterialEditor::updateLive(object_override.mObjectId, results[i].mSide);
                         }
                     }
                     else
@@ -275,7 +295,7 @@ public:
                         // unblock material editor
                         if (obj && obj->isAnySelected())
                         {
-                            LLMaterialEditor::updateLive(object_id, sides[i].asInteger());
+                            LLMaterialEditor::updateLive(object_override.mObjectId, results[i].mSide);
                         }
                     }
                 }
@@ -290,7 +310,7 @@ public:
                             obj->setTEGLTFMaterialOverride(i, nullptr);
                             if (object_has_selection)
                             {
-                                LLMaterialEditor::updateLive(object_id, i);
+                                LLMaterialEditor::updateLive(object_override.mObjectId, i);
                             }
                         }
                     }
@@ -309,8 +329,6 @@ public:
                 }
             }
         });
-
-        return true;
     }
 };
 
@@ -439,8 +457,6 @@ void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
 
         sUpdates = LLSD::emptyArray();
     }
-
-    
 }
 
 class AssetLoadUserData
@@ -695,32 +711,7 @@ void LLGLTFMaterialList::modifyMaterialCoro(std::string cap_url, LLSD overrides,
     }
 }
 
-void LLGLTFMaterialList::writeCacheOverrides(LLSD const & message, std::string const & llsdRaw)
+void LLGLTFMaterialList::loadCacheOverrides(const LLGLTFOverrideCacheEntry& override)
 {
-    LL_DEBUGS("GLTF") << "material overrides cache" << LL_ENDL;
-
-    // default to main region if message doesn't specify
-    LLViewerRegion * region = gAgent.getRegion();;
-
-    if (message.has("region_handle_low") && message.has("region_handle_high"))
-    {
-        // TODO start requiring this once server sends this for all messages
-        U64 region_handle_low = message["region_handle_low"].asInteger();
-        U64 region_handle_high = message["region_handle_high"].asInteger();
-        U64 region_handle = (region_handle_low & 0x00000000ffffffffUL) || (region_handle_high << 32);
-        region = LLWorld::instance().getRegionFromHandle(region_handle);
-    }
-
-    if (region) {
-        region->cacheFullUpdateExtras(message, llsdRaw);
-    } else {
-        LL_WARNS("GLTF") << "could not access region for material overrides message cache, region_handle: " << LL_ENDL;
-    }
-}
-
-void LLGLTFMaterialList::loadCacheOverrides(std::string const & message)
-{
-    std::vector<std::string> strings(1, message);
-
-    handle_gltf_override_message(nullptr, "", LLUUID::null, strings);
+    LLGLTFMaterialOverrideDispatchHandler::applyData(override);
 }