Fixes CppMicroServices#986, deadlock on concurrent operations within asyncworkservice (CppMicroServices#987) (CppMicroServices#1152)

Co-authored-by: tcormackMW <113473781+tcormackMW@users.noreply.github.com>

Author: RobertLauferElektrobit

compendium/DeclarativeServices/src/SCRBundleExtension.cpp

@@ -51,21 +51,23 @@ namespace cppmicroservices
             , logger(logger)
             , configNotifier(configNotifier)
         {
-            if (!bundle || !registry || !logger  || !configNotifier)
+            if (!bundle || !registry || !logger || !configNotifier)
             {
                 throw std::invalid_argument("Invalid parameters passed to SCRBundleExtension constructor");
             }
             managers = std::make_shared<std::vector<std::shared_ptr<ComponentManager>>>();
         }
 
-       void SCRBundleExtension::Initialize(cppmicroservices::AnyMap const& scrMetadata,
+        void
+        SCRBundleExtension::Initialize(
+            cppmicroservices::AnyMap const& scrMetadata,
             std::shared_ptr<cppmicroservices::async::AsyncWorkService> const& asyncWorkService)
-         {
-            if ( scrMetadata.empty() || !asyncWorkService )
+        {
+            if (scrMetadata.empty() || !asyncWorkService)
             {
                 throw std::invalid_argument("Invalid parameters passed to SCRBundleExtension::Initialize");
             }
- 
+
             auto version = ObjectValidator(scrMetadata, "version").GetValue<int>();
             auto metadataparser = metadata::MetadataParserFactory::Create(version, logger);
             std::vector<std::shared_ptr<ComponentMetadata>> componentsMetadata;
@@ -131,12 +133,14 @@ namespace cppmicroservices
                         "Deleting instance of SCRBundleExtension for " + bundle_.GetSymbolicName());
             for (auto& compManager : *managers)
             {
-                auto fut = compManager->Disable();
+                std::shared_ptr<std::atomic<bool>> asyncStarted = std::make_shared<std::atomic<bool>>(false);
+                auto fut = compManager->Disable(asyncStarted);
                 registry->RemoveComponentManager(compManager);
                 try
                 {
-                    fut.get(); // since this happens when the bundle is stopped. Wait until the disable is finished on
-                               // the other thread.
+                    // since this happens when the bundle is stopped,
+                    // wait until the disable is finished on the other thread.
+                    compManager->WaitForFuture(fut, asyncStarted);
                 }
                 catch (...)
                 {

compendium/DeclarativeServices/src/manager/BundleOrPrototypeComponentConfiguration.cpp

@@ -33,7 +33,7 @@ namespace cppmicroservices
     {
 
         BundleOrPrototypeComponentConfigurationImpl::BundleOrPrototypeComponentConfigurationImpl(
-            std::shared_ptr<const metadata::ComponentMetadata> metadata,
+            std::shared_ptr<metadata::ComponentMetadata const> metadata,
             cppmicroservices::Bundle const& bundle,
             std::shared_ptr<ComponentRegistry> registry,
             std::shared_ptr<cppmicroservices::logservice::LogService> logger,
@@ -176,7 +176,9 @@ namespace cppmicroservices
                               {
                                   try
                                   {
-                                      compMgr->Disable().get();
+                                      std::shared_ptr<std::atomic<bool>> asyncStarted = std::make_shared<std::atomic<bool>>(false);
+                                      auto f = compMgr->Disable(asyncStarted);
+                                      compMgr->WaitForFuture(f, asyncStarted);
                                   }
                                   catch (...)
                                   {

compendium/DeclarativeServices/src/manager/ComponentManager.hpp

@@ -49,6 +49,23 @@ namespace cppmicroservices
             ComponentManager& operator=(ComponentManager&&) = delete;
             virtual ~ComponentManager() = default;
 
+            /**
+             * Waits for the provided future from the asynchronous thread pool and executes
+             * the task on current thread if the thread pool has stalled.
+             *
+             * We determine a likely stall by timeout (50ms) while waiting for the future. After this timeout, we
+             * atomically compare and set the bool, asyncStarted, to 'take ownership' of execution of this task. If that
+             * bool was already set, then the spwaned thread has already taken ownership of the task and we do not need
+             * to execute it again. Otherwise, we execute the task. If we do not hit the timeout, then there was no
+             * stall and we can get() the future.
+             *
+             * \param fut The future to wait on
+             * \param asyncStarted The bool used to synchronize the waiting and posted thread
+             * \return void, once the future has been satisfied
+             */
+            virtual void WaitForFuture(std::shared_future<void>& fut, std::shared_ptr<std::atomic<bool>> asyncStarted)
+                = 0;
+
             /**
              * Returns the name of the component managed by this object. The name is the same
              * as specified in the component description.
@@ -69,15 +86,33 @@ namespace cppmicroservices
              * This method changes the state of the ComponentManager to ENABLED. The method returns
              * immediately after changing the state. Any configurations created as a result of the
              * state change will happen asynchronously on a separate thread.
+             *
+             * \param asyncStarted The bool used to synchronize the waiting and posted thread
+             * \parblock
+             * If returned future IS blocked on: create asyncStarted value and pass in on calling thread.
+             * Once the future is returned, call WaitForFuture() with the returned future and your asyncStarted value
+             *
+             * If future IS NOT blocked on: call Enable(), or explicitly Enable(nullptr), no other action is required
+             * \endparblock
+             * \return std::shared_future<void> assosciated with the state change
              */
-            virtual std::shared_future<void> Enable() = 0;
+            virtual std::shared_future<void> Enable(std::shared_ptr<std::atomic<bool>> asyncStarted = nullptr) = 0;
 
             /**
              * This method changes the state of the ComponentManager to DISABLED. The method returns
              * immediately after changing the state. Any configurations deleted as a result of the
-             * state change will happen asynchronously on a separate thread.
+             * state change will happen asynchronously on a separate thread. 
+             *
+             * \param asyncStarted The bool used to synchronize the waiting and posted thread
+             * \parblock
+             * If future IS blocked on: create asyncStarted value and pass in on calling thread.
+             * Once the future is returned, call WaitForFuture() with the returned future and your asyncStarted value
+             *
+             * If future IS NOT blocked on: call Disable(), or explicitly Disable(nullptr), no other action is required
+             * \endparblock
+             * \return std::shared_future<void> assosciated with the state change
              */
-            virtual std::shared_future<void> Disable() = 0;
+            virtual std::shared_future<void> Disable(std::shared_ptr<std::atomic<bool>> asyncStarted = nullptr) = 0;
 
             /**
              * Returns a vector of ComponentConfiguration objects representing each of the configurations
@@ -89,7 +124,7 @@ namespace cppmicroservices
              * Returns the metadata object representing the component description for the
              * component managed by this object.
              */
-            virtual std::shared_ptr<const metadata::ComponentMetadata> GetMetadata() const = 0;
+            virtual std::shared_ptr<metadata::ComponentMetadata const> GetMetadata() const = 0;
         };
     } // namespace scrimpl
 } // namespace cppmicroservices