Bump revision up to v0.8.2

This should not be required in C++17 because there is an appropriate
class template deduction rule [1] which infers that it's going to be a
weak_ptr<T> when constructing from a shared_ptr<T>. However, in
clang/LLVM's libcxx C++ STL implementation this only got implemented in
May 2020 [2].

[1] https://en.cppreference.com/w/cpp/memory/weak_ptr/deduction_guides
[2] https://reviews.llvm.org/D69603

CMakeLists.txt

@@ -2,9 +2,9 @@
 # PROJECT INFORMATION
 #-------------------------------
 
-cmake_minimum_required(VERSION 3.12)
+cmake_minimum_required(VERSION 3.13)
 
-project(sdbus-c++ VERSION 0.8.1 LANGUAGES C CXX)
+project(sdbus-c++ VERSION 0.8.2 LANGUAGES C CXX)
 
 include(GNUInstallDirs) # Installation directories for `install` command and pkgconfig file
 

ChangeLog

@@ -147,3 +147,16 @@ v0.8.1
 - Switch to full C++17 support
 - Switch to more modern CMake (>=3.12)
 - Provide better names to event loop-related IConnection methods, keep old ones marked as deprecated for backwards compatibility
+
+v0.8.2
+- Introduce support for cancellable async calls
+- Add getObjectPath() for proxy and object classes
+- Sanitize names of namespaces/methods/signals/properties/arguments in sdbus-c++-xml2cpp
+- Fix delivery of signals to multiple proxies subscribed to them
+- Fix file existence condition in sdbus-c++-xml2cpp
+- Fix CallData race condition in Proxy::callMethod
+- Fix integration tests for libsystemd older than 242
+- Fix installation of public sd-bus headers in internal libsystemd build
+- Fix integration test cases failing in specific situations
+- Fix build with clang 9.0.1 and libcxx
+- Fix potential data race in Proxy's condition variable

cmake/LibsystemdExternalProject.cmake

@@ -41,8 +41,8 @@ ExternalProject_Add(LibsystemdBuildProject
                               COMMAND ${MESON} --prefix=<INSTALL_DIR> --buildtype=${LIBSYSTEMD_BUILD_TYPE} -Dstatic-libsystemd=pic <SOURCE_DIR> <BINARY_DIR>
                     BUILD_COMMAND     ${BUILD_VERSION_H}
                           COMMAND     ${NINJA} -C <BINARY_DIR> libsystemd.a
-                    BUILD_ALWAYS      1
-                    INSTALL_COMMAND   ${CMAKE_COMMAND} -E copy_directory <SOURCE_DIR>/src/libsystemd <INSTALL_DIR>/include
+                    BUILD_ALWAYS      0
+                    INSTALL_COMMAND   ${CMAKE_COMMAND} -E copy_directory <SOURCE_DIR>/src/systemd <INSTALL_DIR>/include/systemd
                     LOG_DOWNLOAD 1 LOG_UPDATE 1 LOG_CONFIGURE 1 LOG_BUILD 1)
 
 ExternalProject_Get_property(LibsystemdBuildProject SOURCE_DIR)
@@ -51,6 +51,6 @@ ExternalProject_Get_property(LibsystemdBuildProject INSTALL_DIR)
 
 add_library(Systemd::Libsystemd STATIC IMPORTED)
 set_target_properties(Systemd::Libsystemd PROPERTIES IMPORTED_LOCATION ${BINARY_DIR}/libsystemd.a)
-file(MAKE_DIRECTORY ${INSTALL_DIR}/include) # Trick for CMake to stop complaining about non-existent ${INSTALL_DIR}/include directory
+file(MAKE_DIRECTORY ${INSTALL_DIR}/include/systemd) # Trick for CMake to stop complaining about non-existent ${INSTALL_DIR}/include directory
 target_include_directories(Systemd::Libsystemd INTERFACE ${INSTALL_DIR}/include)
 target_link_libraries(Systemd::Libsystemd INTERFACE ${CAP_LIBRARIES} ${GLIBC_RT_LIBRARY} ${MOUNT_LIBRARIES})

docs/using-sdbus-c++.md

@@ -26,7 +26,7 @@ Introduction
 
 sdbus-c++ is a C++ D-Bus library built on top of [sd-bus](http://0pointer.net/blog/the-new-sd-bus-api-of-systemd.html), a lightweight D-Bus client library implemented within [systemd](https://github.com/systemd/systemd) project. It provides D-Bus functionality on a higher level of abstraction, trying to employ C++ type system to shift as much work as possible from the developer to the compiler.
 
-sdbus-c++ does not cover the entire sd-bus API, but provides tools for implementing the most common functionality - RPC method calls, signals and properties. There is room for additions and improvements, as needed and when needed.
+Although sdbus-c++ covers most of sd-bus API, it does not (yet) fully cover every sd-bus API detail. The focus is put on the most widely used functionality: D-Bus connections, object, proxies, synchronous and asynchronous method calls, signals, and properties. If you are missing a desired functionality, you are welcome to submit an issue, or, best, to contribute to sdbus-c++ by submitting a pull request.
 
 Integrating sdbus-c++ into your project
 ---------------------------------------

include/sdbus-c++/AdaptorInterfaces.h

@@ -131,6 +131,14 @@ namespace sdbus {
             getObject().unregister();
         }
 
+        /*!
+         * @brief Returns object path of the underlying DBus object
+         */
+        const std::string& getObjectPath() const
+        {
+            return getObject().getObjectPath();
+        }
+
     protected:
         using base_type = AdaptorInterfaces;
     };

include/sdbus-c++/ConvenienceApiClasses.h

