diff --git a/ChangeLog b/ChangeLog index d74337b..65841db 100644 --- a/ChangeLog +++ b/ChangeLog @@ -242,14 +242,16 @@ v1.4.0 v2.x - Breaking changes in API/ABI/behavior: - - In *synchronous* D-Bus calls, the proxy now **always** blocks the connection for concurrent use until the call finishes (with either a received reply, - an error, or time out). If this creates a connection contention issue in your multi-threaded design, use short-lived, light-weight proxies, or call - the method in an asynchronous way. - - The `PollData` struct has been extended with a new data member: `eventFd`. All hooks with external event loops shall be modified to poll on this fd as well. - - `PollData::timeout_usec` was renamed to `PollData::timeout` and its type has been changed to `std::chrono::microseconds`. This member now holds directly - what before had to be obtained through `PollData::getAbsoluteTimeout()` call. - - `PollData::getRelativeTimeout()` return type was changed to `std::chrono::microseconds`. - - `IConnection::processPendingRequest()` was renamed to `IConnection::processPendingEvent()`. + - In *synchronous* D-Bus calls, the proxy now **always** blocks the connection for concurrent use until the call finishes (with either a received reply, + an error, or time out). If this creates a connection contention issue in your multi-threaded design, use short-lived, light-weight proxies, or call + the method in an asynchronous way. + - Signatures of callbacks `async_reply_handler`, `signal_handler`, `message_handler` and `property_set_callback` were modified to take input message objects + by value, as opposed to non-const ref. Callee assumes ownership of the message. This API is more idiomatic, cleaner and safer. + - The `PollData` struct has been extended with a new data member: `eventFd`. All hooks with external event loops shall be modified to poll on this fd as well. + - `PollData::timeout_usec` was renamed to `PollData::timeout` and its type has been changed to `std::chrono::microseconds`. This member now holds directly + what before had to be obtained through `PollData::getAbsoluteTimeout()` call. + - `PollData::getRelativeTimeout()` return type was changed to `std::chrono::microseconds`. + - `IConnection::processPendingRequest()` was renamed to `IConnection::processPendingEvent()`. - Systemd of at least v238 is required to compile sdbus-c++ - A proper fix for timeout handling - Fix for external event loops in which the event loop thread ID was not correctly initialized (now fixed and simplified by not needing the thread ID anymore) diff --git a/docs/using-sdbus-c++.md b/docs/using-sdbus-c++.md index 8e4f947..d771402 100644 --- a/docs/using-sdbus-c++.md +++ b/docs/using-sdbus-c++.md @@ -330,7 +330,7 @@ Please note that we can create and destroy D-Bus objects on a connection dynamic #include #include -void onConcatenated(sdbus::Signal& signal) +void onConcatenated(sdbus::Signal signal) { std::string concatenatedString; signal >> concatenatedString; @@ -389,7 +389,7 @@ int main(int argc, char *argv[]) In simple cases, we don't need to create D-Bus connection explicitly for our proxies. Unless a connection is provided to a proxy object explicitly via factory parameter, the proxy will create a connection of his own (unless it is a light-weight, short-lived proxy created with `dont_run_event_loop_thread_t`), and it will be a system bus connection. This is the case in the example above. (This approach is not scalable and resource-saving if we have plenty of proxies; see section [Working with D-Bus connections](#working-with-d-bus-connections-in-sdbus-c) for elaboration.) So, in the example, we create a proxy for object `/org/sdbuscpp/concatenator` publicly available at bus `org.sdbuscpp.concatenator`. We register signal handlers, if any, and finish the registration, making the proxy ready for use. -The callback for a D-Bus signal handler on this level is any callable of signature `void(sdbus::Signal& signal)`. The one and only parameter `signal` is the incoming signal message. We need to deserialize arguments from it, and then we can do our business logic with it. +The callback for a D-Bus signal handler on this level is any callable of signature `void(sdbus::Signal signal)`. The one and only parameter `signal` is the incoming signal message. We need to deserialize arguments from it, and then we can do our business logic with it. Subsequently, we invoke two RPC calls to object's `concatenate()` method. We create a method call message by invoking proxy's `createMethodCall()`. We serialize method input arguments into it, and make a synchronous call via proxy's `callMethod()`. As a return value we get the reply message as soon as it arrives. We deserialize return values from that message, and further use it in our program. The second `concatenate()` RPC call is done with invalid arguments, so we get a D-Bus error reply from the service, which as we can see is manifested via `sdbus::Error` exception being thrown. @@ -1117,7 +1117,7 @@ int main(int argc, char *argv[]) { /* ... */ - auto callback = [](MethodReply& reply, const sdbus::Error* error) + auto callback = [](MethodReply reply, const sdbus::Error* error) { if (error == nullptr) // No error { diff --git a/include/sdbus-c++/ConvenienceApiClasses.inl b/include/sdbus-c++/ConvenienceApiClasses.inl index f84dc93..0dd883c 100644 --- a/include/sdbus-c++/ConvenienceApiClasses.inl +++ b/include/sdbus-c++/ConvenienceApiClasses.inl @@ -317,7 +317,7 @@ namespace sdbus { if (propertySignature_.empty()) propertySignature_ = signature_of_function_input_arguments<_Function>::str(); - setter_ = [callback = std::forward<_Function>(callback)](PropertySetCall& call) + setter_ = [callback = std::forward<_Function>(callback)](PropertySetCall call) { // Default-construct property value using property_type = function_argument_t<_Function, 0>; @@ -580,7 +580,7 @@ namespace sdbus { { assert(method_.isValid()); // onInterface() must be placed/called prior to this function - auto asyncReplyHandler = [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. @@ -656,7 +656,7 @@ namespace sdbus { proxy_.registerSignalHandler( interfaceName_ , signalName_ - , [callback = std::forward<_Function>(callback)](Signal& signal) + , [callback = std::forward<_Function>(callback)](Signal signal) { // Create a tuple of callback input arguments' types, which will be used // as a storage for the argument values deserialized from the signal message. diff --git a/include/sdbus-c++/IConnection.h b/include/sdbus-c++/IConnection.h index 77a79f0..bb6e59c 100644 --- a/include/sdbus-c++/IConnection.h +++ b/include/sdbus-c++/IConnection.h @@ -286,7 +286,7 @@ namespace sdbus { * The syntax of the match rule expression passed in match is described in the D-Bus specification. * The specified handler function callback is called for each incoming message matching the specified * expression. The match is installed synchronously when connected to a bus broker, i.e. the call - * sends a control message requested the match to be added to the broker and waits until the broker + * sends a control message requesting the match to be added to the broker and waits until the broker * confirms the match has been installed successfully. * * Simply let go of the slot instance to uninstall the match rule from the bus connection. The slot diff --git a/include/sdbus-c++/TypeTraits.h b/include/sdbus-c++/TypeTraits.h index 1de2ef4..acd8607 100644 --- a/include/sdbus-c++/TypeTraits.h +++ b/include/sdbus-c++/TypeTraits.h @@ -62,10 +62,10 @@ namespace sdbus { // Callbacks from sdbus-c++ using method_callback = std::function; - using async_reply_handler = std::function; - using signal_handler = std::function; - using message_handler = std::function; - using property_set_callback = std::function; + using async_reply_handler = std::function; + using signal_handler = std::function; + using message_handler = std::function; + using property_set_callback = std::function; using property_get_callback = std::function; // Type-erased RAII-style handle to callbacks/subscriptions registered to sdbus-c++ diff --git a/src/Connection.cpp b/src/Connection.cpp index 3b8a470..9099a6a 100644 --- a/src/Connection.cpp +++ b/src/Connection.cpp @@ -811,7 +811,7 @@ int Connection::sdbus_match_callback(sd_bus_message *sdbusMessage, void *userDat { auto* matchInfo = static_cast(userData); auto message = Message::Factory::create(sdbusMessage, &matchInfo->connection.getSdBusInterface()); - auto ok = invokeHandlerAndCatchErrors([&](){ matchInfo->callback(message); }, retError); + auto ok = invokeHandlerAndCatchErrors([&](){ matchInfo->callback(std::move(message)); }, retError); return ok ? 0 : -1; } @@ -819,7 +819,7 @@ int Connection::sdbus_match_install_callback(sd_bus_message *sdbusMessage, void { auto* matchInfo = static_cast(userData); auto message = Message::Factory::create(sdbusMessage, &matchInfo->connection.getSdBusInterface()); - auto ok = invokeHandlerAndCatchErrors([&](){ matchInfo->installCallback(message); }, retError); + auto ok = invokeHandlerAndCatchErrors([&](){ matchInfo->installCallback(std::move(message)); }, retError); return ok ? 0 : -1; } diff --git a/src/Object.cpp b/src/Object.cpp index d972195..8cefe61 100644 --- a/src/Object.cpp +++ b/src/Object.cpp @@ -341,7 +341,7 @@ int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, auto& callback = interfaceData->methods[message.getMemberName()].callback; assert(callback); - auto ok = invokeHandlerAndCatchErrors([&](){ callback(message); }, retError); + auto ok = invokeHandlerAndCatchErrors([&](){ callback(std::move(message)); }, retError); return ok ? 1 : -1; } @@ -390,7 +390,7 @@ int Object::sdbus_property_set_callback( sd_bus */*bus*/ auto value = Message::Factory::create(sdbusValue, &object.connection_.getSdBusInterface()); - auto ok = invokeHandlerAndCatchErrors([&](){ callback(value); }, retError); + auto ok = invokeHandlerAndCatchErrors([&](){ callback(std::move(value)); }, retError); return ok ? 1 : -1; } diff --git a/src/Proxy.cpp b/src/Proxy.cpp index a2826c6..856dffc 100644 --- a/src/Proxy.cpp +++ b/src/Proxy.cpp @@ -119,10 +119,10 @@ std::future Proxy::callMethod(const MethodCall& message, uint64_t t auto promise = std::make_shared>(); auto future = promise->get_future(); - async_reply_handler asyncReplyCallback = [promise = std::move(promise)](MethodReply& reply, const Error* error) noexcept + async_reply_handler asyncReplyCallback = [promise = std::move(promise)](MethodReply reply, const Error* error) noexcept { if (error == nullptr) - promise->set_value(reply); + promise->set_value(std::move(reply)); else promise->set_exception(std::make_exception_ptr(*error)); }; @@ -230,12 +230,12 @@ int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userDat const auto* error = sd_bus_message_get_error(sdbusMessage); if (error == nullptr) { - asyncCallData->callback(message, nullptr); + asyncCallData->callback(std::move(message), nullptr); } else { Error exception(error->name, error->message); - asyncCallData->callback(message, &exception); + asyncCallData->callback(std::move(message), &exception); } }, retError); @@ -250,7 +250,7 @@ int Proxy::sdbus_signal_handler(sd_bus_message *sdbusMessage, void *userData, sd auto message = Message::Factory::create(sdbusMessage, &signalData->proxy.connection_->getSdBusInterface()); - auto ok = invokeHandlerAndCatchErrors([&](){ signalData->callback(message); }, retError); + auto ok = invokeHandlerAndCatchErrors([&](){ signalData->callback(std::move(message)); }, retError); return ok ? 0 : -1; } diff --git a/src/Proxy.h b/src/Proxy.h index af45d63..ab1d7ad 100644 --- a/src/Proxy.h +++ b/src/Proxy.h @@ -119,7 +119,7 @@ namespace sdbus::internal { public: struct CallData { - enum class State + enum class State // TODO: In release/v2.0, we no more have sync-in-terms-of-async, we can simplify code with just bool finished like before { NOT_ASYNC , RUNNING , FINISHED diff --git a/tests/integrationtests/DBusGeneralTests.cpp b/tests/integrationtests/DBusGeneralTests.cpp index 17816c0..c7b318b 100644 --- a/tests/integrationtests/DBusGeneralTests.cpp +++ b/tests/integrationtests/DBusGeneralTests.cpp @@ -76,7 +76,7 @@ TYPED_TEST(AConnection, WillCallCallbackHandlerForIncomingMessageMatchingMatchRu { auto matchRule = "sender='" + BUS_NAME + "',path='" + OBJECT_PATH + "'"; std::atomic matchingMessageReceived{false}; - auto slot = this->s_proxyConnection->addMatch(matchRule, [&](sdbus::Message& msg) + auto slot = this->s_proxyConnection->addMatch(matchRule, [&](sdbus::Message msg) { if(msg.getPath() == OBJECT_PATH) matchingMessageReceived = true; @@ -93,12 +93,12 @@ TYPED_TEST(AConnection, CanInstallMatchRuleAsynchronously) std::atomic matchingMessageReceived{false}; std::atomic matchRuleInstalled{false}; auto slot = this->s_proxyConnection->addMatchAsync( matchRule - , [&](sdbus::Message& msg) + , [&](sdbus::Message msg) { if(msg.getPath() == OBJECT_PATH) matchingMessageReceived = true; } - , [&](sdbus::Message& /*msg*/) + , [&](sdbus::Message /*msg*/) { matchRuleInstalled = true; } ); @@ -114,7 +114,7 @@ TYPED_TEST(AConnection, WillUnsubscribeMatchRuleWhenClientDestroysTheAssociatedS { auto matchRule = "sender='" + BUS_NAME + "',path='" + OBJECT_PATH + "'"; std::atomic matchingMessageReceived{false}; - auto slot = this->s_proxyConnection->addMatch(matchRule, [&](sdbus::Message& msg) + auto slot = this->s_proxyConnection->addMatch(matchRule, [&](sdbus::Message msg) { if(msg.getPath() == OBJECT_PATH) matchingMessageReceived = true; @@ -132,7 +132,7 @@ TYPED_TEST(AConnection, CanAddFloatingMatchRule) std::atomic matchingMessageReceived{false}; auto con = sdbus::createSystemBusConnection(); con->enterEventLoopAsync(); - auto callback = [&](sdbus::Message& msg) + auto callback = [&](sdbus::Message msg) { if(msg.getPath() == OBJECT_PATH) matchingMessageReceived = true; @@ -153,7 +153,7 @@ TYPED_TEST(AConnection, WillNotPassToMatchCallbackMessagesThatDoNotMatchTheRule) { auto matchRule = "type='signal',interface='" + INTERFACE_NAME + "',member='simpleSignal'"; std::atomic numberOfMatchingMessages{}; - auto slot = this->s_proxyConnection->addMatch(matchRule, [&](sdbus::Message& msg) + auto slot = this->s_proxyConnection->addMatch(matchRule, [&](sdbus::Message msg) { if(msg.getMemberName() == "simpleSignal") numberOfMatchingMessages++;