Signal handling issues

src/core/connect/dbus_connect.cpp

@@ -87,7 +87,7 @@ void DBusConnect::process()
                     10);
                 guard();
             }
-            std::this_thread::sleep_for(1ms);
+            std::this_thread::sleep_for(20ms);
         }
         catch (const sdbusplus::exception_t& ex)
         {

src/core/connect/dbus_connect.hpp

@@ -87,7 +87,7 @@ class DBusConnect : protected IConnect
                     calledUnsafe = true;
                     return std::invoke(operation);
                 }
-                std::this_thread::sleep_for(priority * 20us);
+                std::this_thread::sleep_for(priority * 500us);
                 vocationThreadId = std::thread::id(0);
             }
             return std::invoke(operation);

src/core/entity/dbus_query.cpp

@@ -26,7 +26,7 @@ DBusQuery::InstanceRemoveHandlers DBusQuery::instanceRemoveHandlers;
 
 DBusInstance::WatchersMap DBusInstance::listeners;
 std::vector<DBusInstance::InstanceHash> DBusInstance::toCleanupInstances;
-std::atomic<std::thread::id> DBusInstance::instanceAccessGuard =
+std::atomic<std::thread::id> DBusInstance::AccessGuard::instanceAccessGuard =
     std::thread::id(0);
 
 std::size_t FindObjectDBusQuery::DBusObjectEndpoint::getHash() const