@@ -42,6 +42,7 @@ namespace sdbus {
     class IProxy;
     class Variant;
     class Error;
+    class PendingAsyncCall;
 }
 
 namespace sdbus {
@@ -193,7 +194,7 @@ namespace sdbus {
         template <typename _Rep, typename _Period>
         AsyncMethodInvoker& withTimeout(const std::chrono::duration<_Rep, _Period>& timeout);
         template <typename... _Args> AsyncMethodInvoker& withArguments(_Args&&... args);
-        template <typename _Function> void uponReplyInvoke(_Function&& callback);
+        template <typename _Function> PendingAsyncCall uponReplyInvoke(_Function&& callback);
 
     private:
         IProxy& proxy_;

include/sdbus-c++/ConvenienceApiClasses.inl

@@ -576,7 +576,7 @@ namespace sdbus {
     }
 
     template <typename _Function>
-    void AsyncMethodInvoker::uponReplyInvoke(_Function&& callback)
+    PendingAsyncCall AsyncMethodInvoker::uponReplyInvoke(_Function&& callback)
     {
         assert(method_.isValid()); // onInterface() must be placed/called prior to this function
 
@@ -594,7 +594,7 @@ namespace sdbus {
             sdbus::apply(callback, error, args);
         };
 
-        proxy_.callMethod(method_, std::move(asyncReplyHandler), timeout_);
+        return proxy_.callMethod(method_, std::move(asyncReplyHandler), timeout_);
     }
 
     /*** ---------------- ***/

include/sdbus-c++/IObject.h

@@ -435,6 +435,11 @@ namespace sdbus {
          * @throws sdbus::Error in case of failure
          */
         [[nodiscard]] SignalEmitter emitSignal(const std::string& signalName);
+
+        /*!
+         * @brief Returns object path of the underlying DBus object
+         */
+        virtual const std::string& getObjectPath() const = 0;
     };
 
     // Out-of-line member definitions

include/sdbus-c++/IProxy.h

@@ -38,6 +38,10 @@ namespace sdbus {
     class MethodCall;
     class MethodReply;
     class IConnection;
+    class PendingAsyncCall;
+    namespace internal {
+        class Proxy;
+    }
 }
 
 namespace sdbus {
@@ -108,6 +112,7 @@ namespace sdbus {
          * @param[in] message Message representing an async method call
          * @param[in] asyncReplyCallback Handler for the async reply
          * @param[in] timeout Timeout for dbus call in microseconds
+         * @return Cookie for the the pending asynchronous call
          *
          * The call is non-blocking. It doesn't wait for the reply. Once the reply arrives,
          * the provided async reply handler will get invoked from the context of the connection
@@ -117,13 +122,13 @@ namespace sdbus {
          *
          * @throws sdbus::Error in case of failure
          */
-        virtual void callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout = 0) = 0;
+        virtual PendingAsyncCall callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout = 0) = 0;
 
         /*!
          * @copydoc IProxy::callMethod(const MethodCall&,async_reply_handler,uint64_t)
          */
         template <typename _Rep, typename _Period>
-        void callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, const std::chrono::duration<_Rep, _Period>& timeout);
+        PendingAsyncCall callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, const std::chrono::duration<_Rep, _Period>& timeout);
 
         /*!
          * @brief Registers a handler for the desired signal emitted by the proxied D-Bus object
@@ -261,6 +266,51 @@ namespace sdbus {
          * @throws sdbus::Error in case of failure
          */
         [[nodiscard]] PropertySetter setProperty(const std::string& propertyName);
+
+        /*!
+         * @brief Returns object path of the underlying DBus object
+         */
+        virtual const std::string& getObjectPath() const = 0;
+    };
+
+    /********************************************//**
+     * @class PendingAsyncCall
+     *
+     * PendingAsyncCall represents a simple handle type to cancel the delivery
+     * of the asynchronous D-Bus call result to the application.
+     *
+     * The handle is lifetime-independent from the originating Proxy object.
+     * It's safe to call its methods even after the Proxy has gone.
+     *
+     ***********************************************/
+    class PendingAsyncCall
+    {
+    public:
+        /*!
+         * @brief Cancels the delivery of the pending asynchronous call result
+         *
+         * This function effectively removes the callback handler registered to the
+         * async D-Bus method call result delivery. Does nothing if the call was
+         * completed already, or if the originating Proxy object has gone meanwhile.
+         */
+        void cancel();
+
+        /*!
+         * @brief Answers whether the asynchronous call is still pending
+         *
+         * @return True if the call is pending, false if the call has been fully completed
+         *
+         * Pending call in this context means a call whose results have not arrived, or
+         * have arrived and are currently being processed by the callback handler.
+         */
+        bool isPending() const;
+
+    private:
+        friend internal::Proxy;
+        PendingAsyncCall(std::weak_ptr<void> callData);
+
+    private:
+        std::weak_ptr<void> callData_;
     };
 
     // Out-of-line member definitions
@@ -273,10 +323,10 @@ namespace sdbus {
     }
 
     template <typename _Rep, typename _Period>
-    inline void IProxy::callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, const std::chrono::duration<_Rep, _Period>& timeout)
+    inline PendingAsyncCall IProxy::callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, const std::chrono::duration<_Rep, _Period>& timeout)
     {
         auto microsecs = std::chrono::duration_cast<std::chrono::microseconds>(timeout);
-        callMethod(message, std::move(asyncReplyCallback), microsecs.count());
+        return callMethod(message, std::move(asyncReplyCallback), microsecs.count());
     }
 
     inline MethodInvoker IProxy::callMethod(const std::string& methodName)

include/sdbus-c++/ProxyInterfaces.h

@@ -165,6 +165,14 @@ namespace sdbus {
             getProxy().unregister();
         }
 
+        /*!
+         * @brief Returns object path of the underlying DBus object
+         */
+        const std::string& getObjectPath() const
+        {
+            return getProxy().getObjectPath();
+        }
+
     protected:
         using base_type = ProxyInterfaces;
     };

src/Object.cpp

@@ -225,6 +225,11 @@ sdbus::IConnection& Object::getConnection() const
     return dynamic_cast<sdbus::IConnection&>(connection_);
 }
 
