| commit | 90f8d9b44700c4c4017355cab97788f262e7f9fd | [log] [tgz] |
|---|---|---|
| author | Hannu Lounento <hannu.lounento@vaisala.com> | Mon Mar 13 11:05:08 2023 +0200 |
| committer | Patrick Williams <patrick@stwcx.xyz> | Tue May 09 18:08:30 2023 +0000 |
| tree | 226532af656f0d5a88b339bcbf2678c8836ee2e2 | |
| parent | 27877eb995a2da529b2baab3ae50eec785a37a99 [diff] |
Allow propagating exceptions from server methods
sdbusplus calls the D-Bus method implementation functions in such a way
that the callstack contains sd_bus C functions from libsystemd, and
sdbusplus only catches the specific exceptions that match the errors
defined for the D-Bus method before returning execution to the C
functions. If any D-Bus method implementation propagates any other
exception than the errors the method declares, the C functions in the
callstack may leak resources.
There isn't any documentation or other statement that would prohibit
exceptions from being thrown / propagated from method implementations.
This seems to be a known issue though [1]:
By throwing an exception, you're not making an error return to the
calling client, but instead blowing through all of the `sd_bus` C
code with your C++ exception and putting your application into an
invalid state. At a minimum you are leaking memory.
Thus, catch any exceptions propagated from method implementations and
re-throw them from sdbusplus::bus::bus::process*().
Catching and re-throwing avoids resource leaks from sd_bus C functions.
And as a consequence it would allow propagating exceptions in the normal
way until the caller that is prepared to handle them -- ultimately even
up to main(). Propagating exceptions to `main()` allows for terminating
the process in a controlled fashion in case unexpected failures, which,
in turn, allows for a chance to recover from potential failures modes
that persist until application restart (e.g. due to some invalid state
stored in RAM).
Also, terminating the application, in which case the D-Bus daemon
returns the standard error org.freedesktop.DBus.Error.NoReply to the
client, avoids the need to declare an error for internal failures in the
D-Bus API. Internal errors in API add little value over
org.freedesktop.DBus.Error.NoReply and bloat the API as clients many
times can't handle them in any better way than
org.freedesktop.DBus.Error.NoReply.
Only std::exceptions are catched (i.e. not `catch (...)`) and propagated
to keep the implementation simple. The assumption is that only
exceptions (i.e. classes derived from std::exception) are used for
reporting errors.
Tested: With the calculator example changed to throw an unexpected exception:
From 914e045d1f92a1730d32f84ac7a74cd867c76760 Mon Sep 17 00:00:00 2001
From: Hannu Lounento <hannu.lounento@vaisala.com>
Date: Mon, 13 Mar 2023 15:53:17 +0200
Subject: [PATCH] [dbg] Add a propagating exception to the calculator example
Not to be merged to sdbusplus master but only for RFC / demonstration
purposes.
Can be triggered by commands like
busctl --user call net.poettering.Calculator /net/poettering/calculator net.poettering.Calculator Clear
busctl --user get-property net.poettering.Calculator /net/poettering/calculator net.poettering.Calculator LastResult
busctl --user set-property net.poettering.Calculator /net/poettering/calculator net.poettering.Calculator LastResult x 1
Change-Id: Ie9f47227afe7e0fa9765c602bec987aea71b9b86
Signed-off-by: Hannu Lounento <hannu.lounento@vaisala.com>
---
example/calculator-server.cpp | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/example/calculator-server.cpp b/example/calculator-server.cpp
index aa60662..eba9cc0 100644
--- a/example/calculator-server.cpp
+++ b/example/calculator-server.cpp
@@ -3,6 +3,7 @@
#include <net/poettering/Calculator/server.hpp>
#include <sdbusplus/server.hpp>
+#include <cstdlib>
#include <iostream>
#include <string_view>
@@ -42,10 +43,22 @@ struct Calculator : Calculator_inherit
/** Clear lastResult, broadcast 'Cleared' signal */
void clear() override
{
- auto v = lastResult();
- lastResult(0);
- cleared(v);
- return;
+ throw std::runtime_error{"A propagating exception"};
+ }
+
+ int64_t lastResult() const override
+ {
+ throw std::runtime_error{"A propagating exception"};
+ }
+
+ int64_t lastResult(int64_t /*value*/, bool /*skipSignal*/) override
+ {
+ throw std::runtime_error{"A propagating exception"};
+ }
+
+ int64_t lastResult(int64_t /*value*/) override
+ {
+ throw std::runtime_error{"A propagating exception"};
}
};
@@ -71,5 +84,13 @@ int main()
Calculator c1{b, path};
// Handle dbus processing forever.
- b.process_loop();
+ try
+ {
+ b.process_loop();
+ }
+ catch (const std::exception& e)
+ {
+ std::cerr << "Terminating due to a fatal condition: " << e.what() << std::endl;
+ return EXIT_FAILURE;
+ }
}
--
2.40.0
1) Built and executed unit tests:
sdbusplus$ rm -rf build/ && CXXFLAGS=-Wno-maybe-uninitialized meson setup build && cd build && ninja && ninja test
The Meson build system
Version: 0.62.2
Source dir: /path/to/sdbusplus
Build dir: /path/to/sdbusplus/build
Build type: native build
Project name: sdbusplus
Project version: 1.0.0
C compiler for the host machine: ccache cc (gcc 12.2.1 "cc (GCC) 12.2.1 20221121 (Red Hat 12.2.1-4)")
C linker for the host machine: cc ld.bfd 2.37-37
C++ compiler for the host machine: ccache c++ (gcc 12.2.1 "c++ (GCC) 12.2.1 20221121 (Red Hat 12.2.1-4)")
C++ linker for the host machine: c++ ld.bfd 2.37-37
Host machine cpu family: x86_64
Host machine cpu: x86_64
Found pkg-config: /usr/bin/pkg-config (1.8.0)
Run-time dependency libsystemd found: YES 250
Program python3 (inflection, yaml, mako) found: YES (/usr/bin/python3) modules: inflection, yaml, mako
Run-time dependency Boost found: YES 1.76.0 (/usr)
Program sdbus++ found: YES (/path/to/sdbusplus/tools/sdbus++)
Program sdbus++ found: YES (overridden)
Program sdbus++-gen-meson found: YES (/path/to/sdbusplus/tools/sdbus++-gen-meson)
Program sdbus++-gen-meson found: YES (overridden)
Header <boost/asio.hpp> has symbol "boost::asio::io_context" : YES
Run-time dependency Boost (found: context, coroutine) found: YES 1.76.0 (/usr)
Run-time dependency GTest found: YES 1.11.0
Run-time dependency GMock found: YES 1.11.0
Build targets in project: 34
Found ninja-1.10.2 at /usr/bin/ninja
[77/77] Linking target example/calculator-client
[0/1] Running all tests.
1/21 test_async_task OK 0.03s
2/21 test_bus_list_names OK 0.02s
3/21 test_bus_match OK 0.02s
4/21 test_bus_exception OK 0.02s
5/21 test_exception_sdbus_error OK 0.02s
6/21 test_message_append OK 0.01s
7/21 test_message_native_types OK 0.01s
8/21 test_message_read OK 0.03s
9/21 test_message_types OK 0.02s
10/21 test_unpack_properties OK 0.02s
11/21 test_utility_tuple_to_array OK 0.02s
12/21 test_utility_type_traits OK 0.01s
13/21 test-bus_aio OK 0.01s
14/21 test-vtable OK 0.01s
15/21 test-server OK 0.01s
16/21 test-server-message-variant OK 0.01s
17/21 test_message_call OK 0.07s
18/21 test_event_event OK 0.31s
19/21 test_async_timer OK 0.51s
20/21 test_async_context OK 0.52s
21/21 test_timer OK 18.02s
Ok: 21
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
[...]
-Wno-maybe-uninitialized was used because the current master
384943be7e81b08ed102abfaa3f2be2ad915e800 ("sdbus++: async: client: fix
client_t usage") failed to build with
In file included from /usr/include/gtest/internal/gtest-death-test-internal.h:39,
from /usr/include/gtest/gtest-death-test.h:41,
from /usr/include/gtest/gtest.h:64,
from /usr/include/gmock/internal/gmock-internal-utils.h:47,
from /usr/include/gmock/gmock-actions.h:145,
from /usr/include/gmock/gmock.h:59,
from ../include/sdbusplus/test/sdbus_mock.hpp:6,
from ../test/exception/sdbus_error.cpp:4:
In copy constructor ‘testing::internal::MatcherBase<T>::MatcherBase(const testing::internal::MatcherBase<T>&) [with T = sd_bus_error*]’,
inlined from ‘testing::Matcher<sd_bus_error*>::Matcher(const testing::Matcher<sd_bus_error*>&)’ at /usr/include/gtest/gtest-matchers.h:479:7,
inlined from ‘testing::internal::MockSpec<int(sd_bus_error*, int)> sdbusplus::SdBusMock::gmock_sd_bus_error_set_errno(const testing::Matcher<sd_bus_error*>&, const testing::Matcher<int>&)’ at ../include/sdbusplus/test/sdbus_mock.hpp:51:5,
inlined from ‘virtual void {anonymous}::SdBusError_NotSetErrno_Test::TestBody()’ at ../test/exception/sdbus_error.cpp:59:5:
/usr/include/gtest/gtest-matchers.h:302:33: error: ‘<unnamed>.testing::Matcher<sd_bus_error*>::<unnamed>.testing::internal::MatcherBase<sd_bus_error*>::buffer_’ may be used uninitialized [-Werror=maybe-uninitialized]
302 | : vtable_(other.vtable_), buffer_(other.buffer_) {
| ^~~~~~~~~~~~~~~~~~~~~~
on Fedora 36 with
$ gcc --version
gcc (GCC) 12.2.1 20221121 (Red Hat 12.2.1-4)
and
$ dnf info gtest-devel
[...]
Name : gtest-devel
Version : 1.11.0
Release : 2.fc36
Architecture : x86_64
2) Verified no memory leaks were reported by Valgrind anymore:
sdbusplus/build$ valgrind --leak-check=full --show-leak-kinds=all ./example/calculator-server
==61886== Memcheck, a memory error detector
==61886== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==61886== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==61886== Command: ./example/calculator-server
==61886==
and called the throwing method:
build$ busctl --user call net.poettering.Calculator /net/poettering/calculator net.poettering.Calculator Clear
Call failed: Remote peer disconnected
and verified the process terminates but produces no memory leaks:
Terminating due to a fatal condition: A propagating exception
==61886==
==61886== HEAP SUMMARY:
==61886== in use at exit: 0 bytes in 0 blocks
==61886== total heap usage: 143 allocs, 143 frees, 96,732 bytes allocated
==61886==
==61886== All heap blocks were freed -- no leaks are possible
==61886==
==61886== For lists of detected and suppressed errors, rerun with: -s
==61886== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Similarly reading the `LastResult` property:
build$ valgrind --leak-check=full --show-leak-kinds=all ./example/calculator-server
==36140== Memcheck, a memory error detector
==36140== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==36140== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==36140== Command: ./example/calculator-server
==36140==
build$ busctl --user get-property net.poettering.Calculator /net/poettering/calculator net.poettering.Calculator LastResult
Failed to get property LastResult on interface net.poettering.Calculator: Remote peer disconnected
Terminating due to a fatal condition: A propagating exception
==36140==
==36140== HEAP SUMMARY:
==36140== in use at exit: 0 bytes in 0 blocks
==36140== total heap usage: 148 allocs, 148 frees, 96,992 bytes allocated
==36140==
==36140== All heap blocks were freed -- no leaks are possible
==36140==
==36140== For lists of detected and suppressed errors, rerun with: -s
==36140== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
and writing the property:
==36383== Memcheck, a memory error detector
==36383== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==36383== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==36383== Command: ./example/calculator-server
==36383==
build$ busctl --user set-property net.poettering.Calculator /net/poettering/calculator net.poettering.Calculator LastResult x 1
Terminating due to a fatal condition: A propagating exception
==36383==
==36383== HEAP SUMMARY:
==36383== in use at exit: 0 bytes in 0 blocks
==36383== total heap usage: 146 allocs, 146 frees, 97,010 bytes allocated
==36383==
==36383== All heap blocks were freed -- no leaks are possible
==36383==
==36383== For lists of detected and suppressed errors, rerun with: -s
==36383== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
[1] https://lore.kernel.org/openbmc/YTDvfIn4Z05mGdCx@heinlein/
Change-Id: I014099a91f526a92c0f6646e75b4029513ad19a8
Signed-off-by: Hannu Lounento <hannu.lounento@vaisala.com>
sdbusplus contains two parts:
The sdbusplus library requires sd-bus, which is contained in libsystemd.
The sdbus++ application requires Python 3 and the Python libraries mako and inflection.
The sdbusplus library is built using meson.
meson build
cd build
ninja
ninja test
ninja install
Optionally, building the tests and examples can be disabled by passing -Dtests=disabled and -Dexamples=disabled respectively to meson.
The sdbus++ application is installed as a standard Python package using setuptools.
cd tools ./setup.py install
The sdbusplus library builds on top of the sd-bus library to create a modern C++ API for D-Bus. The library attempts to be as lightweight as possible, usually compiling to exactly the sd-bus API calls that would have been necessary, while also providing compile-time type-safety and memory leak protection afforded by modern C++ practices.
Consider the following code:
auto b = bus::new_default_system(); auto m = b.new_method_call("org.freedesktop.login1", "/org/freedesktop/login1", "org.freedesktop.login1.Manager", "ListUsers"); auto reply = b.call(m); std::vector<std::tuple<uint32_t, std::string, message::object_path>> users; reply.read(users); // or auto users = reply.unpack< std::vector<std::tuple<uint32_t, std::string, message::object_path>>>();
In a few, relatively succinct, C++ lines this snippet will create a D-Bus connection to the system bus, and call the systemd login manager to get a list of active users. The message and bus objects are automatically freed when they leave scope and the message format strings are generated at compile time based on the types being read. Compare this to the corresponding server code within logind.
In general, the library attempts to mimic the naming conventions of the sd-bus library: ex. sd_bus_call becomes sdbusplus::bus::call, sd_bus_get_unique_name becomes sdbusplus::bus::get_unique_name, sd_bus_message_get_signature becomes sdbusplus::message::get_signature, etc. This allows a relatively straight-forward translation back to the sd-bus functions for looking up the manpage details.
sdbusplus also contains a bindings generator tool: sdbus++. The purpose of a bindings generator is to reduce the boilerplate associated with creating D-Bus server or client applications. When creating a server application, rather than creating sd-bus vtables and writing C-style functions to handle each vtable callback, you can create a small YAML file to define your D-Bus interface and the sdbus++ tool will create a C++ class that implements your D-Bus interface. This class has a set of virtual functions for each method and property, which you can overload to create your own customized behavior for the interface.
There are currently two types of YAML files: interface and error. Interfaces are used to create server and client D-Bus interfaces. Errors are used to define C++ exceptions which can be thrown and will automatically turn into D-Bus error responses.
[[D-Bus client bindings are not yet implemented. See openbmc/openbmc#851.]]
The path of your file will be the interface name. For example, for an interface org.freedesktop.Example, you would create the files org/freedesktop/Example.interface.yaml and org/freedesktop/Example.errors.yaml] for interfaces and errors respectively. These can then be used to generate the server and error bindings:
sdbus++ interface server-header org.freedesktop.Example > \ org/freedesktop/Example/server.hpp sdbus++ interface server-cpp org.freedesktop.Example > \ org/freedesktop/Example/server.cpp sdbus++ error exception-header org.freedesktop.Example > \ org/freedesktop/Example/error.hpp \ sdbus++ error exception-cpp org.freedesktop.Example > \ org/freedesktop/Example/error.cpp
Markdown-based documentation can also be generated from the interface and exception files:
sdbus++ interface markdown org.freedesktop.Example > \ org/freedesktop/Example.md sdbus++ error markdown org.freedesktop.Example >> \ org/freedesktop/Example.md
See the example/meson.build for more details.
Installation of sdbusplus bindings on a custom distribution requires a few packages to be installed prior. Although these packages are the same for several distributions the names of these packages do differ. Below are the packages needed for Ubuntu and Fedora.
sudo apt install git meson libtool pkg-config g++ libsystemd-dev \ python3 python3-pip python3-yaml python3-mako python3-inflection
sudo dnf install git meson libtool gcc-c++ pkgconfig systemd-devel \ python3 python3-pip python3-yaml python3-mako
Install the inflection package using the pip utility (on Fedora)
pip3 install inflection