From c139110112e602b3e8953e369ac938ca2a558619 Mon Sep 17 00:00:00 2001 From: lubo-svk <49922839+lubo-svk@users.noreply.github.com> Date: Sun, 3 Nov 2019 13:54:13 +0100 Subject: [PATCH] Add support for custom timeout value for D-Bus method calls (#72) --- CMakeLists.txt | 2 +- cmake/LibsystemdExternalProject.cmake | 2 + include/sdbus-c++/ConvenienceApiClasses.h | 10 +++ include/sdbus-c++/ConvenienceApiClasses.inl | 67 ++++++++++++++++++- include/sdbus-c++/IConnection.h | 41 +++++++++++- include/sdbus-c++/IObject.h | 2 + include/sdbus-c++/IProxy.h | 35 +++++++++- include/sdbus-c++/Message.h | 6 +- src/Connection.cpp | 18 +++++ src/Connection.h | 3 + src/ConvenienceApiClasses.cpp | 2 +- src/ISdBus.h | 7 +- src/Message.cpp | 12 ++-- src/Proxy.cpp | 8 +-- src/Proxy.h | 4 +- src/SdBus.cpp | 27 ++++++++ src/SdBus.h | 3 + tests/CMakeLists.txt | 2 + .../integrationtests/AdaptorAndProxy_test.cpp | 38 +++++++++++ tests/integrationtests/TestingProxy.h | 2 - tests/integrationtests/proxy-glue.h | 8 +++ tests/unittests/mocks/SdBusMock.h | 3 + tools/xml2cpp-codegen/AdaptorGenerator.cpp | 2 +- tools/xml2cpp-codegen/ProxyGenerator.cpp | 21 +++++- 24 files changed, 296 insertions(+), 29 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1a7e95b..2e46259 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -98,7 +98,7 @@ option(BUILD_SHARED_LIBS "Build shared libraries (.so) instead of static ones (. # Having an object target allows unit tests to reuse already built sources without re-building add_library(sdbus-c++-objlib OBJECT ${SDBUSCPP_SRCS}) -target_compile_definitions(sdbus-c++-objlib PRIVATE BUILDLIB=1) +target_compile_definitions(sdbus-c++-objlib PRIVATE BUILD_LIB=1 LIBSYSTEMD_VERSION=${SYSTEMD_VERSION}) target_include_directories(sdbus-c++-objlib PUBLIC $ $ $) diff --git a/cmake/LibsystemdExternalProject.cmake b/cmake/LibsystemdExternalProject.cmake index d93ec06..d30d1ad 100644 --- a/cmake/LibsystemdExternalProject.cmake +++ b/cmake/LibsystemdExternalProject.cmake @@ -50,6 +50,8 @@ set(SYSTEMD_INCLUDE_DIRS ${SOURCE_DIR}/src) ExternalProject_Get_property(LibsystemdBuildProject BINARY_DIR) set(SYSTEMD_LIBRARY_DIRS ${BINARY_DIR}) +set(SYSTEMD_VERSION ${LIBSYSTEMD_VERSION}) + add_library(Systemd::Libsystemd STATIC IMPORTED) set_target_properties(Systemd::Libsystemd PROPERTIES IMPORTED_LOCATION ${SYSTEMD_LIBRARY_DIRS}/libsystemd.a) set(SYSTEMD_LIBRARIES Systemd::Libsystemd ${CAP_LIBRARIES} ${GLIBC_RT_LIBRARY} ${MOUNT_LIBRARIES}) diff --git a/include/sdbus-c++/ConvenienceApiClasses.h b/include/sdbus-c++/ConvenienceApiClasses.h index 3faf6a9..1904937 100644 --- a/include/sdbus-c++/ConvenienceApiClasses.h +++ b/include/sdbus-c++/ConvenienceApiClasses.h @@ -32,6 +32,8 @@ #include #include #include +#include +#include // Forward declarations namespace sdbus { @@ -164,6 +166,9 @@ namespace sdbus { ~MethodInvoker() noexcept(false); MethodInvoker& onInterface(const std::string& interfaceName); + MethodInvoker& withTimeout(uint64_t usec); + template + MethodInvoker& withTimeout(const std::chrono::duration<_Rep, _Period>& timeout); template MethodInvoker& withArguments(_Args&&... args); template void storeResultsTo(_Args&... args); @@ -172,6 +177,7 @@ namespace sdbus { private: IProxy& proxy_; const std::string& methodName_; + uint64_t timeout_{}; MethodCall method_; int exceptions_{}; // Number of active exceptions when MethodInvoker is constructed bool methodCalled_{}; @@ -182,12 +188,16 @@ namespace sdbus { public: AsyncMethodInvoker(IProxy& proxy, const std::string& methodName); AsyncMethodInvoker& onInterface(const std::string& interfaceName); + AsyncMethodInvoker& withTimeout(uint64_t usec); + template + AsyncMethodInvoker& withTimeout(const std::chrono::duration<_Rep, _Period>& timeout); template AsyncMethodInvoker& withArguments(_Args&&... args); template void uponReplyInvoke(_Function&& callback); private: IProxy& proxy_; const std::string& methodName_; + uint64_t timeout_{}; AsyncMethodCall method_; }; diff --git a/include/sdbus-c++/ConvenienceApiClasses.inl b/include/sdbus-c++/ConvenienceApiClasses.inl index 538bbf6..009edab 100644 --- a/include/sdbus-c++/ConvenienceApiClasses.inl +++ b/include/sdbus-c++/ConvenienceApiClasses.inl @@ -40,6 +40,10 @@ namespace sdbus { + /*** ----------------- ***/ + /*** MethodRegistrator ***/ + /*** ----------------- ***/ + // Moved into the library to isolate from C++17 dependency /* inline MethodRegistrator::MethodRegistrator(IObject& object, const std::string& methodName) @@ -149,6 +153,9 @@ namespace sdbus { return *this; } + /*** ----------------- ***/ + /*** SignalRegistrator ***/ + /*** ----------------- ***/ // Moved into the library to isolate from C++17 dependency /* @@ -203,6 +210,9 @@ namespace sdbus { return *this; } + /*** ------------------- ***/ + /*** PropertyRegistrator ***/ + /*** ------------------- ***/ // Moved into the library to isolate from C++17 dependency /* @@ -309,6 +319,9 @@ namespace sdbus { return *this; } + /*** -------------------- ***/ + /*** InterfaceFlagsSetter ***/ + /*** -------------------- ***/ // Moved into the library to isolate from C++17 dependency /* @@ -369,6 +382,9 @@ namespace sdbus { return *this; } + /*** ------------- ***/ + /*** SignalEmitter ***/ + /*** ------------- ***/ // Moved into the library to isolate from C++17 dependency /* @@ -416,6 +432,9 @@ namespace sdbus { detail::serialize_pack(signal_, std::forward<_Args>(args)...); } + /*** ------------- ***/ + /*** MethodInvoker ***/ + /*** ------------- ***/ // Moved into the library to isolate from C++17 dependency /* @@ -456,6 +475,20 @@ namespace sdbus { return *this; } + inline MethodInvoker& MethodInvoker::withTimeout(uint64_t usec) + { + timeout_ = usec; + + return *this; + } + + template + inline MethodInvoker& MethodInvoker::withTimeout(const std::chrono::duration<_Rep, _Period>& timeout) + { + auto microsecs = std::chrono::duration_cast(timeout); + return withTimeout(microsecs.count()); + } + template inline MethodInvoker& MethodInvoker::withArguments(_Args&&... args) { @@ -471,7 +504,7 @@ namespace sdbus { { SDBUS_THROW_ERROR_IF(!method_.isValid(), "DBus interface not specified when calling a DBus method", EINVAL); - auto reply = proxy_.callMethod(method_); + auto reply = proxy_.callMethod(method_, timeout_); methodCalled_ = true; detail::deserialize_pack(reply, args...); @@ -484,6 +517,9 @@ namespace sdbus { method_.dontExpectReply(); } + /*** ------------------ ***/ + /*** AsyncMethodInvoker ***/ + /*** ------------------ ***/ inline AsyncMethodInvoker::AsyncMethodInvoker(IProxy& proxy, const std::string& methodName) : proxy_(proxy) @@ -498,6 +534,20 @@ namespace sdbus { return *this; } + inline AsyncMethodInvoker& AsyncMethodInvoker::withTimeout(uint64_t usec) + { + timeout_ = usec; + + return *this; + } + + template + inline AsyncMethodInvoker& AsyncMethodInvoker::withTimeout(const std::chrono::duration<_Rep, _Period>& timeout) + { + auto microsecs = std::chrono::duration_cast(timeout); + return withTimeout(microsecs.count()); + } + template inline AsyncMethodInvoker& AsyncMethodInvoker::withArguments(_Args&&... args) { @@ -513,7 +563,7 @@ namespace sdbus { { SDBUS_THROW_ERROR_IF(!method_.isValid(), "DBus interface not specified when calling a DBus method", EINVAL); - proxy_.callMethod(method_, [callback = std::forward<_Function>(callback)](MethodReply& reply, const Error* error) + auto asyncReplyHandler = [callback = std::forward<_Function>(callback)](MethodReply& reply, const Error* error) { // Create a tuple of callback input arguments' types, which will be used // as a storage for the argument values deserialized from the message. @@ -525,9 +575,14 @@ namespace sdbus { // Invoke callback with input arguments from the tuple. sdbus::apply(callback, error, args); // TODO: Use std::apply when switching to full C++17 support - }); + }; + + proxy_.callMethod(method_, std::move(asyncReplyHandler), timeout_); } + /*** ---------------- ***/ + /*** SignalSubscriber ***/ + /*** ---------------- ***/ inline SignalSubscriber::SignalSubscriber(IProxy& proxy, const std::string& signalName) : proxy_(proxy) @@ -563,6 +618,9 @@ namespace sdbus { }); } + /*** -------------- ***/ + /*** PropertyGetter ***/ + /*** -------------- ***/ inline PropertyGetter::PropertyGetter(IProxy& proxy, const std::string& propertyName) : proxy_(proxy) @@ -581,6 +639,9 @@ namespace sdbus { return var; } + /*** -------------- ***/ + /*** PropertySetter ***/ + /*** -------------- ***/ inline PropertySetter::PropertySetter(IProxy& proxy, const std::string& propertyName) : proxy_(proxy) diff --git a/include/sdbus-c++/IConnection.h b/include/sdbus-c++/IConnection.h index 7f9fbc4..27701db 100644 --- a/include/sdbus-c++/IConnection.h +++ b/include/sdbus-c++/IConnection.h @@ -27,9 +27,10 @@ #ifndef SDBUS_CXX_ICONNECTION_H_ #define SDBUS_CXX_ICONNECTION_H_ -//#include #include #include +#include +#include namespace sdbus { @@ -145,8 +146,46 @@ namespace sdbus { * @throws sdbus::Error in case of failure */ virtual bool processPendingRequest() = 0; + + /*! + * @brief Sets general method call timeout + * + * @param[in] timeout Timeout value in microseconds + * + * General method call timeout is used for all method calls upon this connection. + * Method call-specific timeout overrides this general setting. + * + * Supported by libsystemd>=v240. + * + * @throws sdbus::Error in case of failure + */ + virtual void setMethodCallTimeout(uint64_t timeout) = 0; + + /*! + * @copydoc IConnection::setMethodCallTimeout(uint64_t) + */ + template + void setMethodCallTimeout(const std::chrono::duration<_Rep, _Period>& timeout); + + /*! + * @brief Gets general method call timeout + * + * @return Timeout value in microseconds + * + * Supported by libsystemd>=v240. + * + * @throws sdbus::Error in case of failure + */ + virtual uint64_t getMethodCallTimeout() const = 0; }; + template + inline void IConnection::setMethodCallTimeout(const std::chrono::duration<_Rep, _Period>& timeout) + { + auto microsecs = std::chrono::duration_cast(timeout); + return setMethodCallTimeout(microsecs.count()); + } + /*! * @brief Creates/opens D-Bus system connection * diff --git a/include/sdbus-c++/IObject.h b/include/sdbus-c++/IObject.h index 0b24138..ab31997 100644 --- a/include/sdbus-c++/IObject.h +++ b/include/sdbus-c++/IObject.h @@ -387,6 +387,8 @@ namespace sdbus { SignalEmitter emitSignal(const std::string& signalName); }; + // Out-of-line member definitions + inline MethodRegistrator IObject::registerMethod(const std::string& methodName) { return MethodRegistrator(*this, methodName); diff --git a/include/sdbus-c++/IProxy.h b/include/sdbus-c++/IProxy.h index ea3c1d0..0f3715c 100644 --- a/include/sdbus-c++/IProxy.h +++ b/include/sdbus-c++/IProxy.h @@ -31,6 +31,7 @@ #include #include #include +#include // Forward declarations namespace sdbus { @@ -95,6 +96,7 @@ namespace sdbus { * @brief Calls method on the proxied D-Bus object * * @param[in] message Message representing a method call + * @param[in] timeout Timeout for dbus call in microseconds * @return A method reply message * * Normally, the call is blocking, i.e. it waits for the remote method to finish with either @@ -108,13 +110,20 @@ namespace sdbus { * * @throws sdbus::Error in case of failure */ - virtual MethodReply callMethod(const MethodCall& message) = 0; + virtual MethodReply callMethod(const MethodCall& message, uint64_t timeout = 0) = 0; + + /*! + * @copydoc IProxy::callMethod(const MethodCall&,uint64_t) + */ + template + MethodReply callMethod(const MethodCall& message, const std::chrono::duration<_Rep, _Period>& timeout); /*! * @brief Calls method on the proxied D-Bus object asynchronously * * @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 * * 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 @@ -124,7 +133,13 @@ namespace sdbus { * * @throws sdbus::Error in case of failure */ - virtual void callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback) = 0; + virtual void callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout = 0) = 0; + + /*! + * @copydoc IProxy::callMethod(const AsyncMethodCall&,async_reply_handler,uint64_t) + */ + template + void callMethod(const AsyncMethodCall& 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 @@ -264,6 +279,22 @@ namespace sdbus { PropertySetter setProperty(const std::string& propertyName); }; + // Out-of-line member definitions + + template + inline MethodReply IProxy::callMethod(const MethodCall& message, const std::chrono::duration<_Rep, _Period>& timeout) + { + auto microsecs = std::chrono::duration_cast(timeout); + return callMethod(message, microsecs.count()); + } + + template + inline void IProxy::callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback, const std::chrono::duration<_Rep, _Period>& timeout) + { + auto microsecs = std::chrono::duration_cast(timeout); + callMethod(message, std::move(asyncReplyCallback), microsecs.count()); + } + inline MethodInvoker IProxy::callMethod(const std::string& methodName) { return MethodInvoker(*this, methodName); diff --git a/include/sdbus-c++/Message.h b/include/sdbus-c++/Message.h index 08fceb2..9889425 100644 --- a/include/sdbus-c++/Message.h +++ b/include/sdbus-c++/Message.h @@ -172,14 +172,14 @@ namespace sdbus { public: MethodCall() = default; - MethodReply send() const; + MethodReply send(uint64_t timeout = 0) const; MethodReply createReply() const; MethodReply createErrorReply(const sdbus::Error& error) const; void dontExpectReply(); bool doesntExpectReply() const; private: - MethodReply sendWithReply() const; + MethodReply sendWithReply(uint64_t timeout) const; MethodReply sendWithNoReply() const; }; @@ -193,7 +193,7 @@ namespace sdbus { AsyncMethodCall() = default; explicit AsyncMethodCall(MethodCall&& call) noexcept; - Slot send(void* callback, void* userData) const; + Slot send(void* callback, void* userData, uint64_t timeout = 0) const; }; class MethodReply : public Message diff --git a/src/Connection.cpp b/src/Connection.cpp index e1a25f2..91ca4f9 100644 --- a/src/Connection.cpp +++ b/src/Connection.cpp @@ -145,6 +145,24 @@ SlotPtr Connection::addObjectManager(const std::string& objectPath, void* /*dumm return {slot, [this](void *slot){ iface_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; } +void Connection::setMethodCallTimeout(uint64_t timeout) +{ + auto r = iface_->sd_bus_set_method_call_timeout(bus_.get(), timeout); + + SDBUS_THROW_ERROR_IF(r < 0, "Failed to set method call timeout", -r); +} + +uint64_t Connection::getMethodCallTimeout() const +{ + uint64_t timeout; + + auto r = iface_->sd_bus_get_method_call_timeout(bus_.get(), &timeout); + + SDBUS_THROW_ERROR_IF(r < 0, "Failed to get method call timeout", -r); + + return timeout; +} + SlotPtr Connection::addObjectVTable( const std::string& objectPath , const std::string& interfaceName , const sd_bus_vtable* vtable diff --git a/src/Connection.h b/src/Connection.h index b8f8da4..ff50e98 100644 --- a/src/Connection.h +++ b/src/Connection.h @@ -63,6 +63,9 @@ namespace sdbus { namespace internal { void addObjectManager(const std::string& objectPath) override; SlotPtr addObjectManager(const std::string& objectPath, void* /*dummy*/) override; + void setMethodCallTimeout(uint64_t timeout) override; + uint64_t getMethodCallTimeout() const override; + const ISdBus& getSdBusInterface() const override; ISdBus& getSdBusInterface() override; diff --git a/src/ConvenienceApiClasses.cpp b/src/ConvenienceApiClasses.cpp index ed16073..b900003 100644 --- a/src/ConvenienceApiClasses.cpp +++ b/src/ConvenienceApiClasses.cpp @@ -203,7 +203,7 @@ MethodInvoker::~MethodInvoker() noexcept(false) // since C++11, destructors must // Therefore, we can allow callMethod() to throw even if we are in the destructor. // Bottomline is, to be on the safe side, the caller must take care of catching and reacting // to the exception thrown from here if the caller is a destructor itself. - proxy_.callMethod(method_); + proxy_.callMethod(method_, timeout_); } } diff --git a/src/ISdBus.h b/src/ISdBus.h index 44b8ac0..d2f5a9b 100644 --- a/src/ISdBus.h +++ b/src/ISdBus.h @@ -42,6 +42,8 @@ namespace sdbus { namespace internal { uint64_t timeout_usec; }; + virtual ~ISdBus() = default; + virtual sd_bus_message* sd_bus_message_ref(sd_bus_message *m) = 0; virtual sd_bus_message* sd_bus_message_unref(sd_bus_message *m) = 0; @@ -54,6 +56,9 @@ namespace sdbus { namespace internal { virtual int sd_bus_message_new_method_return(sd_bus_message *call, sd_bus_message **m) = 0; virtual int sd_bus_message_new_method_error(sd_bus_message *call, sd_bus_message **m, const sd_bus_error *e) = 0; + virtual int sd_bus_set_method_call_timeout(sd_bus *bus, uint64_t usec) = 0; + virtual int sd_bus_get_method_call_timeout(sd_bus *bus, uint64_t *ret) = 0; + virtual int sd_bus_emit_properties_changed_strv(sd_bus *bus, const char *path, const char *interface, char **names) = 0; virtual int sd_bus_emit_object_added(sd_bus *bus, const char *path) = 0; virtual int sd_bus_emit_object_removed(sd_bus *bus, const char *path) = 0; @@ -74,8 +79,6 @@ namespace sdbus { namespace internal { virtual int sd_bus_flush(sd_bus *bus) = 0; virtual sd_bus *sd_bus_flush_close_unref(sd_bus *bus) = 0; - - virtual ~ISdBus() = default; }; }} diff --git a/src/Message.cpp b/src/Message.cpp index fff3462..8ebefa2 100644 --- a/src/Message.cpp +++ b/src/Message.cpp @@ -628,21 +628,21 @@ bool MethodCall::doesntExpectReply() const return r == 0; } -MethodReply MethodCall::send() const +MethodReply MethodCall::send(uint64_t timeout) const { if (!doesntExpectReply()) - return sendWithReply(); + return sendWithReply(timeout); else return sendWithNoReply(); } -MethodReply MethodCall::sendWithReply() const +MethodReply MethodCall::sendWithReply(uint64_t timeout) const { sd_bus_error sdbusError = SD_BUS_ERROR_NULL; SCOPE_EXIT{ sd_bus_error_free(&sdbusError); }; sd_bus_message* sdbusReply{}; - auto r = sdbus_->sd_bus_call(nullptr, (sd_bus_message*)msg_, 0, &sdbusError, &sdbusReply); + auto r = sdbus_->sd_bus_call(nullptr, (sd_bus_message*)msg_, timeout, &sdbusError, &sdbusReply); if (sd_bus_error_is_set(&sdbusError)) throw sdbus::Error(sdbusError.name, sdbusError.message); @@ -687,11 +687,11 @@ AsyncMethodCall::AsyncMethodCall(MethodCall&& call) noexcept { } -AsyncMethodCall::Slot AsyncMethodCall::send(void* callback, void* userData) const +AsyncMethodCall::Slot AsyncMethodCall::send(void* callback, void* userData, uint64_t timeout) const { 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); + auto r = sdbus_->sd_bus_call_async(nullptr, &slot, (sd_bus_message*)msg_, (sd_bus_message_handler_t)callback, userData, timeout); 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); }}; diff --git a/src/Proxy.cpp b/src/Proxy.cpp index e3eed14..1f4fe1f 100644 --- a/src/Proxy.cpp +++ b/src/Proxy.cpp @@ -69,17 +69,17 @@ AsyncMethodCall Proxy::createAsyncMethodCall(const std::string& interfaceName, c return AsyncMethodCall{Proxy::createMethodCall(interfaceName, methodName)}; } -MethodReply Proxy::callMethod(const MethodCall& message) +MethodReply Proxy::callMethod(const MethodCall& message, uint64_t timeout) { - return message.send(); + return message.send(timeout); } -void Proxy::callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback) +void Proxy::callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout) { auto callback = (void*)&Proxy::sdbus_async_reply_handler; auto callData = std::make_unique(AsyncCalls::CallData{*this, std::move(asyncReplyCallback), {}}); - callData->slot = message.send(callback, callData.get()); + callData->slot = message.send(callback, callData.get(), timeout); pendingAsyncCalls_.addCall(callData->slot.get(), std::move(callData)); } diff --git a/src/Proxy.h b/src/Proxy.h index 39ba423..d211db6 100644 --- a/src/Proxy.h +++ b/src/Proxy.h @@ -52,8 +52,8 @@ namespace internal { MethodCall createMethodCall(const std::string& interfaceName, const std::string& methodName) override; AsyncMethodCall createAsyncMethodCall(const std::string& interfaceName, const std::string& methodName) override; - MethodReply callMethod(const MethodCall& message) override; - void callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback) override; + MethodReply callMethod(const MethodCall& message, uint64_t timeout) override; + void callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback, uint64_t timeout) override; void registerSignalHandler( const std::string& interfaceName , const std::string& signalName diff --git a/src/SdBus.cpp b/src/SdBus.cpp index d5ca5be..7944fcb 100644 --- a/src/SdBus.cpp +++ b/src/SdBus.cpp @@ -26,6 +26,7 @@ */ #include "SdBus.h" +#include namespace sdbus { namespace internal { @@ -92,6 +93,32 @@ int SdBus::sd_bus_message_new_method_error(sd_bus_message *call, sd_bus_message return ::sd_bus_message_new_method_error(call, m, e); } +int SdBus::sd_bus_set_method_call_timeout(sd_bus *bus, uint64_t usec) +{ +#if LIBSYSTEMD_VERSION>=240 + std::unique_lock lock(sdbusMutex_); + + return ::sd_bus_set_method_call_timeout(bus, usec); +#else + (void)bus; + (void)usec; + throw sdbus::Error(SD_BUS_ERROR_NOT_SUPPORTED, "Setting general method call timeout not supported by underlying version of libsystemd"); +#endif +} + +int SdBus::sd_bus_get_method_call_timeout(sd_bus *bus, uint64_t *ret) +{ +#if LIBSYSTEMD_VERSION>=240 + std::unique_lock lock(sdbusMutex_); + + return ::sd_bus_get_method_call_timeout(bus, ret); +#else + (void)bus; + (void)ret; + throw sdbus::Error(SD_BUS_ERROR_NOT_SUPPORTED, "Getting general method call timeout not supported by underlying version of libsystemd"); +#endif +} + int SdBus::sd_bus_emit_properties_changed_strv(sd_bus *bus, const char *path, const char *interface, char **names) { std::unique_lock lock(sdbusMutex_); diff --git a/src/SdBus.h b/src/SdBus.h index 05e8581..38adf15 100644 --- a/src/SdBus.h +++ b/src/SdBus.h @@ -48,6 +48,9 @@ public: virtual int sd_bus_message_new_method_return(sd_bus_message *call, sd_bus_message **m) override; virtual int sd_bus_message_new_method_error(sd_bus_message *call, sd_bus_message **m, const sd_bus_error *e) override; + virtual int sd_bus_set_method_call_timeout(sd_bus *bus, uint64_t usec) override; + virtual int sd_bus_get_method_call_timeout(sd_bus *bus, uint64_t *ret) override; + virtual int sd_bus_emit_properties_changed_strv(sd_bus *bus, const char *path, const char *interface, char **names) override; virtual int sd_bus_emit_object_added(sd_bus *bus, const char *path) override; virtual int sd_bus_emit_object_removed(sd_bus *bus, const char *path) override; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e387c42..0313b80 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -85,12 +85,14 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR}) #---------------------------------- add_executable(sdbus-c++-unit-tests ${UNITTESTS_SRCS} $) +target_compile_definitions(sdbus-c++-unit-tests PRIVATE LIBSYSTEMD_VERSION=${SYSTEMD_VERSION}) target_include_directories(sdbus-c++-unit-tests PRIVATE ${SYSTEMD_INCLUDE_DIRS} ${CMAKE_SOURCE_DIR}/src ${CMAKE_SOURCE_DIR}/include) target_link_libraries(sdbus-c++-unit-tests ${SYSTEMD_LIBRARIES} gmock gmock_main) add_executable(sdbus-c++-integration-tests ${INTEGRATIONTESTS_SRCS}) +target_compile_definitions(sdbus-c++-integration-tests PRIVATE LIBSYSTEMD_VERSION=${SYSTEMD_VERSION}) target_link_libraries(sdbus-c++-integration-tests sdbus-c++ gmock gmock_main) # Manual performance and stress tests diff --git a/tests/integrationtests/AdaptorAndProxy_test.cpp b/tests/integrationtests/AdaptorAndProxy_test.cpp index 96b6020..68753a0 100644 --- a/tests/integrationtests/AdaptorAndProxy_test.cpp +++ b/tests/integrationtests/AdaptorAndProxy_test.cpp @@ -232,6 +232,30 @@ TEST_F(SdbusTestObject, CallsMultiplyMethodWithNoReplyFlag) ASSERT_THAT(m_adaptor->m_multiplyResult, Eq(INT64_VALUE * DOUBLE_VALUE)); } +TEST_F(SdbusTestObject, CallsMethodWithCustomTimeoutSuccessfully) +{ + auto res = m_proxy->doOperationWith500msTimeout(20); // The operation will take 20ms, but the timeout is 500ms, so we are fine + ASSERT_THAT(res, Eq(20)); +} + +TEST_F(SdbusTestObject, ThrowsTimeoutErrorWhenMethodTimesOut) +{ + try + { + m_proxy->doOperationWith500msTimeout(1000); // The operation will take 1s, but the timeout is 500ms, so we should time out + FAIL() << "Expected sdbus::Error exception"; + } + catch (const sdbus::Error& e) + { + ASSERT_THAT(e.getName(), Eq("org.freedesktop.DBus.Error.Timeout")); + ASSERT_THAT(e.getMessage(), Eq("Connection timed out")); + } + catch(...) + { + FAIL() << "Expected sdbus::Error exception"; + } +} + TEST_F(SdbusTestObject, CallsMethodThatThrowsError) { try @@ -368,6 +392,20 @@ TEST_F(SdbusTestObject, FailsCallingMethodOnNonexistentObject) ASSERT_THROW(proxy.getInt(), sdbus::Error); } +#if LIBSYSTEMD_VERSION>=240 +TEST_F(SdbusTestObject, CanSetGeneralMethodTimeoutWithLibsystemdVersionGreaterThan239) +{ + s_connection->setMethodCallTimeout(5000000); + ASSERT_THAT(s_connection->getMethodCallTimeout(), Eq(5000000)); +} +#else +TEST_F(SdbusTestObject, CannotSetGeneralMethodTimeoutWithLibsystemdVersionLessThan240) +{ + ASSERT_THROW(s_connection->setMethodCallTimeout(5000000), sdbus::Error); + ASSERT_THROW(s_connection->getMethodCallTimeout(), sdbus::Error); +} +#endif + // Signals TEST_F(SdbusTestObject, EmitsSimpleSignalSuccesfully) diff --git a/tests/integrationtests/TestingProxy.h b/tests/integrationtests/TestingProxy.h index ceaf573..d45254e 100644 --- a/tests/integrationtests/TestingProxy.h +++ b/tests/integrationtests/TestingProxy.h @@ -122,6 +122,4 @@ public: // for tests std::function&)> m_onInterfacesRemovedHandler; }; - - #endif /* SDBUS_CPP_INTEGRATIONTESTS_TESTINGPROXY_H_ */ diff --git a/tests/integrationtests/proxy-glue.h b/tests/integrationtests/proxy-glue.h index c7ed37a..fb2b768 100644 --- a/tests/integrationtests/proxy-glue.h +++ b/tests/integrationtests/proxy-glue.h @@ -136,6 +136,14 @@ public: return result; } + uint32_t doOperationWith500msTimeout(uint32_t param) + { + using namespace std::chrono_literals; + uint32_t result; + object_.callMethod("doOperation").onInterface(INTERFACE_NAME).withTimeout(500000us).withArguments(param).storeResultsTo(result); + return result; + } + uint32_t doOperationAsync(uint32_t param) { uint32_t result; diff --git a/tests/unittests/mocks/SdBusMock.h b/tests/unittests/mocks/SdBusMock.h index c0dd9a1..830ff31 100644 --- a/tests/unittests/mocks/SdBusMock.h +++ b/tests/unittests/mocks/SdBusMock.h @@ -47,6 +47,9 @@ public: MOCK_METHOD2(sd_bus_message_new_method_return, int(sd_bus_message *call, sd_bus_message **m)); MOCK_METHOD3(sd_bus_message_new_method_error, int(sd_bus_message *call, sd_bus_message **m, const sd_bus_error *e)); + MOCK_METHOD2(sd_bus_set_method_call_timeout, int(sd_bus *bus, uint64_t usec)); + MOCK_METHOD2(sd_bus_get_method_call_timeout, int(sd_bus *bus, uint64_t *ret)); + MOCK_METHOD4(sd_bus_emit_properties_changed_strv, int(sd_bus *bus, const char *path, const char *interface, char **names)); MOCK_METHOD2(sd_bus_emit_object_added, int(sd_bus *bus, const char *path)); MOCK_METHOD2(sd_bus_emit_object_removed, int(sd_bus *bus, const char *path)); diff --git a/tools/xml2cpp-codegen/AdaptorGenerator.cpp b/tools/xml2cpp-codegen/AdaptorGenerator.cpp index 146388b..e7ec62c 100644 --- a/tools/xml2cpp-codegen/AdaptorGenerator.cpp +++ b/tools/xml2cpp-codegen/AdaptorGenerator.cpp @@ -193,7 +193,7 @@ std::tuple AdaptorGenerator::processMethods(const Node if (annotationValue == "true") annotationRegistration += ".markAsPrivileged()"; } - else + else if (annotationName != "org.freedesktop.DBus.Method.Timeout") // Whatever else... { std::cerr << "Node: " << methodName << ": " << "Option '" << annotationName << "' not allowed or supported in this context! Option ignored..." << std::endl; diff --git a/tools/xml2cpp-codegen/ProxyGenerator.cpp b/tools/xml2cpp-codegen/ProxyGenerator.cpp index 3082862..e48c4fa 100644 --- a/tools/xml2cpp-codegen/ProxyGenerator.cpp +++ b/tools/xml2cpp-codegen/ProxyGenerator.cpp @@ -34,7 +34,6 @@ #include #include - using std::endl; using sdbuscpp::xml::Document; @@ -142,6 +141,8 @@ std::tuple ProxyGenerator::processMethods(const Nodes& bool dontExpectReply{false}; bool async{false}; + std::string timeoutValue; + Nodes annotations = (*method)["annotation"]; for (const auto& annotation : annotations) { @@ -150,6 +151,8 @@ std::tuple ProxyGenerator::processMethods(const Nodes& else if (annotation->get("name") == "org.freedesktop.DBus.Method.Async" && (annotation->get("value") == "client" || annotation->get("value") == "clientserver")) async = true; + if (annotation->get("name") == "org.freedesktop.DBus.Method.Timeout") + timeoutValue = annotation->get("value"); } if (dontExpectReply && outArgs.size() > 0) { @@ -157,6 +160,12 @@ std::tuple ProxyGenerator::processMethods(const Nodes& std::cerr << "Option 'org.freedesktop.DBus.Method.NoReply' not allowed for methods with 'out' variables! Option ignored..." << std::endl; dontExpectReply = false; } + if (!timeoutValue.empty() && dontExpectReply) + { + std::cerr << "Function: " << name << ": "; + std::cerr << "Option 'org.freedesktop.DBus.Method.Timeout' not allowed for 'NoReply' methods! Option ignored..." << std::endl; + timeoutValue.clear(); + } auto retType = outArgsToType(outArgs); std::string inArgStr, inArgTypeStr; @@ -167,6 +176,11 @@ std::tuple ProxyGenerator::processMethods(const Nodes& definitionSS << tab << (async ? "void" : retType) << " " << name << "(" << inArgTypeStr << ")" << endl << tab << "{" << endl; + if (!timeoutValue.empty()) + { + definitionSS << tab << tab << "using namespace std::chrono_literals;" << endl; + } + if (outArgs.size() > 0 && !async) { definitionSS << tab << tab << retType << " result;" << endl; @@ -175,6 +189,11 @@ std::tuple ProxyGenerator::processMethods(const Nodes& definitionSS << tab << tab << "proxy_.callMethod" << (async ? "Async" : "") << "(\"" << name << "\")" ".onInterface(INTERFACE_NAME)"; + if (!timeoutValue.empty()) + { + definitionSS << ".withTimeout(" << timeoutValue << "us)"; + } + if (inArgs.size() > 0) { definitionSS << ".withArguments(" << inArgStr << ")";