+const std::string& Object::getObjectPath() const
+{
+    return objectPath_;
+}
+
 const std::vector<sd_bus_vtable>& Object::createInterfaceVTable(InterfaceData& interfaceData)
 {
     auto& vtable = interfaceData.vtable;

src/Object.h

@@ -101,6 +101,7 @@ namespace sdbus::internal {
         bool hasObjectManager() const override;
 
         sdbus::IConnection& getConnection() const override;
+        const std::string& getObjectPath() const override;
 
     private:
         using InterfaceName = std::string;

src/Proxy.cpp

@@ -93,16 +93,20 @@ MethodReply Proxy::callMethod(const MethodCall& message, uint64_t timeout)
     return sendMethodCallMessageAndWaitForReply(message, timeout);
 }
 
-void Proxy::callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout)
+PendingAsyncCall Proxy::callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout)
 {
     SDBUS_THROW_ERROR_IF(!message.isValid(), "Invalid async method call message provided", EINVAL);
 
     auto callback = (void*)&Proxy::sdbus_async_reply_handler;
-    auto callData = std::make_unique<AsyncCalls::CallData>(AsyncCalls::CallData{*this, std::move(asyncReplyCallback), {}});
+    auto callData = std::make_shared<AsyncCalls::CallData>(AsyncCalls::CallData{*this, std::move(asyncReplyCallback), {}});
+    auto weakData = std::weak_ptr<AsyncCalls::CallData>{callData};
 
     callData->slot = message.send(callback, callData.get(), timeout);
 
-    pendingAsyncCalls_.addCall(callData->slot.get(), std::move(callData));
+    auto slotPtr = callData->slot.get();
+    pendingAsyncCalls_.addCall(slotPtr, std::move(callData));
+
+    return {weakData};
 }
 
 MethodReply Proxy::sendMethodCallMessageAndWaitForReply(const MethodCall& message, uint64_t timeout)
@@ -123,8 +127,8 @@ MethodReply Proxy::sendMethodCallMessageAndWaitForReply(const MethodCall& messag
 
 void Proxy::SyncCallReplyData::sendMethodReplyToWaitingThread(MethodReply& reply, const Error* error)
 {
-    SCOPE_EXIT{ cond_.notify_one(); };
-    std::unique_lock<std::mutex> lock{mutex_};
+    std::unique_lock lock{mutex_};
+    SCOPE_EXIT{ cond_.notify_one(); }; // This must happen before unlocking the mutex to avoid potential data race on spurious wakeup in the waiting thread
     SCOPE_EXIT{ arrived_ = true; };
 
     //error_ = nullptr; // Necessary if SyncCallReplyData instance is thread_local
@@ -137,7 +141,7 @@ void Proxy::SyncCallReplyData::sendMethodReplyToWaitingThread(MethodReply& reply
 
 MethodReply Proxy::SyncCallReplyData::waitForMethodReply()
 {
-    std::unique_lock<std::mutex> lock{mutex_};
+    std::unique_lock lock{mutex_};
     cond_.wait(lock, [this](){ return arrived_; });
 
     //arrived_ = false; // Necessary if SyncCallReplyData instance is thread_local
@@ -194,19 +198,27 @@ void Proxy::unregister()
     interfaces_.clear();
 }
 
+const std::string& Proxy::getObjectPath() const
+{
+    return objectPath_;
+}
+
 int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error */*retError*/)
 {
     auto* asyncCallData = static_cast<AsyncCalls::CallData*>(userData);
     assert(asyncCallData != nullptr);
     assert(asyncCallData->callback);
     auto& proxy = asyncCallData->proxy;
+    auto slot = asyncCallData->slot.get();
 
+    // We are removing the CallData item at the complete scope exit, after the callback has been invoked.
+    // We can't do it earlier (before callback invocation for example), because CallBack data (slot release)
+    // is the synchronization point between callback invocation and Proxy::unregister.
     SCOPE_EXIT
     {
-        // Slot may be null if we're doing blocking synchronous call implemented by means of asynchronous call,
-        // because in that case the call data is still alive on the stack, we don't need to manage it separately.
-        if (asyncCallData->slot)
-            proxy.pendingAsyncCalls_.removeCall(asyncCallData->slot.get());
+        // Remove call meta-data if it's a real async call (a sync call done in terms of async has slot == nullptr)
+        if (slot)
+            proxy.pendingAsyncCalls_.removeCall(slot);
     };
 
     auto message = Message::Factory::create<MethodReply>(sdbusMessage, &proxy.connection_->getSdBusInterface());
@@ -238,7 +250,35 @@ int Proxy::sdbus_signal_handler(sd_bus_message *sdbusMessage, void *userData, sd
 
     callback(message);
 
-    return 1;
+    return 0;
+}
+
+}
+
+namespace sdbus {
+
+PendingAsyncCall::PendingAsyncCall(std::weak_ptr<void> callData)
+    : callData_(std::move(callData))
+{
+}
+
+void PendingAsyncCall::cancel()
+{
+    if (auto ptr = callData_.lock(); ptr != nullptr)
+    {
+        auto* callData = static_cast<internal::Proxy::AsyncCalls::CallData*>(ptr.get());
+        callData->proxy.pendingAsyncCalls_.removeCall(callData->slot.get());
+
+        // At this point, the callData item is being deleted, leading to the release of the
+        // sd-bus slot pointer. This release locks the global sd-bus mutex. If the async
+        // callback is currently being processed, the sd-bus mutex is locked by the event
+        // loop thread, thus access to the callData item is synchronized and thread-safe.
+    }
+}
+
+bool PendingAsyncCall::isPending() const
+{
+    return !callData_.expired();
 }
 
 }

src/Proxy.h

@@ -52,7 +52,7 @@ namespace sdbus::internal {
 
         MethodCall createMethodCall(const std::string& interfaceName, const std::string& methodName) override;
         MethodReply callMethod(const MethodCall& message, uint64_t timeout) override;
-        void callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout) override;
+        PendingAsyncCall callMethod(const MethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout) override;
 
         void registerSignalHandler( const std::string& interfaceName
                                   , const std::string& signalName
@@ -60,6 +60,8 @@ namespace sdbus::internal {
         void finishRegistration() override;
         void unregister() override;
 
+        const std::string& getObjectPath() const override;
+
     private:
         class SyncCallReplyData
         {
@@ -81,6 +83,8 @@ namespace sdbus::internal {
         static int sdbus_signal_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError);
 
     private:
+        friend PendingAsyncCall;
+
         std::unique_ptr< sdbus::internal::IConnection
                        , std::function<void(sdbus::internal::IConnection*)>
                        > connection_;
@@ -119,29 +123,42 @@ namespace sdbus::internal {
                 clear();
             }
 
-            bool addCall(void* slot, std::unique_ptr<CallData>&& asyncCallData)
+            bool addCall(void* slot, std::shared_ptr<CallData> asyncCallData)
             {
                 std::lock_guard lock(mutex_);
                 return calls_.emplace(slot, std::move(asyncCallData)).second;
             }
 
-            bool removeCall(void* slot)
+            void removeCall(void* slot)
             {
-                std::lock_guard lock(mutex_);
-                return calls_.erase(slot) > 0;
+                std::unique_lock lock(mutex_);
+                if (auto it = calls_.find(slot); it != calls_.end())
+                {
+                    auto callData = std::move(it->second);
+                    calls_.erase(it);
+                    lock.unlock();
+
+                    // Releasing call slot pointer acquires global sd-bus mutex. We have to perform the release
+                    // out of the `mutex_' critical section here, because if the `removeCall` is called by some
+                    // thread and at the same time Proxy's async reply handler (which already holds global sd-bus
+                    // mutex) is in progress in a different thread, we get double-mutex deadlock.
+                }
             }
 
             void clear()
             {
-                std::unique_lock<std::mutex> lock(mutex_);
+                std::unique_lock lock(mutex_);
                 auto asyncCallSlots = std::move(calls_);
-                // Perform releasing of sd-bus slots outside of the calls_ critical section which avoids
-                // double mutex dead lock when the async reply handler is invoked at the same time.
                 lock.unlock();
+
+                // Releasing call slot pointer acquires global sd-bus mutex. We have to perform the release
+                // out of the `mutex_' critical section here, because if the `clear` is called by some thread
+                // and at the same time Proxy's async reply handler (which already holds global sd-bus
+                // mutex) is in progress in a different thread, we get double-mutex deadlock.
             }
 
         private:
-            std::unordered_map<void*, std::unique_ptr<CallData>> calls_;
+            std::unordered_map<void*, std::shared_ptr<CallData>> calls_;
             std::mutex mutex_;
         } pendingAsyncCalls_;
     };

tests/integrationtests/AdaptorAndProxy_test.cpp

@@ -71,7 +71,8 @@ public:
         s_connection->releaseName(INTERFACE_NAME);
     }
 
-    static bool waitUntil(std::atomic<bool>& flag, std::chrono::milliseconds timeout = 5s)
+    template <typename _Fnc>
+    static bool waitUntil(_Fnc&& fnc, std::chrono::milliseconds timeout = 5s)
     {
         std::chrono::milliseconds elapsed{};
         std::chrono::milliseconds step{5ms};
@@ -80,11 +81,16 @@ public:
             elapsed += step;
             if (elapsed > timeout)
                 return false;
-        } while (!flag);
+        } while (!fnc());
 
         return true;
     }
 
