Merge branch 'development' of https://github.com/o3de/o3de into daimini/settings-registry-origin-tracking

# Conflicts:
#	Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp

Code/Framework/AzCore/AzCore/EBus/Event.h

@@ -58,6 +58,12 @@ namespace AZ
 
         Event& operator=(Event&& rhs);
 
+        //! Take the handlers registered with the other event
+        //! and move them to this event. The other will event
+        //! will be cleared after call
+        //! @param other event to move handlers
+        Event& ClaimHandlers(Event&& other);
+
         //! Returns true if at least one handler is connected to this event.
         bool HasHandlerConnected() const;
 

Code/Framework/AzCore/AzCore/EBus/Event.inl

@@ -207,6 +207,32 @@ namespace AZ
     }
 
 
+    template <typename... Params>
+    auto Event<Params...>::ClaimHandlers(Event&& other) -> Event&
+    {
+        auto handlers = AZStd::move(other.m_handlers);
+        auto addList = AZStd::move(other.m_addList);
+        other.m_freeList = {};
+        other.m_updating = false;
+
+        AZStd::array handlerContainers{ &handlers, &addList };
+        for (AZStd::vector<Handler*>* handlerList : handlerContainers)
+        {
+            for (Handler* handler : *handlerList)
+            {
+                if (handler != nullptr)
+                {
+                    handler->m_index = 0;
+                    handler->m_event = this;
+                    Connect(*handler);
+                }
+            }
+        }
+
+        return *this;
+    }
+
+
     template <typename... Params>
     bool Event<Params...>::HasHandlerConnected() const
     {

Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.cpp

@@ -20,7 +20,7 @@
 namespace AZ
 {
     template<typename T>
-    bool SettingsRegistryImpl::SetValueInternal(AZStd::string_view path, T value, SettingsRegistryInterface::Type type)
+    bool SettingsRegistryImpl::SetValueInternal(AZStd::string_view path, T value)
     {
         if (path.empty())
         {
@@ -56,7 +56,6 @@ namespace AZ
                 static_assert(!AZStd::is_same_v<T, T>, "SettingsRegistryImpl::SetValueInternal called with unsupported type.");
             }
 
-            m_notifiers.Signal(path, type);
             return true;
         }
         return false;
@@ -157,11 +156,11 @@ namespace AZ
             // Setting to empty string to prevent assert
             path = "";
         }
-        AZStd::scoped_lock lock(m_settingMutex);
 
         rapidjson::Pointer pointer(path.data(), path.length());
         if (pointer.IsValid())
         {
+            AZStd::scoped_lock lock(m_settingMutex);
             const rapidjson::Value* value = pointer.Get(m_settings);
             if (value)
             {
@@ -207,7 +206,7 @@ namespace AZ
     {
         NotifyEventHandler notifyHandler{ callback };
         {
-            AZStd::scoped_lock lock(m_settingMutex);
+            AZStd::scoped_lock lock(m_notifierMutex);
             notifyHandler.Connect(m_notifiers);
         }
         return notifyHandler;
@@ -217,7 +216,7 @@ namespace AZ
     {
         NotifyEventHandler notifyHandler{ AZStd::move(callback) };
         {
-            AZStd::scoped_lock lock(m_settingMutex);
+            AZStd::scoped_lock lock(m_notifierMutex);
             notifyHandler.Connect(m_notifiers);
         }
         return notifyHandler;
@@ -225,7 +224,7 @@ namespace AZ
 
     void SettingsRegistryImpl::ClearNotifiers()
     {
-        AZStd::scoped_lock lock(m_settingMutex);
+        AZStd::scoped_lock lock(m_notifierMutex);
         m_notifiers.DisconnectAllHandlers();
     }
 
@@ -276,6 +275,31 @@ namespace AZ
         m_postMergeEvent.DisconnectAllHandlers();
     }
 
+    void SettingsRegistryImpl::SignalNotifier(AZStd::string_view jsonPath, Type type)
+    {
+        // Move the Notifier AZ::Event to a local AZ::Event in order to allow
+        // the notifier handlers to be signaled outside of the notifier mutex
+        // This allows other threads to register notifiers while this thread
+        // is invoking the handlers
+        decltype(m_notifiers) localNotifierEvent;
+        {
+            AZStd::scoped_lock lock(m_notifierMutex);
+            localNotifierEvent = AZStd::move(m_notifiers);
+        }
+
+        localNotifierEvent.Signal(jsonPath, type);
+
+        {
+            // Swap the local handlers with the current m_notifiers which
+            // will contain any handlers added during the signaling of the
+            // local event
+            AZStd::scoped_lock lock(m_notifierMutex);
+            AZStd::swap(m_notifiers, localNotifierEvent);
+            // Append any added handlers to the m_notifier structure
+            m_notifiers.ClaimHandlers(AZStd::move(localNotifierEvent));
+        }
+    }
+
     SettingsRegistryInterface::Type SettingsRegistryImpl::GetType(AZStd::string_view path) const
     {
         if (path.empty())
@@ -286,11 +310,11 @@ namespace AZ
             path = "";
         }
 
-        AZStd::scoped_lock lock(m_settingMutex);
 
         rapidjson::Pointer pointer(path.data(), path.length());
         if (pointer.IsValid())
         {
+            AZStd::scoped_lock lock(m_settingMutex);
             const rapidjson::Value* value = pointer.Get(m_settings);
             if (value)
             {
@@ -363,11 +387,11 @@ namespace AZ
             // Setting to empty string to prevent assert
             path = "";
         }
-        AZStd::scoped_lock lock(m_settingMutex);
 
         rapidjson::Pointer pointer(path.data(), path.length());
         if (pointer.IsValid())
         {
+            AZStd::scoped_lock lock(m_settingMutex);
             const rapidjson::Value* value = pointer.Get(m_settings);
             if (value)
             {
@@ -380,32 +404,52 @@ namespace AZ
 
     bool SettingsRegistryImpl::Set(AZStd::string_view path, bool value)
     {
-        AZStd::scoped_lock lock(m_settingMutex);
-        return SetValueInternal(path, value, Type::Boolean);
+        if (AZStd::scoped_lock lock(m_settingMutex); !SetValueInternal(path, value))
+        {
+            return false;
+        }
+        SignalNotifier(path, Type::Boolean);
+        return true;
     }
 
     bool SettingsRegistryImpl::Set(AZStd::string_view path, s64 value)
     {
-        AZStd::scoped_lock lock(m_settingMutex);
-        return SetValueInternal(path, value, Type::Integer);
+        if (AZStd::scoped_lock lock(m_settingMutex); !SetValueInternal(path, value))
+        {
+            return false;
+        }
+        SignalNotifier(path, Type::Integer);
+        return true;
     }
 
     bool SettingsRegistryImpl::Set(AZStd::string_view path, u64 value)
     {
-        AZStd::scoped_lock lock(m_settingMutex);
-        return SetValueInternal(path, value, Type::Integer);
+        if (AZStd::scoped_lock lock(m_settingMutex); !SetValueInternal(path, value))
+        {
+            return false;
+        }
+        SignalNotifier(path, Type::Integer);
+        return true;
     }
 
     bool SettingsRegistryImpl::Set(AZStd::string_view path, double value)
     {
-        AZStd::scoped_lock lock(m_settingMutex);
-        return SetValueInternal(path, value, Type::FloatingPoint);
+        if (AZStd::scoped_lock lock(m_settingMutex); !SetValueInternal(path, value))
+        {
+            return false;
+        }
+        SignalNotifier(path, Type::FloatingPoint);
+        return true;
     }
 
     bool SettingsRegistryImpl::Set(AZStd::string_view path, AZStd::string_view value)
     {
-        AZStd::scoped_lock lock(m_settingMutex);
-        return SetValueInternal(path, value, Type::String);
+        if (AZStd::scoped_lock lock(m_settingMutex); !SetValueInternal(path, value))
+        {
+            return false;
+        }
+        SignalNotifier(path, Type::String);
+        return true;
     }
 
     bool SettingsRegistryImpl::Set(AZStd::string_view path, const char* value)
@@ -423,7 +467,6 @@ namespace AZ
             path = "";
         }
 
-        AZStd::scoped_lock lock(m_settingMutex);
 
         rapidjson::Pointer pointer(path.data(), path.length());
         if (pointer.IsValid())
@@ -433,9 +476,10 @@ namespace AZ
                 value, nullptr, valueTypeID, m_serializationSettings);
             if (jsonResult.GetProcessing() != JsonSerializationResult::Processing::Halted)
             {
+                AZStd::scoped_lock lock(m_settingMutex);
                 rapidjson::Value& setting = pointer.Create(m_settings, m_settings.GetAllocator());
                 setting = AZStd::move(store);
-                m_notifiers.Signal(path, Type::Object);
+                SignalNotifier(path, Type::Object);
                 return true;
             }
         }
@@ -451,13 +495,13 @@ namespace AZ
             // Setting to empty string to prevent assert
             path = "";
         }
-        AZStd::scoped_lock lock(m_settingMutex);
         rapidjson::Pointer pointerPath(path.data(), path.size());
         if (!pointerPath.IsValid())
         {
             return false;
         }
 
+        AZStd::scoped_lock lock(m_settingMutex);
         return pointerPath.Erase(m_settings);
     }
 
@@ -587,7 +631,7 @@ namespace AZ
             return false;
         }
 
-        m_notifiers.Signal("", Type::Object);
+        SignalNotifier("", Type::Object);
 
         return true;
     }
