release lock before calling customer code for config changes(CppMicroServices#1012) (CppMicroServices#1172)

Signed-off-by: Toby Cormack <tcormack@mathworks.com>
Co-authored-by: tcormackMW <113473781+tcormackMW@users.noreply.github.com>

Author: RobertLauferElektrobit

compendium/DeclarativeServices/src/manager/ConfigurationNotifier.cpp

@@ -46,13 +46,13 @@ namespace cppmicroservices
             , logger(logger)
             , componentFactory(std::make_shared<ComponentFactoryImpl>(context, logger, asyncWorkSvc, extensionReg))
 
-          {
+        {
             if (!context || !(this->logger) || !asyncWorkSvc || !extensionReg || !componentFactory)
             {
                 throw std::invalid_argument("ConfigurationNotifier Constructor "
                                             "provided with invalid arguments");
             }
-         }
+        }
 
         cppmicroservices::ListenerTokenId
         ConfigurationNotifier::RegisterListener(std::string const& pid,
@@ -112,7 +112,8 @@ namespace cppmicroservices
             }
         }
         bool
-        ConfigurationNotifier::AnyListenersForPid(std::string const& pid, cppmicroservices::AnyMap const& properties) noexcept
+        ConfigurationNotifier::AnyListenersForPid(std::string const& pid,
+                                                  cppmicroservices::AnyMap const& properties) noexcept
         {
             std::string factoryName;
             std::vector<std::shared_ptr<ComponentConfigurationImpl>> mgrs;
@@ -179,30 +180,43 @@ namespace cppmicroservices
             return true;
         }
 
-
         void
         ConfigurationNotifier::NotifyAllListeners(std::string const& pid,
                                                   cppmicroservices::service::cm::ConfigurationEventType type,
                                                   std::shared_ptr<cppmicroservices::AnyMap> properties)
         {
+            // lock held throughout to guarantee ordering for calls to this function
+            std::lock_guard<std::mutex> l(notificationOrderingLock);
             ConfigChangeNotification notification
                 = ConfigChangeNotification(pid, std::move(properties), std::move(type));
-
-            auto listenersMapHandle = listenersMap.lock();
-            auto iter = listenersMapHandle->find(pid);
-            if (iter != listenersMapHandle->end())
+            std::vector<Listener> callbacks;
             {
-                for (auto const& configListenerPtr : *(iter->second))
+                auto listenersMapHandle = listenersMap.lock();
+                auto iter = listenersMapHandle->find(pid);
+                callbacks.reserve((iter->second)->size());
+
+                if (iter != listenersMapHandle->end())
                 {
-                    configListenerPtr.second.notify(notification);
+
+                    for (auto const& configListenerPtr : *(iter->second))
+                    {
+                        // deep copy to get actual objects as opposed to shared_ptr to mutable map
+                        callbacks.emplace_back(configListenerPtr.second);
+                    }
+                }
+                else
+                {
+                    return;
                 }
             }
-            else
+            for (auto const& listener : callbacks)
             {
-                return;
+                listener.notify(notification);
             }
         }
-        std::shared_ptr<ComponentFactoryImpl>  ConfigurationNotifier::GetComponentFactory() {
+        std::shared_ptr<ComponentFactoryImpl>
+        ConfigurationNotifier::GetComponentFactory()
+        {
             return componentFactory;
         }
         void
@@ -222,9 +236,9 @@ namespace cppmicroservices
                 {
                     // This reference has a dynamic target
                     logger->Log(cppmicroservices::logservice::SeverityLevel::LOG_ERROR,
-                                "Properties for component " + metadata->name + "contains a dynamic target for interface "
-                                    + ref.interfaceName + " target= " + ref.target
-                                    + " Dynamic targets are only valid for factory components");
+                                "Properties for component " + metadata->name
+                                    + "contains a dynamic target for interface " + ref.interfaceName + " target= "
+                                    + ref.target + " Dynamic targets are only valid for factory components");
                 }
             }
         }

compendium/DeclarativeServices/src/manager/ConfigurationNotifier.hpp

@@ -119,6 +119,7 @@ namespace cppmicroservices
 
             std::shared_ptr<cppmicroservices::logservice::LogService> logger;
             std::shared_ptr<ComponentFactoryImpl> componentFactory;
+            std::mutex notificationOrderingLock;
          };
 
     } // namespace scrimpl