+    static bool waitUntil(std::atomic<bool>& flag, std::chrono::milliseconds timeout = 5s)
+    {
+        return waitUntil([&flag]() -> bool { return flag; }, timeout);
+    }
+
 private:
     void SetUp() override
     {
@@ -110,6 +116,8 @@ std::unique_ptr<sdbus::IConnection> AdaptorAndProxyFixture::s_connection = sdbus
 
 }
 
+using SdbusTestObject = AdaptorAndProxyFixture;
+
 /*-------------------------------------*/
 /* --          TEST CASES           -- */
 /*-------------------------------------*/
@@ -127,8 +135,6 @@ TEST(AdaptorAndProxy, CanBeConstructedSuccesfully)
 
 // Methods
 
-using SdbusTestObject = AdaptorAndProxyFixture;
-
 TEST_F(SdbusTestObject, CallsEmptyMethodSuccesfully)
 {
     ASSERT_NO_THROW(m_proxy->noArgNoReturn());
@@ -210,7 +216,7 @@ TEST_F(SdbusTestObject, CallsMethodWithSignatureSuccesfully)
 
 TEST_F(SdbusTestObject, CallsMethodWithObjectPathSuccesfully)
 {
-    auto resObjectPath = m_proxy->getObjectPath();
+    auto resObjectPath = m_proxy->getObjPath();
     ASSERT_THAT(resObjectPath, Eq(OBJECT_PATH_VALUE));
 }
 
@@ -385,6 +391,51 @@ TEST_F(SdbusTestObject, InvokesMethodAsynchronouslyOnClientSide)
     ASSERT_THAT(future.get(), Eq(100));
 }
 
