diff --git a/doc/using-sdbus-c++.md b/doc/using-sdbus-c++.md index b4b1dc1..1a8c00e 100644 --- a/doc/using-sdbus-c++.md +++ b/doc/using-sdbus-c++.md @@ -454,7 +454,7 @@ The generator tool takes D-Bus XML IDL description of D-Bus interfaces on its in sdbus-c++-xml2cpp database-bindings.xml --adaptor=database-server-glue.h --proxy=database-client-glue.h ``` -The adaptor header file contains classes that can be used to implement described interfaces (these classes represent object interfaces). The proxy header file contains classes that can be used to make calls to remote objects (these classes represent remote object interfaces). +The adaptor header file contains classes that can be used to implement interfaces described in the IDL (these classes represent object interfaces). The proxy header file contains classes that can be used to make calls to remote objects (these classes represent remote object interfaces). ### XML description of the Concatenator interface @@ -571,7 +571,7 @@ public: } private: - sdbus::IProxy& object_; + sdbus::IProxy& proxy_; }; }} // namespaces @@ -585,6 +585,14 @@ To implement a D-Bus object that implements all its D-Bus interfaces, we now nee How do we do that technically? Simply, our object class just needs to inherit from `AdaptorInterfaces` variadic template class. We fill its template arguments with a list of all generated interface classes. The `AdaptorInterfaces` is a convenience class that hides a few boiler-plate details. For example, in its constructor, it creates an `Object` instance, and it takes care of proper initialization of all adaptor superclasses. +In our object class we need to: + + * Give an implementation to the D-Bus object's methods by overriding corresponding virtual functions, + * call `registerAdaptor()` in the constructor, which makes the adaptor (the D-Bus object underneath it) available for remote calls, + * call `unregisterAdaptor()`, which, conversely, unregisters the adaptor from the bus. + +Calling `registerAdaptor()` and `unregisterAdaptor()` was not necessary in previous sdbus-c++ versions, as it was handled by the parent class. This was convenient, but suffered from a potential pure virtual function call issue. Only the class that implements virtual functions can do the registration, hence this slight inconvenience on user's shoulders. + ```cpp #include #include "concatenator-server-glue.h" @@ -593,8 +601,14 @@ class Concatenator : public sdbus::AdaptorInterfaces(connection, std::move(objectPath)) + : sdbus::AdaptorInterfaces(connection, std::move(objectPath)) { + registerAdaptor(); + } + + ~Concatenator() + { + unregisterAdaptor(); } protected: @@ -648,6 +662,14 @@ To implement a proxy for a remote D-Bus object, we shall create a class represen How do we do that technically? Simply, our proxy class just needs to inherit from `ProxyInterfaces` variadic template class. We fill its template arguments with a list of all generated interface classes. The `ProxyInterfaces` is a convenience class that hides a few boiler-plate details. For example, in its constructor, it can create a `Proxy` instance for us, and it takes care of proper initialization of all generated interface superclasses. +In our proxy class we need to: + + * Give an implementation to signal handlers and asynchronous method reply handlers (if any) by overriding corresponding virtual functions, + * call `registerProxy()` in the constructor, which makes the proxy (the D-Bus proxy object underneath it) ready to receive signals and async call replies, + * call `unregisterProxy()`, which, conversely, unregisters the proxy from the bus. + +Calling `registerProxy()` and `unregisterProxy()` was not necessary in previous versions of sdbus-c++, as it was handled by the parent class. This was convenient, but suffered from a potential pure virtual function call issue. Only the class that implements virtual functions can do the registration, hence this slight inconvenience on user's shoulders. + ```cpp #include #include "concatenator-client-glue.h" @@ -656,8 +678,14 @@ class ConcatenatorProxy : public sdbus::ProxyInterfaces(std::move(destination), std::move(objectPath)) + : sdbus::ProxyInterfaces(std::move(destination), std::move(objectPath)) { + registerProxy(); + } + + ~Concatenator() + { + unregisterProxy(); } protected: diff --git a/include/sdbus-c++/AdaptorInterfaces.h b/include/sdbus-c++/AdaptorInterfaces.h index 8f78d04..5b2a53d 100644 --- a/include/sdbus-c++/AdaptorInterfaces.h +++ b/include/sdbus-c++/AdaptorInterfaces.h @@ -81,6 +81,10 @@ namespace sdbus { * adaptor-side interface classes representing interfaces (with methods, signals and properties) * of the D-Bus object. * + * In the final adaptor class inherited from AdaptorInterfaces, it is necessary to finish + * adaptor registration in class constructor (finishRegistration();`), and, conversely, + * unregister the adaptor in class destructor (`unregister();`). + * ***********************************************/ template class AdaptorInterfaces @@ -88,13 +92,46 @@ namespace sdbus { , public _Interfaces... { public: + /*! + * @brief Creates object instance + * + * @param[in] connection D-Bus connection where the object will publish itself + * @param[in] objectPath Path of the D-Bus object + * + * For more information, consult @ref createObject(sdbus::IConnection&,std::string) + */ AdaptorInterfaces(IConnection& connection, std::string objectPath) : ObjectHolder(createObject(connection, std::move(objectPath))) , _Interfaces(getObject())... { - // TODO: Remove + } + + /*! + * @brief Finishes adaptor API registration and publishes the adaptor on the bus + * + * This function must be called in the constructor of the final adaptor class that implements AdaptorInterfaces. + * + * For more information, see underlying @ref IObject::finishRegistration() + */ + void registerAdaptor() + { getObject().finishRegistration(); } + + /*! + * @brief Unregisters adaptors's API and removes it from the bus + * + * This function must be called in the destructor of the final adaptor class that implements AdaptorInterfaces. + * + * For more information, see underlying @ref IObject::unregister() + */ + void unregisterAdaptor() + { + getObject().unregister(); + } + + protected: + using base_type = AdaptorInterfaces; }; } diff --git a/include/sdbus-c++/IObject.h b/include/sdbus-c++/IObject.h index f0238f5..6ca6f92 100644 --- a/include/sdbus-c++/IObject.h +++ b/include/sdbus-c++/IObject.h @@ -138,15 +138,27 @@ namespace sdbus { virtual void setInterfaceFlags(const std::string& interfaceName, Flags flags) = 0; /*! - * @brief Finishes the registration and exports object API on D-Bus + * @brief Finishes object API registration and publishes the object on the bus * * The method exports all up to now registered methods, signals and properties on D-Bus. - * Must be called only once, after all methods, signals and properties have been registered. + * Must be called after all methods, signals and properties have been registered. * * @throws sdbus::Error in case of failure */ virtual void finishRegistration() = 0; + /*! + * @brief Unregisters object's API and removes object from the bus + * + * This method unregisters the object, its interfaces, methods, signals and properties + * from the bus. Unregistration is done automatically also in object's destructor. This + * method makes sense if, in the process of object removal, we need to make sure that + * callbacks are unregistered explicitly before the final destruction of the object instance. + * + * @throws sdbus::Error in case of failure + */ + virtual void unregister() = 0; + /*! * @brief Creates a signal message * diff --git a/include/sdbus-c++/IProxy.h b/include/sdbus-c++/IProxy.h index 179a006..ace464c 100644 --- a/include/sdbus-c++/IProxy.h +++ b/include/sdbus-c++/IProxy.h @@ -146,6 +146,17 @@ namespace sdbus { */ virtual void finishRegistration() = 0; + /*! + * @brief Unregisters proxy's signal handlers and stops receving replies to pending async calls + * + * Unregistration is done automatically also in proxy's destructor. This method makes + * sense if, in the process of proxy removal, we need to make sure that callbacks + * are unregistered explicitly before the final destruction of the proxy instance. + * + * @throws sdbus::Error in case of failure + */ + virtual void unregister() = 0; + /*! * @brief Calls method on the proxied D-Bus object * diff --git a/include/sdbus-c++/Message.h b/include/sdbus-c++/Message.h index de6a8ed..5f20257 100644 --- a/include/sdbus-c++/Message.h +++ b/include/sdbus-c++/Message.h @@ -35,8 +35,7 @@ #include #include #include - -#include +#include // Forward declarations namespace sdbus { @@ -172,10 +171,12 @@ namespace sdbus { class AsyncMethodCall : public Message { public: + using Slot = std::unique_ptr>; + using Message::Message; AsyncMethodCall() = default; // Fixes gcc 6.3 error (default c-tor is not imported in above using declaration) AsyncMethodCall(MethodCall&& call) noexcept; - void send(void* callback, void* userData) const; + Slot send(void* callback, void* userData) const; }; class MethodReply : public Message diff --git a/include/sdbus-c++/ProxyInterfaces.h b/include/sdbus-c++/ProxyInterfaces.h index 3635da8..f471ed5 100644 --- a/include/sdbus-c++/ProxyInterfaces.h +++ b/include/sdbus-c++/ProxyInterfaces.h @@ -53,6 +53,7 @@ namespace sdbus { ProxyObjectHolder(std::unique_ptr&& proxy) : proxy_(std::move(proxy)) { + assert(proxy_ != nullptr); } const IProxy& getProxy() const @@ -81,6 +82,10 @@ namespace sdbus { * methods. So the _Interfaces template parameter is a list of sdbus-c++-xml2cpp-generated * proxy-side interface classes representing interfaces of the corresponding remote D-Bus object. * + * In the final proxy class inherited from ProxyInterfaces, it is necessary to finish proxy + * registration in class constructor (`finishRegistration();`), and, conversely, unregister + * the proxy in class destructor (`unregister();`). + * ***********************************************/ template class ProxyInterfaces @@ -89,57 +94,78 @@ namespace sdbus { { public: /*! - * @brief Creates fully working object proxy instance + * @brief Creates native-like proxy object instance * * @param[in] destination Bus name that provides a D-Bus object * @param[in] objectPath Path of the D-Bus object * * This constructor overload creates a proxy that manages its own D-Bus connection(s). - * For more information on its behavior, consult @ref createProxy(std::string, std::string) + * For more information on its behavior, consult @ref createProxy(std::string,std::string) */ ProxyInterfaces(std::string destination, std::string objectPath) : ProxyObjectHolder(createProxy(std::move(destination), std::move(objectPath))) , _Interfaces(getProxy())... { - // TODO: Remove - getProxy().finishRegistration(); } /*! - * @brief Creates fully working object proxy instance + * @brief Creates native-like proxy object instance * * @param[in] connection D-Bus connection to be used by the proxy object * @param[in] destination Bus name that provides a D-Bus object * @param[in] objectPath Path of the D-Bus object * * The proxy created this way just references a D-Bus connection owned and managed by the user. - * For more information on its behavior, consult @ref createProxy(IConnection&,std::string, std::string) + * For more information on its behavior, consult @ref createProxy(IConnection&,std::string,std::string) */ ProxyInterfaces(IConnection& connection, std::string destination, std::string objectPath) : ProxyObjectHolder(createProxy(connection, std::move(destination), std::move(objectPath))) , _Interfaces(getProxy())... { - // TODO: Remove - getProxy().finishRegistration(); } /*! - * @brief Creates fully working object proxy instance + * @brief Creates native-like proxy object instance * * @param[in] connection D-Bus connection to be used by the proxy object * @param[in] destination Bus name that provides a D-Bus object * @param[in] objectPath Path of the D-Bus object * * The proxy created this way becomes an owner of the connection. - * For more information on its behavior, consult @ref createProxy(std::unique_ptr&&,std::string, std::string) + * For more information on its behavior, consult @ref createProxy(std::unique_ptr&&,std::string,std::string) */ ProxyInterfaces(std::unique_ptr&& connection, std::string destination, std::string objectPath) : ProxyObjectHolder(createProxy(std::move(connection), std::move(destination), std::move(objectPath))) , _Interfaces(getProxy())... { - // TODO: Remove + } + + /*! + * @brief Finishes proxy registration and makes the proxy ready for use + * + * This function must be called in the constructor of the final proxy class that implements ProxyInterfaces. + * + * For more information, see underlying @ref IProxy::finishRegistration() + */ + void registerProxy() + { getProxy().finishRegistration(); } + + /*! + * @brief Unregisters the proxy so it no more receives signals and async call replies + * + * This function must be called in the destructor of the final proxy class that implements ProxyInterfaces. + * + * See underlying @ref IProxy::unregister() + */ + void unregisterProxy() + { + getProxy().unregister(); + } + + protected: + using base_type = ProxyInterfaces; }; } diff --git a/src/Message.cpp b/src/Message.cpp index d02a69c..2ba7bf8 100644 --- a/src/Message.cpp +++ b/src/Message.cpp @@ -658,10 +658,14 @@ AsyncMethodCall::AsyncMethodCall(MethodCall&& call) noexcept { } -void AsyncMethodCall::send(void* callback, void* userData) const +AsyncMethodCall::Slot AsyncMethodCall::send(void* callback, void* userData) const { - auto r = sdbus_->sd_bus_call_async(nullptr, nullptr, (sd_bus_message*)msg_, (sd_bus_message_handler_t)callback, userData, 0); + sd_bus_slot* slot; + + auto r = sdbus_->sd_bus_call_async(nullptr, &slot, (sd_bus_message*)msg_, (sd_bus_message_handler_t)callback, userData, 0); SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method asynchronously", -r); + + return Slot{slot, [sdbus_ = sdbus_](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; } void MethodReply::send() const diff --git a/src/Object.cpp b/src/Object.cpp index 6361601..cd57bd0 100644 --- a/src/Object.cpp +++ b/src/Object.cpp @@ -120,6 +120,11 @@ void Object::finishRegistration() } } +void Object::unregister() +{ + interfaces_.clear(); +} + sdbus::Signal Object::createSignal(const std::string& interfaceName, const std::string& signalName) { return connection_.createSignal(objectPath_, interfaceName, signalName); diff --git a/src/Object.h b/src/Object.h index 6ee54ff..efe29f6 100644 --- a/src/Object.h +++ b/src/Object.h @@ -73,6 +73,7 @@ namespace internal { void setInterfaceFlags(const std::string& interfaceName, Flags flags) override; void finishRegistration() override; + void unregister() override; sdbus::Signal createSignal(const std::string& interfaceName, const std::string& signalName) override; void emitSignal(const sdbus::Signal& message) override; diff --git a/src/Proxy.cpp b/src/Proxy.cpp index fe9c344..af5d342 100644 --- a/src/Proxy.cpp +++ b/src/Proxy.cpp @@ -28,6 +28,7 @@ #include "sdbus-c++/Message.h" #include "sdbus-c++/IConnection.h" #include "sdbus-c++/Error.h" +#include "ScopeGuard.h" #include #include #include @@ -40,8 +41,8 @@ Proxy::Proxy(sdbus::internal::IConnection& connection, std::string destination, , destination_(std::move(destination)) , objectPath_(std::move(objectPath)) { - // The connection is not ours only, it is managed by the client and we just reference it here, - // so we expect the client to manage the event loop upon this connection themselves. + // The connection is not ours only, it is owned and managed by the user and we just reference + // it here, so we expect the client to manage the event loop upon this connection themselves. } Proxy::Proxy( std::unique_ptr&& connection @@ -51,8 +52,8 @@ Proxy::Proxy( std::unique_ptr&& connection , destination_(std::move(destination)) , objectPath_(std::move(objectPath)) { - // The connection is ours only, so we have to manage event loop upon this connection, - // so we get signals, async replies, and other messages from D-Bus. + // The connection is ours only, i.e. it's us who has to manage the event loop upon this connection, + // in order that we get and process signals, async call replies, and other messages from D-Bus. connection_->enterProcessingLoopAsync(); } @@ -74,10 +75,11 @@ MethodReply Proxy::callMethod(const MethodCall& message) void Proxy::callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback) { auto callback = (void*)&Proxy::sdbus_async_reply_handler; - // Allocated userData gets deleted in the sdbus_async_reply_handler - auto userData = new AsyncReplyUserData{*this, std::move(asyncReplyCallback)}; + auto callData = std::make_unique(AsyncCalls::CallData{*this, std::move(asyncReplyCallback), {}}); - message.send(callback, userData); + callData->slot = message.send(callback, callData.get()); + + pendingAsyncCalls_.addCall(callData->slot.get(), std::move(callData)); } void Proxy::registerSignalHandler( const std::string& interfaceName @@ -122,24 +124,32 @@ void Proxy::registerSignalHandlers(sdbus::internal::IConnection& connection) } } +void Proxy::unregister() +{ + pendingAsyncCalls_.clear(); + interfaces_.clear(); +} + int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error */*retError*/) { - // We are assuming the ownership of the async reply handler pointer passed here - std::unique_ptr asyncReplyUserData{static_cast(userData)}; - assert(asyncReplyUserData != nullptr); - assert(asyncReplyUserData->callback); + auto* asyncCallData = static_cast(userData); + assert(asyncCallData != nullptr); + assert(asyncCallData->callback); + auto& proxy = asyncCallData->proxy; - MethodReply message{sdbusMessage, &asyncReplyUserData->proxy.connection_->getSdBusInterface()}; + SCOPE_EXIT{ proxy.pendingAsyncCalls_.removeCall(asyncCallData->slot.get()); }; + + MethodReply message{sdbusMessage, &proxy.connection_->getSdBusInterface()}; const auto* error = sd_bus_message_get_error(sdbusMessage); if (error == nullptr) { - asyncReplyUserData->callback(message, nullptr); + asyncCallData->callback(message, nullptr); } else { sdbus::Error exception(error->name, error->message); - asyncReplyUserData->callback(message, &exception); + asyncCallData->callback(message, &exception); } return 1; diff --git a/src/Proxy.h b/src/Proxy.h index 20cdcb1..928751e 100644 --- a/src/Proxy.h +++ b/src/Proxy.h @@ -31,6 +31,8 @@ #include #include #include +#include +#include // Forward declarations namespace sdbus { namespace internal { @@ -60,14 +62,9 @@ namespace internal { , const std::string& signalName , signal_handler signalHandler ) override; void finishRegistration() override; + void unregister() override; private: - struct AsyncReplyUserData - { - Proxy& proxy; - async_reply_handler callback; - }; - void registerSignalHandlers(sdbus::internal::IConnection& connection); static int sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError); static int sdbus_signal_callback(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError); @@ -91,6 +88,51 @@ namespace internal { std::map signals_; }; std::map interfaces_; + + // We need to keep track of pending async calls. When the proxy is being destructed, we must + // remove all slots of these pending calls, otherwise in case when the connection outlives + // the proxy, we might get async reply handlers invoked for pending async calls after the proxy + // has been destroyed, which is a free ticket into the realm of undefined behavior. + class AsyncCalls + { + public: + struct CallData + { + Proxy& proxy; + async_reply_handler callback; + AsyncMethodCall::Slot slot; + }; + + ~AsyncCalls() + { + clear(); + } + + bool addCall(void* slot, std::unique_ptr&& asyncCallData) + { + std::lock_guard lock(mutex_); + return calls_.emplace(slot, std::move(asyncCallData)).second; + } + + bool removeCall(void* slot) + { + std::lock_guard lock(mutex_); + return calls_.erase(slot) > 0; + } + + void clear() + { + 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(); + } + + private: + std::unordered_map> calls_; + std::mutex mutex_; + } pendingAsyncCalls_; }; }} diff --git a/test/integrationtests/TestingAdaptor.h b/test/integrationtests/TestingAdaptor.h index 7dd0e59..c4668a2 100644 --- a/test/integrationtests/TestingAdaptor.h +++ b/test/integrationtests/TestingAdaptor.h @@ -35,9 +35,15 @@ class TestingAdaptor : public sdbus::AdaptorInterfaces { public: TestingAdaptor(sdbus::IConnection& connection) : - sdbus::AdaptorInterfaces<::testing_adaptor>(connection, OBJECT_PATH) { } + AdaptorInterfaces(connection, OBJECT_PATH) + { + registerAdaptor(); + } - virtual ~TestingAdaptor() { } + ~TestingAdaptor() + { + unregisterAdaptor(); + } bool wasMultiplyCalled() const { return m_multiplyCalled; } double getMultiplyResult() const { return m_multiplyResult; } diff --git a/test/integrationtests/TestingProxy.h b/test/integrationtests/TestingProxy.h index 03b5210..38458d4 100644 --- a/test/integrationtests/TestingProxy.h +++ b/test/integrationtests/TestingProxy.h @@ -31,7 +31,16 @@ class TestingProxy : public sdbus::ProxyInterfaces<::testing_proxy, sdbus::introspectable_proxy> { public: - using sdbus::ProxyInterfaces<::testing_proxy, sdbus::introspectable_proxy>::ProxyInterfaces; + TestingProxy(std::string destination, std::string objectPath) + : ProxyInterfaces(std::move(destination), std::move(objectPath)) + { + registerProxy(); + } + + ~TestingProxy() + { + unregisterProxy(); + } int getSimpleCallCount() const { return m_simpleCallCounter; } std::map getMap() const { return m_map; } diff --git a/test/perftests/client.cpp b/test/perftests/client.cpp index 4116007..dacb216 100644 --- a/test/perftests/client.cpp +++ b/test/perftests/client.cpp @@ -33,15 +33,22 @@ #include #include #include +#include using namespace std::chrono_literals; -class PerftestClient : public sdbus::ProxyInterfaces +class PerftestProxy : public sdbus::ProxyInterfaces { public: - PerftestClient(std::string destination, std::string objectPath) - : sdbus::ProxyInterfaces(std::move(destination), std::move(objectPath)) + PerftestProxy(std::string destination, std::string objectPath) + : ProxyInterfaces(std::move(destination), std::move(objectPath)) { + registerProxy(); + } + + ~PerftestProxy() + { + unregisterProxy(); } protected: @@ -91,7 +98,7 @@ int main(int /*argc*/, char */*argv*/[]) { const char* destinationName = "org.sdbuscpp.perftests"; const char* objectPath = "/org/sdbuscpp/perftests"; - PerftestClient client(destinationName, objectPath); + PerftestProxy client(destinationName, objectPath); const unsigned int repetitions{20}; unsigned int msgCount = 1000; diff --git a/test/perftests/server.cpp b/test/perftests/server.cpp index 551cfe0..8846da8 100644 --- a/test/perftests/server.cpp +++ b/test/perftests/server.cpp @@ -30,17 +30,24 @@ #include #include #include +#include using namespace std::chrono_literals; std::string createRandomString(size_t length); -class PerftestServer : public sdbus::AdaptorInterfaces +class PerftestAdaptor : public sdbus::AdaptorInterfaces { public: - PerftestServer(sdbus::IConnection& connection, std::string objectPath) - : sdbus::AdaptorInterfaces(connection, std::move(objectPath)) + PerftestAdaptor(sdbus::IConnection& connection, std::string objectPath) + : AdaptorInterfaces(connection, std::move(objectPath)) { + registerAdaptor(); + } + + ~PerftestAdaptor() + { + unregisterAdaptor(); } protected: @@ -89,7 +96,7 @@ int main(int /*argc*/, char */*argv*/[]) auto connection = sdbus::createSystemBusConnection(serviceName); const char* objectPath = "/org/sdbuscpp/perftests"; - PerftestServer server(*connection, objectPath); + PerftestAdaptor server(*connection, objectPath); connection->enterProcessingLoop(); } diff --git a/test/stresstests/sdbus-c++-stress-tests.cpp b/test/stresstests/sdbus-c++-stress-tests.cpp index d606514..7385dd8 100644 --- a/test/stresstests/sdbus-c++-stress-tests.cpp +++ b/test/stresstests/sdbus-c++-stress-tests.cpp @@ -55,7 +55,16 @@ using namespace std::string_literals; class CelsiusThermometerAdaptor : public sdbus::AdaptorInterfaces { public: - using sdbus::AdaptorInterfaces::AdaptorInterfaces; + CelsiusThermometerAdaptor(sdbus::IConnection& connection, std::string objectPath) + : AdaptorInterfaces(connection, std::move(objectPath)) + { + registerAdaptor(); + } + + ~CelsiusThermometerAdaptor() + { + unregisterAdaptor(); + } protected: virtual uint32_t getCurrentTemperature() override @@ -70,67 +79,82 @@ private: class CelsiusThermometerProxy : public sdbus::ProxyInterfaces { public: - using sdbus::ProxyInterfaces::ProxyInterfaces; + CelsiusThermometerProxy(sdbus::IConnection& connection, std::string destination, std::string objectPath) + : ProxyInterfaces(connection, std::move(destination), std::move(objectPath)) + { + registerProxy(); + } + + ~CelsiusThermometerProxy() + { + unregisterProxy(); + } }; class FahrenheitThermometerAdaptor : public sdbus::AdaptorInterfaces< org::sdbuscpp::stresstests::fahrenheit::thermometer_adaptor , org::sdbuscpp::stresstests::fahrenheit::thermometer::factory_adaptor > { public: - FahrenheitThermometerAdaptor(sdbus::IConnection& connection, std::string objectPath) - : sdbus::AdaptorInterfaces< org::sdbuscpp::stresstests::fahrenheit::thermometer_adaptor - , org::sdbuscpp::stresstests::fahrenheit::thermometer::factory_adaptor >(connection, std::move(objectPath)) + FahrenheitThermometerAdaptor(sdbus::IConnection& connection, std::string objectPath, bool isDelegate) + : AdaptorInterfaces(connection, std::move(objectPath)) , celsiusProxy_(connection, SERVICE_2_BUS_NAME, CELSIUS_THERMOMETER_OBJECT_PATH) { - unsigned int workers = std::thread::hardware_concurrency(); - if (workers < 4) - workers = 4; + if (!isDelegate) + { + unsigned int workers = std::thread::hardware_concurrency(); + if (workers < 4) + workers = 4; - for (unsigned int i = 0; i < workers; ++i) - workers_.emplace_back([this]() - { - //std::cout << "Created FTA worker thread 0x" << std::hex << std::this_thread::get_id() << std::dec << std::endl; - - while(!exit_) + for (unsigned int i = 0; i < workers; ++i) + workers_.emplace_back([this]() { - // Pop a work item from the queue - std::unique_lock lock(mutex_); - cond_.wait(lock, [this]{return !requests_.empty() || exit_;}); - if (exit_) - break; - auto request = std::move(requests_.front()); - requests_.pop(); - lock.unlock(); + //std::cout << "Created FTA worker thread 0x" << std::hex << std::this_thread::get_id() << std::dec << std::endl; - // Either create or destroy a delegate object - if (request.delegateObjectPath.empty()) + while(!exit_) { - // Create new delegate object - auto& connection = getObject().getConnection(); - sdbus::ObjectPath newObjectPath = FAHRENHEIT_THERMOMETER_OBJECT_PATH + "/" + std::to_string(request.objectNr); - - // Here we are testing dynamic creation of a D-Bus object in an async way - auto adaptor = std::make_unique(connection, newObjectPath); - - std::unique_lock lock{childrenMutex_}; - children_.emplace(newObjectPath, std::move(adaptor)); + // Pop a work item from the queue + std::unique_lock lock(mutex_); + cond_.wait(lock, [this]{return !requests_.empty() || exit_;}); + if (exit_) + break; + auto request = std::move(requests_.front()); + requests_.pop(); lock.unlock(); - request.result.returnResults(newObjectPath); + // Either create or destroy a delegate object + if (request.delegateObjectPath.empty()) + { + // Create new delegate object + auto& connection = getObject().getConnection(); + sdbus::ObjectPath newObjectPath = FAHRENHEIT_THERMOMETER_OBJECT_PATH + "/" + std::to_string(request.objectNr); + + // Here we are testing dynamic creation of a D-Bus object in an async way + auto adaptor = std::make_unique(connection, newObjectPath, true); + + std::unique_lock lock{childrenMutex_}; + children_.emplace(newObjectPath, std::move(adaptor)); + lock.unlock(); + + request.result.returnResults(newObjectPath); + } + else + { + // Destroy existing delegate object + // Here we are testing dynamic removal of a D-Bus object in an async way + std::lock_guard lock{childrenMutex_}; + children_.erase(request.delegateObjectPath); + } } - else - { - // Destroy existing delegate object - // Here we are testing dynamic removal of a D-Bus object in an async way - std::lock_guard lock{childrenMutex_}; - children_.erase(request.delegateObjectPath); - } - } - }); + }); + } + + registerAdaptor(); } ~FahrenheitThermometerAdaptor() { + unregisterAdaptor(); + exit_ = true; cond_.notify_all(); for (auto& worker : workers_) @@ -185,15 +209,23 @@ class FahrenheitThermometerProxy : public sdbus::ProxyInterfaces< org::sdbuscpp: , org::sdbuscpp::stresstests::fahrenheit::thermometer::factory_proxy > { public: - using sdbus::ProxyInterfaces< org::sdbuscpp::stresstests::fahrenheit::thermometer_proxy - , org::sdbuscpp::stresstests::fahrenheit::thermometer::factory_proxy >::ProxyInterfaces; + FahrenheitThermometerProxy(sdbus::IConnection& connection, std::string destination, std::string objectPath) + : ProxyInterfaces(connection, std::move(destination), std::move(objectPath)) + { + registerProxy(); + } + + ~FahrenheitThermometerProxy() + { + unregisterProxy(); + } }; class ConcatenatorAdaptor : public sdbus::AdaptorInterfaces { public: ConcatenatorAdaptor(sdbus::IConnection& connection, std::string objectPath) - : sdbus::AdaptorInterfaces(connection, std::move(objectPath)) + : AdaptorInterfaces(connection, std::move(objectPath)) { unsigned int workers = std::thread::hardware_concurrency(); if (workers < 4) @@ -225,10 +257,14 @@ public: concatenatedSignal(resultString); } }); + + registerAdaptor(); } ~ConcatenatorAdaptor() { + unregisterAdaptor(); + exit_ = true; cond_.notify_all(); for (auto& worker : workers_) @@ -260,7 +296,16 @@ private: class ConcatenatorProxy : public sdbus::ProxyInterfaces { public: - using sdbus::ProxyInterfaces::ProxyInterfaces; + ConcatenatorProxy(sdbus::IConnection& connection, std::string destination, std::string objectPath) + : ProxyInterfaces(connection, std::move(destination), std::move(objectPath)) + { + registerProxy(); + } + + ~ConcatenatorProxy() + { + unregisterProxy(); + } private: virtual void onConcatenateReply(const std::string& result, const sdbus::Error* error) override @@ -302,21 +347,27 @@ public: int main(int /*argc*/, char */*argv*/[]) { auto service2Connection = sdbus::createSystemBusConnection(SERVICE_2_BUS_NAME); - std::thread service2Thread([&con = *service2Connection]() + std::atomic service2ThreadReady{}; + std::thread service2Thread([&con = *service2Connection, &service2ThreadReady]() { CelsiusThermometerAdaptor thermometer(con, CELSIUS_THERMOMETER_OBJECT_PATH); + service2ThreadReady = true; con.enterProcessingLoop(); }); auto service1Connection = sdbus::createSystemBusConnection(SERVICE_1_BUS_NAME); - std::thread service1Thread([&con = *service1Connection]() + std::atomic service1ThreadReady{}; + std::thread service1Thread([&con = *service1Connection, &service1ThreadReady]() { ConcatenatorAdaptor concatenator(con, CONCATENATOR_OBJECT_PATH); - FahrenheitThermometerAdaptor thermometer(con, FAHRENHEIT_THERMOMETER_OBJECT_PATH); + FahrenheitThermometerAdaptor thermometer(con, FAHRENHEIT_THERMOMETER_OBJECT_PATH, false); + service1ThreadReady = true; con.enterProcessingLoop(); }); - std::this_thread::sleep_for(100ms); + // Wait for both services to export their D-Bus objects + while (!service2ThreadReady || !service1ThreadReady) + std::this_thread::sleep_for(1ms); std::atomic concatenationCallsMade{0}; std::atomic concatenationRepliesReceived{0}; @@ -385,7 +436,6 @@ int main(int /*argc*/, char */*argv*/[]) if ((localCounter % 10) == 0) thermometerCallsMade = localCounter; - proxy.~FahrenheitThermometerProxy(); thermometer.destroyDelegateObject(newObjectPath); } });