@@ -609,8 +653,6 @@ namespace AZ
             scratchBuffer = &buffer;
         }
 
-        AZStd::scoped_lock lock(m_settingMutex);
-
         bool result = false;
         if (path[path.length()] == 0)
         {
@@ -624,6 +666,8 @@ namespace AZ
                     R"(Path "%.*s" is too long. Either make sure that the provided path is terminated or use a shorter path.)",
                     static_cast<int>(path.length()), path.data());
                 Pointer pointer(AZ_SETTINGS_REGISTRY_HISTORY_KEY "/-");
+
+                AZStd::scoped_lock lock(m_settingMutex);
                 Value pathValue(path.data(), aznumeric_caster(path.length()), m_settings.GetAllocator());
                 pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                     .AddMember(StringRef("Error"), StringRef("Unable to read registry file."), m_settings.GetAllocator())
@@ -669,6 +713,7 @@ namespace AZ
         {
             AZ_Error("Settings Registry", false, "Folder path for the Setting Registry is too long: %.*s",
                 static_cast<int>(path.size()), path.data());
+            AZStd::scoped_lock lock(m_settingMutex);
             pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                 .AddMember(StringRef("Error"), StringRef("Folder path for the Setting Registry is too long."), m_settings.GetAllocator())
                 .AddMember(StringRef("Path"), Value(path.data(), aznumeric_caster(path.length()), m_settings.GetAllocator()), m_settings.GetAllocator());
@@ -706,6 +751,7 @@ namespace AZ
                 if (fileList.size() >= MaxRegistryFolderEntries)
                 {
                     AZ_Error("Settings Registry", false, "Too many files in registry folder.");
+                    AZStd::scoped_lock lock(m_settingMutex);
                     pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                         .AddMember(StringRef("Error"), StringRef("Too many files in registry folder."), m_settings.GetAllocator())
                         .AddMember(StringRef("Path"), Value(folderPath.c_str(), aznumeric_caster(folderPath.size()), m_settings.GetAllocator()), m_settings.GetAllocator())
@@ -725,7 +771,6 @@ namespace AZ
         SystemFile::FindFiles(folderPath.c_str(), callback);
 
 
-        AZStd::scoped_lock lock(m_settingMutex);
         if (!platform.empty())
         {
             // Move the folderPath prefix back to the supplied path before the wildcard
@@ -743,6 +788,7 @@ namespace AZ
                     if (fileList.size() >= MaxRegistryFolderEntries)
                     {
                         AZ_Error("Settings Registry", false, "Too many files in registry folder.");
+                        AZStd::scoped_lock lock(m_settingMutex);
                         pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                             .AddMember(StringRef("Error"), StringRef("Too many files in registry folder."), m_settings.GetAllocator())
                             .AddMember(StringRef("Path"), Value(folderPath.c_str(), aznumeric_caster(folderPath.size()), m_settings.GetAllocator()), m_settings.GetAllocator())
@@ -970,6 +1016,8 @@ namespace AZ
         collisionFound = true;
         AZ_Error("Settings Registry", false, R"(Two registry files in "%.*s" point to the same specialization: "%s" and "%s")",
             AZ_STRING_ARG(folderPath), lhs.m_relativePath.c_str(), rhs.m_relativePath.c_str());
+
+        AZStd::scoped_lock lock(m_settingMutex);
         historyPointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
             .AddMember(StringRef("Error"), StringRef("Too many files in registry folder."), m_settings.GetAllocator())
             .AddMember(StringRef("Path"),
@@ -1124,6 +1172,7 @@ namespace AZ
                 }
             }
             
+            AZStd::scoped_lock lock(m_settingMutex);
             pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                 .AddMember(StringRef("Error"), StringRef("Unable to parse registry file due to invalid json."), m_settings.GetAllocator())
                 .AddMember(StringRef("Path"), Value(path, m_settings.GetAllocator()), m_settings.GetAllocator())
@@ -1149,6 +1198,7 @@ namespace AZ
                     R"(To merge the supplied settings registry file, the settings within it must be placed within a JSON Object '{}')"
                     R"( in order to allow moving of its fields using the root-key as an anchor.)", path);
 
+                AZStd::scoped_lock lock(m_settingMutex);
                 pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                     .AddMember(StringRef("Error"), StringRef("Cannot merge registry file with a root which is not a JSON Object,"
                         " an empty root key and a merge approach of JsonMergePatch. Otherwise the Settings Registry would be overridden."
@@ -1167,6 +1217,7 @@ namespace AZ
         JsonSerializationResult::ResultCode mergeResult(JsonSerializationResult::Tasks::Merge);
         if (rootKey.empty())
         {
+            AZStd::scoped_lock lock(m_settingMutex);
             mergeResult = JsonSerialization::ApplyPatch(m_settings, m_settings.GetAllocator(), jsonPatch, mergeApproach, m_applyPatchSettings);
         }
         else
@@ -1174,6 +1225,7 @@ namespace AZ
             Pointer root(rootKey.data(), rootKey.length());
             if (root.IsValid())
             {
+                AZStd::scoped_lock lock(m_settingMutex);
                 Value& rootValue = root.Create(m_settings, m_settings.GetAllocator());
                 mergeResult = JsonSerialization::ApplyPatch(rootValue, m_settings.GetAllocator(), jsonPatch, mergeApproach, m_applyPatchSettings);
             }
@@ -1181,6 +1233,7 @@ namespace AZ
             {
                 AZ_Error("Settings Registry", false, R"(Failed to root path "%.*s" is invalid.)",
                     aznumeric_cast<int>(rootKey.length()), rootKey.data());
+                AZStd::scoped_lock lock(m_settingMutex);
                 pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                     .AddMember(StringRef("Error"), StringRef("Invalid root key."), m_settings.GetAllocator())
                     .AddMember(StringRef("Path"), Value(path, m_settings.GetAllocator()), m_settings.GetAllocator());
@@ -1190,15 +1243,19 @@ namespace AZ
         if (mergeResult.GetProcessing() != JsonSerializationResult::Processing::Completed)
         {
             AZ_Error("Settings Registry", false, R"(Failed to fully merge registry file "%s".)", path);
+            AZStd::scoped_lock lock(m_settingMutex);
             pointer.Create(m_settings, m_settings.GetAllocator()).SetObject()
                 .AddMember(StringRef("Error"), StringRef("Failed to fully merge registry file."), m_settings.GetAllocator())
                 .AddMember(StringRef("Path"), Value(path, m_settings.GetAllocator()), m_settings.GetAllocator());
             return false;
         }
 
-        pointer.Create(m_settings, m_settings.GetAllocator()).SetString(path, m_settings.GetAllocator());
+        {
+            AZStd::scoped_lock lock(m_settingMutex);
+            pointer.Create(m_settings, m_settings.GetAllocator()).SetString(path, m_settings.GetAllocator());
+        }
 
-        m_notifiers.Signal("", Type::Object);
+        SignalNotifier("", Type::Object);
 
         return true;
     }

Code/Framework/AzCore/AzCore/Settings/SettingsRegistryImpl.h

@@ -95,7 +95,7 @@ namespace AZ
         using RegistryFileList = AZStd::fixed_vector<RegistryFile, MaxRegistryFolderEntries>;
 
         template<typename T>
-        bool SetValueInternal(AZStd::string_view path, T value, SettingsRegistryInterface::Type type);
+        bool SetValueInternal(AZStd::string_view path, T value);
         template<typename T>
         bool GetValueInternal(T& result, AZStd::string_view path) const;
         VisitResponse Visit(Visitor& visitor, StackedString& path, AZStd::string_view valueName,
@@ -106,8 +106,11 @@ namespace AZ
             const rapidjson::Pointer& historyPointer, AZStd::string_view folderPath);
         bool ExtractFileDescription(RegistryFile& output, const char* filename, const Specializations& specializations);
         bool MergeSettingsFileInternal(const char* path, Format format, AZStd::string_view rootKey, AZStd::vector<char>& scratchBuffer);
+
+        void SignalNotifier(AZStd::string_view jsonPath, Type type);
         
         mutable AZStd::recursive_mutex m_settingMutex;
+        mutable AZStd::recursive_mutex m_notifierMutex;
         NotifyEvent m_notifiers;
         PreMergeEvent m_preMergeEvent;
         PostMergeEvent m_postMergeEvent;