+TEST_F(SdbusTestObject, AnswersThatAsyncCallIsPendingIfItIsInProgress)
+{
+    m_proxy->installDoOperationClientSideAsyncReplyHandler([&](uint32_t /*res*/, const sdbus::Error* /*err*/){});
+
+    auto call = m_proxy->doOperationClientSideAsync(100);
+
+    ASSERT_TRUE(call.isPending());
+}
+
+TEST_F(SdbusTestObject, CancelsPendingAsyncCallOnClientSide)
+{
+    std::promise<uint32_t> promise;
+    auto future = promise.get_future();
+    m_proxy->installDoOperationClientSideAsyncReplyHandler([&](uint32_t /*res*/, const sdbus::Error* /*err*/){ promise.set_value(1); });
+    auto call = m_proxy->doOperationClientSideAsync(100);
+
+    call.cancel();
+
+    ASSERT_THAT(future.wait_for(300ms), Eq(std::future_status::timeout));
+}
+
+TEST_F(SdbusTestObject, AnswersThatAsyncCallIsNotPendingAfterItHasBeenCancelled)
+{
+    std::promise<uint32_t> promise;
+    auto future = promise.get_future();
+    m_proxy->installDoOperationClientSideAsyncReplyHandler([&](uint32_t /*res*/, const sdbus::Error* /*err*/){ promise.set_value(1); });
+    auto call = m_proxy->doOperationClientSideAsync(100);
+
+    call.cancel();
+
+    ASSERT_FALSE(call.isPending());
+}
+
+TEST_F(SdbusTestObject, AnswersThatAsyncCallIsNotPendingAfterItHasBeenCompleted)
+{
+    std::promise<uint32_t> promise;
+    auto future = promise.get_future();
+    m_proxy->installDoOperationClientSideAsyncReplyHandler([&](uint32_t /*res*/, const sdbus::Error* /*err*/){ promise.set_value(1); });
+
+    auto call = m_proxy->doOperationClientSideAsync(0);
+    (void) future.get(); // Wait for the call to finish
+
+    ASSERT_TRUE(waitUntil([&call](){ return !call.isPending(); }));
+}
+
 TEST_F(SdbusTestObject, InvokesErroneousMethodAsynchronouslyOnClientSide)
 {
     std::promise<uint32_t> promise;
@@ -455,6 +506,18 @@ TEST_F(SdbusTestObject, EmitsSimpleSignalSuccesfully)
     ASSERT_TRUE(waitUntil(m_proxy->m_gotSimpleSignal));
 }
 
+TEST_F(SdbusTestObject, EmitsSimpleSignalToMultipleProxiesSuccesfully)
+{
+    auto proxy1 = std::make_unique<TestingProxy>(*s_connection, INTERFACE_NAME, OBJECT_PATH);
+    auto proxy2 = std::make_unique<TestingProxy>(*s_connection, INTERFACE_NAME, OBJECT_PATH);
+
+    m_adaptor->emitSimpleSignal();
+
+    ASSERT_TRUE(waitUntil(m_proxy->m_gotSimpleSignal));
+    ASSERT_TRUE(waitUntil(proxy1->m_gotSimpleSignal));
+    ASSERT_TRUE(waitUntil(proxy2->m_gotSimpleSignal));
+}
+
 TEST_F(SdbusTestObject, EmitsSignalWithMapSuccesfully)
 {
     m_adaptor->emitSignalWithMap({{0, "zero"}, {1, "one"}});
@@ -622,7 +685,20 @@ TEST_F(SdbusTestObject, EmitsInterfacesAddedSignalForSelectedObjectInterfaces)
         EXPECT_THAT(objectPath, Eq(OBJECT_PATH));
         EXPECT_THAT(interfacesAndProperties, SizeIs(1));
         EXPECT_THAT(interfacesAndProperties.count(INTERFACE_NAME), Eq(1));
+#if LIBSYSTEMD_VERSION<=244
+        // Up to sd-bus v244, all properties are added to the list, i.e. `state', `action', and `blocking' in this case.
         EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(3));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("state"));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("action"));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("blocking"));
+#else
+        // Since v245 sd-bus does not add to the InterfacesAdded signal message the values of properties marked only
+        // for invalidation on change, which makes the behavior consistent with the PropertiesChangedSignal.
+        // So in this specific instance, `action' property is no more added to the list.
+        EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(2));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("state"));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("blocking"));
+#endif
         signalReceived = true;
     };
 
@@ -639,7 +715,20 @@ TEST_F(SdbusTestObject, EmitsInterfacesAddedSignalForAllObjectInterfaces)
     {
         EXPECT_THAT(objectPath, Eq(OBJECT_PATH));
         EXPECT_THAT(interfacesAndProperties, SizeIs(5)); // INTERFACE_NAME + 4 standard interfaces
-        EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(3)); // 3 properties under INTERFACE_NAME
+#if LIBSYSTEMD_VERSION<=244
+        // Up to sd-bus v244, all properties are added to the list, i.e. `state', `action', and `blocking' in this case.
+        EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(3));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("state"));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("action"));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("blocking"));
+#else
+        // Since v245 sd-bus does not add to the InterfacesAdded signal message the values of properties marked only
+        // for invalidation on change, which makes the behavior consistent with the PropertiesChangedSignal.
+        // So in this specific instance, `action' property is no more added to the list.
+        EXPECT_THAT(interfacesAndProperties.at(INTERFACE_NAME), SizeIs(2));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("state"));
+        EXPECT_TRUE(interfacesAndProperties.at(INTERFACE_NAME).count("blocking"));
+#endif
         signalReceived = true;
     };
 

tests/integrationtests/TestingAdaptor.h

@@ -154,7 +154,7 @@ protected:
     {
         return SIGNATURE_VALUE;
     }
-    sdbus::ObjectPath getObjectPath() const override
+    sdbus::ObjectPath getObjPath() const override
     {
         return OBJECT_PATH_VALUE;
     }

tests/integrationtests/TestingProxy.h

@@ -43,6 +43,12 @@ public:
         registerProxy();
     }
 
+    TestingProxy(sdbus::IConnection& connection, std::string destination, std::string objectPath)
+        : ProxyInterfaces(connection, std::move(destination), std::move(objectPath))
+    {
+        registerProxy();
+    }
+
     ~TestingProxy()
     {
         unregisterProxy();

tests/integrationtests/adaptor-glue.h

@@ -97,7 +97,7 @@ protected:
         });
 
         object_.registerMethod("getSignature").onInterface(INTERFACE_NAME).implementedAs([this](){ return this->getSignature(); });
-        object_.registerMethod("getObjectPath").onInterface(INTERFACE_NAME).implementedAs([this](){ return this->getObjectPath(); });
+        object_.registerMethod("getObjPath").onInterface(INTERFACE_NAME).implementedAs([this](){ return this->getObjPath(); });
         object_.registerMethod("getUnixFd").onInterface(INTERFACE_NAME).implementedAs([this](){ return this->getUnixFd(); });
 
         object_.registerMethod("getComplex").onInterface(INTERFACE_NAME).implementedAs([this](){ return this->getComplex(); }).markAsDeprecated();