@@ -254,9 +254,13 @@ const IEntity::InstanceCollection IntrospectServiceDBusQuery::process()
     }
     for (const auto& [objectPath, interfaces] : interfacesResponse)
     {
+        InterfaceList actualInterfaces;
+        for (const auto& [interface, _] : interfaces)
+        {
+            actualInterfaces.emplace_back(interface);
+        }
         auto instance = std::make_shared<DBusInstance>(
-            serviceName, objectPath.str, getSearchPropertiesMap(), getWeakPtr(),
-            true);
+            serviceName, objectPath.str, actualInterfaces, getWeakPtr(), true);
 
         for (const auto& [interfaceName, propertiesMap] : interfaces)
         {
@@ -332,7 +336,8 @@ void DBusInstance::initialize()
 {
     bool hasError = false;
     setUninitialized();
-    for (const auto& [interface, _] : targetProperties)
+    AccessGuard guard;
+    for (const auto& interface : actualInterfaces)
     {
         try
         {
@@ -368,7 +373,7 @@ DBusInstancePtr DBusQuery::createInstance(const ServiceName& serviceName,
                                           const ObjectPath& objectPath,
                                           const InterfaceList& interfaces)
 {
-    DBusPropertyEndpointMap epMap;
+    InterfaceList actualInterfaces;
     for (const auto& interface : interfaces)
     {
         auto epIt = getSearchPropertiesMap().find(interface);
@@ -380,11 +385,11 @@ DBusInstancePtr DBusQuery::createInstance(const ServiceName& serviceName,
                               entry("DBUS_IFACE=%s", interface.c_str()));
             continue;
         }
-        epMap.emplace(epIt->first, epIt->second);
+        actualInterfaces.emplace_back(interface);
     }
 
-    auto instance = std::make_shared<DBusInstance>(serviceName, objectPath,
-                                                   epMap, getWeakPtr());
+    auto instance = std::make_shared<DBusInstance>(
+        serviceName, objectPath, actualInterfaces, getWeakPtr());
 
     instance->initialize();
     return std::forward<DBusInstancePtr>(instance);
@@ -501,8 +506,9 @@ void DBusInstance::bindListeners(const connect::DBusConnectUni& connection)
     using namespace sdbusplus::bus::match;
 
     auto selfWeak = weak_from_this();
+    AccessGuard guard;
 
-    for (auto [interface, _] : targetProperties)
+    for (const auto& interface : actualInterfaces)
     {
         auto matcher = connection->createWatcher(
             rules::propertiesChanged(this->getObjectPath(), interface),
@@ -539,13 +545,8 @@ void DBusInstance::bindListeners(const connect::DBusConnectUni& connection)
                           entry("DBUS_OBJ=%s", getObjectPath().c_str()),
                           entry("DBUS_IFACE=%s", interface.c_str()));
 
-        auto isLocked = instanceAccessGuardLock();
         listeners[getHash()].emplace_back(
             std::forward<sdbusplus::bus::match::match>(matcher));
-        if (isLocked)
-        {
-            instanceAccessGuardUnlock();
-        }
     }
 }
 
@@ -576,13 +577,22 @@ void DBusInstance::supplementOrUpdate(
 void DBusInstance::mergeInternalMetadata(
     const IEntity::InstancePtr& destination)
 {
+    InterfaceList mergedList;
     auto dbusInstanceDest =
         std::dynamic_pointer_cast<DBusInstance>(destination);
     if (!dbusInstanceDest)
     {
         return;
     }
-    this->targetProperties.merge(dbusInstanceDest->targetProperties);
+
+    AccessGuard guard;
+    std::merge(actualInterfaces.begin(), actualInterfaces.end(),
+               dbusInstanceDest->actualInterfaces.begin(),
+               dbusInstanceDest->actualInterfaces.end(),
+               std::back_inserter(mergedList));
+
+    // asuming, a mutex required
+    actualInterfaces.swap(mergedList);
 }
 
 void DBusInstance::supplementOrUpdate(const IEntity::InstancePtr& destination)
@@ -650,7 +660,7 @@ void DBusInstance::captureDBusAssociations(
 
         auto childInstance =
             std::make_shared<DBusInstance>(serviceName, complexAssocDummyPath,
-                                           targetProperties, dbusQuery, true);
+                                           actualInterfaces, dbusQuery, true);
         DBusPropertiesMap properties{
             {fieldSource, source},
             {fieldDestination, destination},
@@ -725,7 +735,7 @@ void DBusInstance::initDefaultFieldsValue()
 
 void DBusInstance::cleanupInstacesWatchers()
 {
-    auto isLocked = instanceAccessGuardLock();
+    AccessGuard guard;
 
     for (auto hash : toCleanupInstances)
     {
@@ -736,34 +746,44 @@ void DBusInstance::cleanupInstacesWatchers()
         }
     }
     toCleanupInstances.clear();
-
-    if (isLocked)
-    {
-        instanceAccessGuardUnlock();
-    }
 }
 
 void DBusInstance::markInstanceIsUnavailable(const DBusInstance& instance)
 {
-    auto isLocked = instanceAccessGuardLock();
+    AccessGuard guard;
     toCleanupInstances.emplace_back(instance.getHash());
-    if (isLocked)
-    {
-        instanceAccessGuardUnlock();
-    }
 }
 
 const DBusPropertySetters&
     DBusInstance::dbusPropertySetters(const InterfaceName& interface) const
 {
-    auto findInterface = this->targetProperties.find(interface);
-    if (findInterface == this->targetProperties.end())
+    static const DBusPropertySetters noSetters;
+    const auto query = this->dbusQuery.lock();
+    AccessGuard guard;
+    auto findInterface = std::find_if(
+        actualInterfaces.begin(), actualInterfaces.end(),
+        [interface](const auto& value) { return value == interface; });
+
+    if (findInterface == actualInterfaces.end())
     {
-        static const DBusPropertySetters noSetters;
         return noSetters;
     }
 
-    return std::move(findInterface->second);
+    if (query)
+    {
+        try
+        {
+            return query->dbusPropertySetters(*findInterface);
+        }
+        catch (const std::invalid_argument& iaEx)
+        {
+            log<level::DEBUG>("Specified interface not found at the endpoint "
+                              "criteria definition",
+                              entry("ERROR=%s", iaEx.what()));
+        }
+    }
+
+    return noSetters;
 }
 
 const DbusVariantType DBusQuery::processFormatters(const PropertyName& property,
@@ -786,6 +806,18 @@ const DbusVariantType DBusQuery::processFormatters(const PropertyName& property,
     return formattedValue;
 }
 
+const DBusPropertySetters&
+    DBusQuery::dbusPropertySetters(const InterfaceName& interface) const
+{
+    auto findSetters = getSearchPropertiesMap().find(interface);
+    if (findSetters != getSearchPropertiesMap().end())
+    {
+        return findSetters->second;
+    }
+
+    throw std::invalid_argument("Interface not found: " + interface);
+}
+
 void DBusQuery::registerObjectCreationObserver(
     std::reference_wrapper<entity::IEntity> entity)
 {

src/core/entity/dbus_query.hpp

@@ -102,10 +102,10 @@ class DBusInstance final :
      *        not.
      */
     bool state;
-    /** @brief The EP configuration (properties, entity-members, formatters,
-     *         validators)
+    /**
+     * @brief Interfaces list that actual for this instance
      */
-    DBusPropertyEndpointMap targetProperties;
+    InterfaceList actualInterfaces;
     /**
      * @brief The weak-pointer to the query object that is obtained instace from
      *        dbus
@@ -127,11 +127,6 @@ class DBusInstance final :
      */
     static std::vector<InstanceHash> toCleanupInstances;
 
-    /**
-     * @brief The DBusInstances internal resources access guard.
-     */
-    static std::atomic<std::thread::id> instanceAccessGuard;
-
   public:
     DBusInstance(const DBusInstance&) = delete;
     DBusInstance& operator=(const DBusInstance&) = delete;
@@ -142,20 +137,20 @@ class DBusInstance final :
      * @brief Construct a new DBusInstance object
      *
      * @param inServiceName         - The DBus service name
-     * @param inObjectPath          - the DBus object path
-     * @param targetPropertiesDict  - The EP configuration (properties,
-     *                                entity-members, formatters, validators)
+     * @param inObjectPath          - The DBus object path
+     * @param actualInterfaces      - Interfaces list that actual for this
+     *                                instance
      * @param queryObject           - The weak-pointer to the query that is
      *                                obtained dbus-object
      */
     explicit DBusInstance(const std::string& inServiceName,
                           const std::string& inObjectPath,
-                          const DBusPropertyEndpointMap& targetPropertiesDict,
+                          const InterfaceList& actualInterfaces,
                           const DBusQueryConstWeakPtr& queryObject,
                           bool state = false) noexcept :
         Entity::StaticInstance(inServiceName + inObjectPath),
         serviceName(inServiceName), objectPath(inObjectPath), state(state),
-        targetProperties(targetPropertiesDict), dbusQuery(queryObject)
+        actualInterfaces(actualInterfaces), dbusQuery(queryObject)
     {
         log<level::DEBUG>("Create DBus instance cache",
                           entry("DBUS_SVC=%s", inServiceName.c_str()),
@@ -410,47 +405,72 @@ class DBusInstance final :
 
   private:
     /**
-     * @brief Lock access to the DBusInstances internal resources.
-     *
-     * @note Thread safe, non-blocking (like spin-lock), global scope
+     * @brief RAII of access-guard of a non-threadsafe DBusInstace
+     *        resources.
      *
-     * @return bool - true if locked, false if protection is not required.
      */
-    inline static bool instanceAccessGuardLock()
+    class AccessGuard final
     {
-        using namespace std::chrono;
-        using namespace std::chrono_literals;
-
-        auto vocation = std::thread::id(0);
-        auto currentThreadId = std::this_thread::get_id();
-
-        /* non-blocking guard to prevent onetime access to the
-         * `toCleanupInstances` from different threads
+        /**
+         * @brief The DBusInstances internal resources access guard.
          */
-        while (!instanceAccessGuard.compare_exchange_strong(
-            vocation, currentThreadId, std::memory_order_release))
+        static std::atomic<std::thread::id> instanceAccessGuard;
+
+        /**
+         * @brief Lock access to the DBusInstances internal resources.
+         *
+         * @note Thread safe, non-blocking (like spin-lock), global scope
+         *
+         * @return bool - true if locked, false if protection is not required.
+         */
+        inline static bool instanceAccessGuardLock()
         {
-            if (vocation == currentThreadId)
+            using namespace std::chrono;
+            using namespace std::chrono_literals;
+
+            auto vocation = std::thread::id(0);
+            auto currentThreadId = std::this_thread::get_id();
+
+            /* non-blocking guard to prevent onetime access to the
+             * `toCleanupInstances` from different threads
+             */
+            while (!instanceAccessGuard.compare_exchange_strong(
+                vocation, currentThreadId, std::memory_order_release))
             {
-                return false;
+                if (vocation == currentThreadId)
+                {
+                    return false;
+                }
+                std::this_thread::sleep_for(10ms);
+                vocation = std::thread::id(0);
             }
-            std::this_thread::sleep_for(10ms);
-            vocation = std::thread::id(0);
+
+            return true;
         }
 
-        return true;
-    }
+        /**
+         * @brief Unlock access to the DBusInstances internal resources.
+         *
+         * @note Thread safe, non-blocking (like spin-lock), global scope
+         *
+         */
+        inline static void instanceAccessGuardUnlock()
+        {
+            instanceAccessGuard.store(std::thread::id(0));
+        }
+        bool isLocked;
 
-    /**
-     * @brief Unlock access to the DBusInstances internal resources.
-     *
-     * @note Thread safe, non-blocking (like spin-lock), global scope
-     *
-     */
-    inline static void instanceAccessGuardUnlock()
-    {
-        instanceAccessGuard.store(std::thread::id(0));
-    }
+      public:
+        explicit AccessGuard() noexcept : isLocked(false)
+        {
+            isLocked = instanceAccessGuardLock();
+        }
+        ~AccessGuard()
+        {
+            if (isLocked)
+                instanceAccessGuardUnlock();
+        }
+    };
 };
 
 class DBusQuery : public IQuery, public virtual Event<app::query::QueryEvent>
@@ -567,6 +587,18 @@ class DBusQuery : public IQuery, public virtual Event<app::query::QueryEvent>
         return std::forward<const QueryFields>(fields);
     }
 
+    /**
+     * @brief Get setters for specified interfaces.
+     *        The setters dictionary origin from endpoint dictionary
+     *
+     * @throw std::invalid_argument       - setters not found for the specified
+     *                                      interface
+     * @param interface                   - interface name
+     * @return const DBusPropertySetters& - setters for the interface properties
+     */
+    const DBusPropertySetters&
+        dbusPropertySetters(const InterfaceName& interface) const;
+
     void configure(std::reference_wrapper<IEntity> entity) override
     {
         registerObjectCreationObserver(entity);