This PR makes things around connection factories a little more consistent and more intuitive:
* createConnection() has been removed. One shall call more expressive createSystemConnection() instead to get a connection to the system bus.
* createDefaultBusConnection() has been renamed to createBusConnection(), so as not to be confused with libsystemd's default_bus, which is a different thing (a reusable thread-local bus).
Proxies still by default call createBusConnection() to get a connection when the connection is not provided explicitly by the caller, but now createBusConnection() does a different thing, so now the proxies connect to either session bus or system bus depending on the context (as opposed to always to system bus like before).
In sdbus-c++ v1, new virtual functions (e.g. overloads of existing virtual functions) were always placed at the end of the class to keep backwards ABI compatibility. Now, with v2, these functions are reordered and functions forming a logical group are together.
This improves usability of sdbus-c++ in downstream CMake projects. CMake options now have `SDBUSCPP_` prefix so potential conflicts with downstream options/variables are minimized. Also, all configurable options and variables are placed in a single place in the root CMake file.
This switches from a raw pointer to std::optional type to pass prospective call errors to the client (using std::optional was not possible years back when sdbus-c++ was based on C++14). This makes the API a little clearer, safer, idiomatically more expressive, and removes potential confusion associated with raw pointers (like ownership, lifetime questions, etc.).
This makes D-Bus proxy signal registration more flexible, more dynamic, and less error-prone since no `finishRegistration()` call is needed. A proxy can register to a signal at any time during its lifetime, and can unregister freely by simply destroying the associated slot.
This improves the D-Bus object API registration/unregistration by making it more flexible, more dynamic, closer to sd-bus API design but still on high abstraction level, and -- most importantly -- less error-prone since no `finishRegistration()` call is needed anymore.
sd_bus_match_signal() is more convenient than more general sd_bus_add_match() for signal registration, and requires less code on our side. Plus, systemd of at least v238 is required for sdbus-c++ v2, and this version already ships with sd_bus_match_signal().
This makes the library more robust and prone to user's errors when the user writes an extension for their custom type. In case they forget to implement a serialization function for that type and yet insert an object of that type into sdbus::Message, the current behavior is that, surprisingly, the library masks the error as it resolves the call to the Variant overload, because Variant provides an implicit template converting constructor, so the library tries to construct first the Variant object from the object of custom type, and then inserting into the message that Variant object. Variant constructor serializes the underlying object into its internal message object, which resolves to the same message insertion overload, creating an infinite recursion and ultimately the stack overflow. This is undesired and plain wrong. Marking this Variant converting constructor solves these problems, plus in overall it makes the code a little safer and more verbose. With explicit Variant constructor, when the user forgets to implement a serialization function for their type, the call of such function will fail with an expressive compilation error, and will produce no undesired, surprising results.
The test now works with Clang, libc++ and -O2 optimization, since the underlying implementation has been completely re-designed and doesn't suffer from that problem anymore.
Since synchronous D-Bus calls are simplified in v2.0 and no more implemented in terms of an async call, the issue reported in #362 is no more relevant, and we can return to the simple boolean flag for indicating a finished call.
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.
The callee assumes ownership of the message. This API is more idiomatic, more expressive, cleaner and safer. Move semantics is used to pass messages to the callback handlers. In some cases, this also improves performance.
In case a signal arrives during the `RequestName` or `ReleaseName` D-Bus call (which is a synchronous call), the signal may not be processed immediately, which is a bug. This is solved now by waking up the event loop.
* Change default test installation path to tests/sdbus-c++ , while also respecting CMAKE_INSTALL_PREFIX
* Introduce INSTALL_TESTS CMake option, so BUILD_TESTS is now split into BUILD_TESTS just for building and INSTALL_TESTS for installing the test binaries
Per discussion in #358 (comment), the change in the default settings is fine a for a minor release.
* chore: Use std::exchange in UnixFd
This was suggested in code review for #376 .
* fix: Protect against UnixFd self-assignment
While self-assignment is rare, it is expected to be safe. Add a check
to prevent putting the object in an invalid state.
* fix: Improve hygiene around dup system call
- Don't try to call dup on a negative value.
- Check dup return code and throw if it fails, rather than returning an
empty UnixFd object.
* chore: Move UnixFd::close to Types.cpp
Minor convenience for applications: unistd.h doesn't have to be included
in the public header.
---------
Co-authored-by: David Reiss <dreiss@meta.com>
* Catch and process all exceptions (not just sdbus::Error) from callback handlers
* Unify handling of exceptions from all types of callbacks -- always set sd_bus_error and return a negative result number in case of exception
Although libsystemd logs (with DEBUG severity) all errors from such callback handlers (except method callback handler), it seems to be out of our control. One of handy sdbus-c++ features could be the ability for clients to install a log callback, which sdbus-c++ would call in case of exceptions flying from callback handlers. In case something doesn't work for clients (especially novices), they can first look into these logs.
This may be handy in common situations like ignored signals on client side because of the inadvertent mismatch between real signal signature and signal handler signature. Like here: #373. (Although in this specific case of signals, there is a solution with an additional const sdbus::Error* argument that would reveal such an error.)
* fix: Use-after-return in synchronous calls
This bug was introduced by c39bc637b8 and can be reproduced by
configuring with
cmake -S . -B build -DBUILD_TESTS=yes -DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_CXX_FLAGS="-Wno-error=deprecated-copy -fno-omit-frame-pointer -fsanitize=address -fsanitize-address-use-after-scope"
and running `cmake --buid build && cmake --build build -t test` or
`build/tests/sdbus-c++-integration-tests --gtest_filter=SdbusTestObject.HandlesCorrectlyABulkOfParallelServerSideAsyncMethods`
The issue is that `sdbus_async_reply_handler` can call `removeCall`, which
writes to `data->finished`, but `data` can point to the stack of
`sendMethodCallMessageAndWaitForReply`, which can return as soon as
`asyncCallData->callback` is called.
As a fix, I restored some of the logic removed in c39bc637b8.
Specifically, in `sdbus_async_reply_handler`, I make a copy of some data
from `asyncCallData` (a new `state` field instead of `slot`), and in the
`SCOPE_GUARD`, I don't call `removeCall` if the call was actually
synchronous.
* refactor: use enum class instead of int
---------
Co-authored-by: David Reiss <dreiss@meta.com>
Co-authored-by: Stanislav Angelovič <stanislav.angelovic@protonmail.com>