@@ -111,7 +111,7 @@ protected:
 
         // registration of signals is optional, it is useful because of introspection
         object_.registerSignal("simpleSignal").onInterface(INTERFACE_NAME).markAsDeprecated();
-        // Note: sd-bus of libsystemd up to v244 has a bug where it doesn't generate signal parameter names in introspection XML. Signal param names commented temporarily.
+        // Note: sd-bus of libsystemd up to (including) v244 has a bug where it doesn't generate signal parameter names in introspection XML. Signal param names commented temporarily.
         object_.registerSignal("signalWithMap").onInterface(INTERFACE_NAME).withParameters<std::map<int32_t, std::string>>(/*"aMap"*/);
         object_.registerSignal("signalWithVariant").onInterface(INTERFACE_NAME).withParameters<sdbus::Variant>(/*"aVariant"*/);
 
@@ -168,7 +168,7 @@ protected:
     virtual uint32_t doOperation(uint32_t param) = 0;
     virtual void doOperationAsync(uint32_t param, sdbus::Result<uint32_t> result) = 0;
     virtual sdbus::Signature getSignature() const  = 0;
-    virtual sdbus::ObjectPath getObjectPath() const = 0;
+    virtual sdbus::ObjectPath getObjPath() const = 0;
     virtual sdbus::UnixFd getUnixFd() const  = 0;
     virtual ComplexType getComplex() const = 0;
     virtual void throwError() const = 0;
@@ -251,19 +251,35 @@ R"delimiter(<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspectio
    <arg type="a{t(a{ya(obva{is})}gs)}" direction="out"/>
    <annotation name="org.freedesktop.DBus.Deprecated" value="true"/>
   </method>
-  <method name="getInt">
-   <arg type="i" name="anInt" direction="out"/>
+  <method name="getInt">)delimiter"
+#if LIBSYSTEMD_VERSION>=242
+R"delimiter(
+   <arg type="i" name="anInt" direction="out"/>)delimiter"
+#else
+R"delimiter(
+   <arg type="i" direction="out"/>)delimiter"
+#endif
+R"delimiter(
   </method>
   <method name="getInts16FromStruct">
    <arg type="(yndsan)" direction="in"/>
    <arg type="an" direction="out"/>
   </method>
-  <method name="getMapOfVariants">
+  <method name="getMapOfVariants">)delimiter"
+#if LIBSYSTEMD_VERSION>=242
+R"delimiter(
    <arg type="ai" name="x" direction="in"/>
    <arg type="(vv)" name="y" direction="in"/>
-   <arg type="a{iv}" name="aMapOfVariants" direction="out"/>
+   <arg type="a{iv}" name="aMapOfVariants" direction="out"/>)delimiter"
+#else
+R"delimiter(
+   <arg type="ai" direction="in"/>
+   <arg type="(vv)" direction="in"/>
+   <arg type="a{iv}" direction="out"/>)delimiter"
+#endif
+R"delimiter(
   </method>
-  <method name="getObjectPath">
+  <method name="getObjPath">
    <arg type="o" direction="out"/>
   </method>
   <method name="getSignature">
@@ -279,10 +295,19 @@ R"delimiter(<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspectio
   <method name="getUnixFd">
    <arg type="h" direction="out"/>
   </method>
-  <method name="multiply">
+  <method name="multiply">)delimiter"
+#if LIBSYSTEMD_VERSION>=242
+R"delimiter(
    <arg type="x" name="a" direction="in"/>
    <arg type="d" name="b" direction="in"/>
-   <arg type="d" name="result" direction="out"/>
+   <arg type="d" name="result" direction="out"/>)delimiter"
+#else
+R"delimiter(
+   <arg type="x" direction="in"/>
+   <arg type="d" direction="in"/>
+   <arg type="d" direction="out"/>)delimiter"
+#endif
+R"delimiter(
   </method>
   <method name="multiplyWithNoReply">
    <arg type="x" direction="in"/>

tests/integrationtests/proxy-glue.h

@@ -156,15 +156,15 @@ public:
         return result;
     }
 
-    void doOperationClientSideAsync(uint32_t param)
+    sdbus::PendingAsyncCall doOperationClientSideAsync(uint32_t param)
     {
-        object_.callMethodAsync("doOperation")
-               .onInterface(INTERFACE_NAME)
-               .withArguments(param)
-               .uponReplyInvoke([this](const sdbus::Error* error, uint32_t returnValue)
-                                {
-                                    this->onDoOperationReply(returnValue, error);
-                                });
+        return object_.callMethodAsync("doOperation")
+                      .onInterface(INTERFACE_NAME)
+                      .withArguments(param)
+                      .uponReplyInvoke([this](const sdbus::Error* error, uint32_t returnValue)
+                                       {
+                                           this->onDoOperationReply(returnValue, error);
+                                       });
     }
 
     void doErroneousOperationClientSideAsync()
@@ -197,10 +197,10 @@ public:
         return result;
     }
 
-    sdbus::ObjectPath getObjectPath()
+    sdbus::ObjectPath getObjPath()
     {
         sdbus::ObjectPath result;
-        object_.callMethod("getObjectPath").onInterface(INTERFACE_NAME).storeResultsTo(result);
+        object_.callMethod("getObjPath").onInterface(INTERFACE_NAME).storeResultsTo(result);
         return result;
     }
 

tests/stresstests/concatenator-proxy.h

@@ -33,9 +33,9 @@ protected:
     virtual void onConcatenateReply(const std::string& result, const sdbus::Error* error) = 0;
 
 public:
-    void concatenate(const std::map<std::string, sdbus::Variant>& params)
+    sdbus::PendingAsyncCall concatenate(const std::map<std::string, sdbus::Variant>& params)
     {
-        proxy_.callMethodAsync("concatenate").onInterface(INTERFACE_NAME).withArguments(params).uponReplyInvoke([this](const sdbus::Error* error, const std::string& result){ this->onConcatenateReply(result, error); });
+        return proxy_.callMethodAsync("concatenate").onInterface(INTERFACE_NAME).withArguments(params).uponReplyInvoke([this](const sdbus::Error* error, const std::string& result){ this->onConcatenateReply(result, error); });
     }
 
 private:

tools/xml2cpp-codegen/AdaptorGenerator.cpp

@@ -164,6 +164,7 @@ std::tuple<std::string, std::string> AdaptorGenerator::processMethods(const Node
     for (const auto& method : methods)
     {
         auto methodName = method->get("name");
+        auto methodNameSafe = mangle_name(methodName);
 
         auto annotations = getAnnotations(*method);
         bool async{false};
@@ -219,7 +220,7 @@ std::tuple<std::string, std::string> AdaptorGenerator::processMethods(const Node
                 << "[this]("
                 << (async ? "sdbus::Result<" + outArgsToType(outArgs, true) + ">&& result" + (argTypeStr.empty() ? "" : ", ") : "")
                 << argTypeStr
-                << "){ " << (async ? "" : "return ") << "this->" << methodName << "("
+                << "){ " << (async ? "" : "return ") << "this->" << methodNameSafe << "("
                 << (async ? "std::move(result)"s + (argTypeStr.empty() ? "" : ", ") : "")
                 << argStr << "); })"
                 << annotationRegistration << ";" << endl;
@@ -227,7 +228,7 @@ std::tuple<std::string, std::string> AdaptorGenerator::processMethods(const Node
         declarationSS << tab
                 << "virtual "
                 << (async ? "void" : outArgsToType(outArgs))
-                << " " << methodName
+                << " " << methodNameSafe
                 << "("
                 << (async ? "sdbus::Result<" + outArgsToType(outArgs, true) + ">&& result" + (argTypeStr.empty() ? "" : ", ") : "")
                 << argTypeStr
@@ -279,6 +280,7 @@ std::tuple<std::string, std::string> AdaptorGenerator::processSignals(const Node
 
         auto nameWithCapFirstLetter = name;
         nameWithCapFirstLetter[0] = std::toupper(nameWithCapFirstLetter[0]);
+        nameWithCapFirstLetter = mangle_name(nameWithCapFirstLetter);
 
         signalMethodSS << tab << "void emit" << nameWithCapFirstLetter << "(" << argTypeStr << ")" << endl
                 << tab << "{" << endl
@@ -305,6 +307,7 @@ std::tuple<std::string, std::string> AdaptorGenerator::processProperties(const N
     for (const auto& property : properties)
     {
         auto propertyName = property->get("name");
+        auto propertyNameSafe = mangle_name(propertyName);
         auto propertyAccess = property->get("access");
         auto propertySignature = property->get("type");
 
@@ -336,23 +339,23 @@ std::tuple<std::string, std::string> AdaptorGenerator::processProperties(const N
 
         if (propertyAccess == "read" || propertyAccess == "readwrite")
         {
-            registrationSS << ".withGetter([this](){ return this->" << propertyName << "(); })";
+            registrationSS << ".withGetter([this](){ return this->" << propertyNameSafe << "(); })";
         }
 
         if (propertyAccess == "readwrite" || propertyAccess == "write")
         {
             registrationSS
                 << ".withSetter([this](" << propertyTypeArg << ")"
-                   "{ this->" << propertyName << "(" << propertyArg << "); })";
+                   "{ this->" << propertyNameSafe << "(" << propertyArg << "); })";
         }
 
         registrationSS << annotationRegistration;
         registrationSS << ";" << endl;
 
         if (propertyAccess == "read" || propertyAccess == "readwrite")
-            declarationSS << tab << "virtual " << propertyType << " " << propertyName << "() = 0;" << endl;
+            declarationSS << tab << "virtual " << propertyType << " " << propertyNameSafe << "() = 0;" << endl;
         if (propertyAccess == "readwrite" || propertyAccess == "write")
-            declarationSS << tab << "virtual void " << propertyName << "(" << propertyTypeArg << ") = 0;" << endl;
+            declarationSS << tab << "virtual void " << propertyNameSafe << "(" << propertyTypeArg << ") = 0;" << endl;
     }
 
     return std::make_tuple(registrationSS.str(), declarationSS.str());

tools/xml2cpp-codegen/BaseGenerator.cpp

@@ -96,6 +96,7 @@ std::tuple<unsigned, std::string> BaseGenerator::generateNamespaces(const std::s
     {
         std::string nspace;
         getline(ss, nspace, '.');
+        nspace = mangle_name(nspace);
         body << "namespace " << nspace << " {" << endl;
         ++count;
     }
@@ -125,17 +126,18 @@ std::tuple<std::string, std::string, std::string, std::string> BaseGenerator::ar
         {
             argName = "arg" + std::to_string(i);
         }
+        auto argNameSafe = mangle_name(argName);
         auto type = signature_to_type(arg->get("type"));
         argStringsSS << "\"" << argName << "\"";
         if (!async)
         {
-            argSS << argName;
-            argTypeSS << "const " << type << "& " << argName;
+            argSS << argNameSafe;
+            argTypeSS << "const " << type << "& " << argNameSafe;
         }
         else
         {
-            argSS << "std::move(" << argName << ")";
-            argTypeSS << type << " " << argName;
+            argSS << "std::move(" << argNameSafe << ")";
+            argTypeSS << type << " " << argNameSafe;
         }
         typeSS << type;
     }

tools/xml2cpp-codegen/ProxyGenerator.cpp

@@ -135,6 +135,7 @@ std::tuple<std::string, std::string> ProxyGenerator::processMethods(const Nodes&
     for (const auto& method : methods)
     {
         auto name = method->get("name");
+        auto nameSafe = mangle_name(name);
         Nodes args = (*method)["arg"];
         Nodes inArgs = args.select("direction" , "in");
         Nodes outArgs = args.select("direction" , "out");
@@ -173,7 +174,8 @@ std::tuple<std::string, std::string> ProxyGenerator::processMethods(const Nodes&
         std::string outArgStr, outArgTypeStr;
         std::tie(outArgStr, outArgTypeStr, std::ignore, std::ignore) = argsToNamesAndTypes(outArgs);
 
-        definitionSS << tab << (async ? "void" : retType) << " " << name << "(" << inArgTypeStr << ")" << endl
+        const std::string realRetType = (async && !dontExpectReply ? "sdbus::PendingAsyncCall" : async ? "void" : retType);
+        definitionSS << tab << realRetType << " " << nameSafe << "(" << inArgTypeStr << ")" << endl
                 << tab << "{" << endl;
 
         if (!timeoutValue.empty())
@@ -186,8 +188,8 @@ std::tuple<std::string, std::string> ProxyGenerator::processMethods(const Nodes&
             definitionSS << tab << tab << retType << " result;" << endl;
         }
 
-        definitionSS << tab << tab << "proxy_.callMethod" << (async ? "Async" : "") << "(\"" << name << "\")"
-                        ".onInterface(INTERFACE_NAME)";
+        definitionSS << tab << tab << (async && !dontExpectReply ? "return " : "")
+                     << "proxy_.callMethod" << (async ? "Async" : "") << "(\"" << name << "\").onInterface(INTERFACE_NAME)";
 
         if (!timeoutValue.empty())
         {
@@ -259,6 +261,7 @@ std::string ProxyGenerator::processProperties(const Nodes& properties) const
     for (const auto& property : properties)
     {
         auto propertyName = property->get("name");
+        auto propertyNameSafe = mangle_name(propertyName);
         auto propertyAccess = property->get("access");
         auto propertySignature = property->get("type");
 
@@ -268,7 +271,7 @@ std::string ProxyGenerator::processProperties(const Nodes& properties) const
 
         if (propertyAccess == "read" || propertyAccess == "readwrite")
         {
-            propertySS << tab << propertyType << " " << propertyName << "()" << endl
+            propertySS << tab << propertyType << " " << propertyNameSafe << "()" << endl
                     << tab << "{" << endl;
             propertySS << tab << tab << "return proxy_.getProperty(\"" << propertyName << "\")"
                             ".onInterface(INTERFACE_NAME)";
@@ -277,7 +280,7 @@ std::string ProxyGenerator::processProperties(const Nodes& properties) const
 
         if (propertyAccess == "readwrite" || propertyAccess == "write")
         {
-            propertySS << tab << "void " << propertyName << "(" << propertyTypeArg << ")" << endl
+            propertySS << tab << "void " << propertyNameSafe << "(" << propertyTypeArg << ")" << endl
                     << tab << "{" << endl;
             propertySS << tab << tab << "proxy_.setProperty(\"" << propertyName << "\")"
                             ".onInterface(INTERFACE_NAME)"

tools/xml2cpp-codegen/generator_utils.cpp

@@ -7,6 +7,7 @@
 #include <map>
 
 #include "generator_utils.h"
+#include "reserved_names.h"
 
 
 std::string underscorize(const std::string& str)
@@ -142,3 +143,11 @@ std::string signature_to_type(const std::string& signature)
     _parse_signature(signature, type, i);
     return type;
 }
+
+std::string mangle_name(const std::string& name)
+{
+    if (reserved_names.find(name) != reserved_names.end())
+        return name + "_";
+    else
+        return name;
+}

tools/xml2cpp-codegen/generator_utils.h

@@ -19,4 +19,6 @@ std::string underscorize(const std::string& str);
 
 constexpr const char* getHeaderComment() noexcept { return "\n/*\n * This file was automatically generated by sdbus-c++-xml2cpp; DO NOT EDIT!\n */\n\n"; }
 
+std::string mangle_name (const std::string& name);
+
 #endif  //__SDBUSCPP_TOOLS_GENERATOR_UTILS_H

tools/xml2cpp-codegen/reserved_names.h

@@ -0,0 +1,104 @@
+#include <set>
+
+static const std::set<std::string> reserved_names = {
+    // c++ keywords taken from https://en.cppreference.com/w/cpp/keyword
+    "alignas",
+    "alignof",
+    "and",
+    "and_eq",
+    "asm",
+    "atomic_cancel",
+    "atomic_commit",
+    "atomic_noexcept",
+    "auto",
+    "bitand",
+    "bitor",
+    "bool",
+    "break",
+    "case",
+    "catch",
+    "char",
+    "char8_t",
+    "char16_t",
+    "char32_t",
+    "class",
+    "compl",
+    "concept",
+    "const",
+    "consteval",
+    "constexpr",
+    "constinit",
+    "const_cast",
+    "continue",
+    "co_await",
+    "co_return",
+    "co_yield",
+    "decltype",
+    "default",
+    "delete",
+    "do",
+    "double",
+    "dynamic_cast",
+    "else",
+    "enum",
+    "explicit",
+    "export",
+    "extern",
+    "false",
+    "float",
+    "for",
+    "friend",
+    "goto",
+    "if",
+    "inline",
+    "int",
+    "long",
+    "mutable",
+    "namespace",
+    "new",
+    "noexcept",
+    "not",
+    "not_eq",
+    "nullptr",
+    "operator",
+    "or",
+    "or_eq",
+    "private",
+    "protected",
+    "public",
+    "reflexpr",
+    "register",
+    "reinterpret_cast",
+    "requires",
+    "return",
+    "short",
+    "signed",
+    "sizeof",
+    "static",
+    "static_assert",
+    "static_cast",
+    "struct",
+    "switch",
+    "synchronized",
+    "template",
+    "this",
+    "thread_local",
+    "throw",
+    "true",
+    "try",
+    "typedef",
+    "typeid",
+    "typename",
+    "union",
+    "unsigned",
+    "using",
+    "virtual",
+    "void",
+    "volatile",
+    "wchar_t",
+    "while",
+    "xor",
+    "xor_eq",
+    // common defines
+    "NULL"
+};

tools/xml2cpp-codegen/xml2cpp.cpp

@@ -149,7 +149,7 @@ int main(int argc, char **argv)
 
             std::ifstream input(xmlFile);
 
-            if (input.bad())
+            if (input.fail())
             {
                 std::cerr << "Unable to open file " << xmlFile << endl;
                 return 1;