feat: add coroutine support for client-side asynchronous calls#527
Conversation
Include .vscode/ in .gitignore to prevent IDE-specific files from being tracked in the repository.
Prepare for C++20 coroutine support for asynchronous D-Bus method calls by introducing the Awaitable<T> class, enabling co_await syntax. This class contains the three necessary methods to support the co_await operator: await_ready, await_suspend and await_resume. A shared data structure is used to hold data needed by the callback that will resume the coroutine and feed in the result or exception. This work similarly to how a future/promise pair work. Potential race conditions in the multithreaded scenario are covered by atomic operations on an AwaitableState value, preventing situations that could lead to a double coroutine resume or permanent suspension.
Add three new flavors of the callMethodAsync method in the IProxy interface that use the with_awaitable_t tag to return an Awaitable<MethodReply>. Implementation relies on the callback-based mechanism to insert the results into the shared AwaitableData, transition the state to Completed and resume the coroutine.
Add a .getResultAsAwaitable method to each of the relevant high-level API helpers (AsyncMethodInvoker, AsyncPropertyGetter, AsyncPropertySetter, and AsyncAllPropertiesGetter). Implementation is similar to the low-level API: rely on the .uponReplyInvoke() mechanism to set the results on the shared data, atomically flip the state, and resume the suspended coroutine.
Adapt the StandardInterface.h classes to include an awaitable-based overload of the async methods.
…ation The current implementation relies on two flags to determine if async is enabled for methods/properties. In preparation to introduce the generation of the coroutine-friendly awaitable methods as an async implementation, refactor this as an enum containing the implementations and an std::optional to mark simultaneously if async is enabled and the method. There are no behavioral changes or new functonality. C++ 17 is needed for std::optional, therefore the used C++ standard for the tool has been changed.
Use the new awaitable-returning methods from the high level API to add support to a third async implementation mode alongside the existing callback and future implementations. Add a new enum entry to AsyncImpl and parse the "awaitable" or "coroutine" values in the relevant annotations for methods and properties. Methods annotated with this implementation will return sdbus::Awaitable<ReturnType> and call .getResultAsAwaitable<>() on the high-level API. Properties follow the same pattern, returning sdbus::Awaitable<sdbus::Variant> for getters and sdbus::Awaitable<void> for setters. The nested ternary expressions for determining return types have been refactored into explicit if-else blocks for improved readability and to accommodate the additional implementation type. The existing callback and future implementations remain unchanged in behavior.
|
Hi there!
Also thanks for the work you put in the project, I always had a good time using it. Please let me know if there are further changes or design considerations needed |
Introduce new awaitable methods in TestProxy for asynchronous operations and add corresponding integration tests in DBusAwaitableMethodsTests.cpp to validate functionality. The test uses a simple coroutine task type which uses a promise/future pair to pass data between the callback and the coroutine, leveraging the fact that the tests are all multithreaded. This approach would not work in a single-threaded scenario, as the future's .get() method would block the thread.
Update the main tutorial with the new awaitable-based async method.
alexcani
force-pushed
the
alexcani/awaitables
branch
from
e8ceebb to
3988cd7
Compare
|
Hey @alexcani. Thanks a lot for contributing and submitting the PR! I started looking into that but am a bit overloaded currently, I'll definitely come back to this asap. |
|
Hi! I reviewed the PR. Appears to me to be one of the best, well-done PRs I've reviewed on this project. It is enjoyable to see changes done to nice little details and across all levels. I have no experience with coroutines, so I had to do the research first. Still, given lack of experience, I don't feel 100% confident about all the details of good coroutines design. But given the quality of your work, I think I can trust you. However, I did some deeper reflection about things like overall design, thread coordination, exception handling, etc. -- and when I tried to think about them out-of-the-box, I got back to your solution, understanding your motives. Again, I see you've put an aspect of quality and craftsmanship to your work, and this is something I highly appreciate. One thing I was asking myself was -- in
I've already prepared a local patch for that. It works nicely. I can push that to your branch if you're okay with that, for you to review. Let me know. In the code I touched, I also changed the formatting a bit to stay consistent with the surrounding code. Regarding code style, yes, clang-format would be great. I have that on my TODO list. Regarding copyright, all fine with me. Regarding API, yes, it is currently C++17 compatible. Would be great to use feature macros to have this feature conditionally available. Regarding Awaitable public c-tor, I don't have a strong opinion, but for consistency with other similar solutions in the library, I would slightly tend towards private c-tor and befriending the relevant classes. It's already done this way in other places in the library. Additionally, I would add Tests LGTM. Clang-tidy job is failing -- shall I address the reported issues, or will you? I'm glad you've enjoyed using the library so far. That was my ultimate design goal, and I've put my best into it. |
|
Hi!
Thanks for taking the time to review and also for the kind words!
Of course, feel free to push the patches you have prepared. std::variant
makes sense as well, i think I didnt use it just based on personal taste.
I'm currently in vacation with no access to my setup (replying to this via
email on my phone), so it will take a couple weeks before I'm able to put
some work into it, however i can work on the other points (c++17 compat,
the privatr ctor + friends, the assertions and the linter issues) when I'm
back if you prefer. Just let me know!
Also i was thinking if we need some tests for making sure the feature is
properly toggled via the feature macros on different compilers.
Kind regards,
Alex
Em qua., 25 de mar. de 2026, 8:51 PM, Stanislav Angelovič <
***@***.***> escreveu:
… *sangelovic* left a comment (Kistler-Group/sdbus-cpp#527)
<#527?email_source=notifications&email_token=AHTVLZ53XWBIHQXYDTNZPJL4SRWJFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJTGA2TQNJZGI2KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4130585924>
@alexcani <https://github.com/alexcani>
Hi! I reviewed the PR. Appears to me to be one of the best, well-done PRs
I've reviewed on this project. It is enjoyable to see changes done to nice
little details and across all levels. I have no experience with coroutines,
so I had to do the research first. Still, given lack of experience, I don't
feel 100% confident about all the details of good coroutines design. But
given the quality of your work, I think I can trust you.
However, I did some deeper reflection about things like overall design,
thread coordination, exception handling, etc. -- and when I tried to think
about them out-of-the-box, I got back to your solution, understanding your
motives. Again, I see you've put an aspect of quality and craftsmanship to
your work, and this is something I highly appreciate.
One thing I was asking myself was -- in AwaitableData template, could we
simply use std::variant> for both the result and the exception, instead
of std::optional for one and the other? I think that
- would idiomatically be more self-revealing that only one of those is
available at a time, would transitively would eliminate potential confusion
of a reader about combinations resulting from having two std::optionals
(like can it be that both are set at the same time? Or can it be that both
are unset?), and
- would need only one member instead of always two, making it simpler,
- with no extra performance penalty.
I've already prepared a local patch for that. It works nicely. I can push
that to your branch if you're okay with that for you to review. Let me
know. In the code I touched, I also changed the formatting a bit to stay
consistent with the surrounding code.
Regarding code style, yes, clang-format would be great. I have that on my
TODO list.
Regarding copyright, all fine with me.
Regarding API, yes, it is currently C++17 compatible. Would be great to
use feature macros to have this feature conditionally available.
Regarding Awaitable public c-tor, I don't have a strong opinion, but for
consistency with other similar solutions in the library, I would slightly
tend towards private c-tor and befriending the relevant classes. It's
already done this way in other places in the library. Additionally, I would
add assert(data_ != nullptr); in the c-tor body to express the invariant
of always non-null ptr clearly.
Tests LGTM.
Clang-tidy job is failing -- shall I address the reported issues, or will
you?
I'm glad you've enjoyed using the library so far. That's was my ultimate
design goal, and I've put my best into it.
—
Reply to this email directly, view it on GitHub
<#527?email_source=notifications&email_token=AHTVLZ53XWBIHQXYDTNZPJL4SRWJFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJTGA2TQNJZGI2KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4130585924>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHTVLZZ3T3GNCLK7WP6TW5T4SRWJFAVCNFSM6AAAAACWDPIINKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCMZQGU4DKOJSGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
OK, I'll commit my patch. And no problem -- if I find time then I may do the adjustments myself. If not, you can do it when you are back. Enjoy your vacation. |
This is to make the public API buildable also under C++17.
sangelovic
force-pushed
the
alexcani/awaitables
branch
from
db11ba5 to
eef7d0f
Compare
|
Hi @alexcani. Today I found some time so I pushed the C++17 compat was not that easy. I tried to conditionally remove all functions using awaitables, but we have virtual methods and there we get into troubles if the library is built with C++20 and the client with C++17 (broken virtual table contract). So I kept the API even when
Does it look OK to you? With new major release v3 in the future, we may remove that conditional build and completely switch to C++20 or C++23 in public API as well. |
Hi! Thank you for the work, I like the std::variant better as well! I did some thinkering with out AI overlord and came up with a solution by keeping the layout the same via a type-erased handle (a.k.a. a pointer) in diff --git a/include/sdbus-c++/Awaitable.h b/include/sdbus-c++/Awaitable.h
index 2a9e638..e42742b 100644
--- a/include/sdbus-c++/Awaitable.h
+++ b/include/sdbus-c++/Awaitable.h
@@ -69,15 +69,14 @@ namespace sdbus {
{
using result_type = std::conditional_t<std::is_void_v<T>, std::monostate, T>;
std::variant<result_type, std::exception_ptr> result;
-#ifdef __cpp_lib_coroutine
- std::coroutine_handle<> handle;
-#endif // __cpp_lib_coroutine
+ void *erasedCoroutineHandle = nullptr;
std::atomic<AwaitableState> status{AwaitableState::NotReady};
void resumeCoroutine()
{
#ifdef __cpp_lib_coroutine
- handle.resume();
+ if (erasedCoroutineHandle)
+ std::coroutine_handle<>::from_address(erasedCoroutineHandle).resume();
#endif // __cpp_lib_coroutine
}
};
@@ -118,7 +117,7 @@ namespace sdbus {
// resume the coroutine.
bool await_suspend(std::coroutine_handle<> handle) noexcept
{
- data_->handle = handle;
+ data_->erasedCoroutineHandle = handle.address();
// Attempt transition from NotReady to Waiting.
AwaitableState expected = AwaitableState::NotReady;The coroutine handles are trivially copyiable and can be easily erased and recovered via What do you think? |
|
@alexcani Hi, thanks for the review! I looked at your described case and this is what I see: The original idea of mine is that But still, if a C++17-based client would call the
Does this make sense to you, or perhaps I am missing something? |
|
Hi! I agree that within reasonable use it is unlikely that a C++17 client will interact with a C++20 compiled I don't want to enter the world of "what-if's", but it's relatively easy for the client to generate code that would trigger UB: // Get proxy from the library
std::unique_ptr<sdbus::IProxy> proxy = /* ... */;
// Call virtual method -> lib method is called with AwaitableData containing handle
auto aw = proxy->callMethodAsync(/*...*/, sdbus::with_awaitable);
// Since Awaitable contains 1 member, its address coincides with the member's
auto* sharedPtr = reinterpret_cast<std::shared_ptr<sdbus::AwaitableData<MethodReply>>*>(&aw);
// Memory stomping, since 'status' is in a different offset than the one seen from the client
(*sharedPtr)->status.exchange(sdbus::AwaitableState::Completed, std::memory_order_acq_rel);Of course the client is shooting itself in the foot (and reinterpreting the private layout of Awaitable is already UB itself), and the code is a bit hacky and makes no practical sense, but the point is that the ABI break exists, we just lack a convenient access path to it at the moment. It's not a deal-breaker for me as well if you prefer to keep it as-is, as well, but I think it would be worth it. |
|
In general, I would be strongly reluctant to make the code more complex for the hypothetical edge case of someone anyway determined to shoot his foot (where is the boundary of sane vs. totally safe API design then, right?), but what you propose seems acceptable -- the code is not more complex, it's "just" a bit less type-safe and less expressive. I would have yet another idea -- simply make the coroutine_handle the last member of |
Have the coroutine handle be the last member in AwaitableData to keep the class layout compatible between C++ 17 and C++20 or later clients.
Hi! I think that's a good middle ground. I have pushed the change moving the handle last in the class. |
OK, great, I think we are good to go. Thank you for your contribution! Very appreciated. |
C++ 20 introduced coroutine support to the language, enabling writing asynchronous code using the co_await, co_return and co_yield operators. Similar to other languages, coroutines allow functions to suspend execution at certain points and later resume from where they left off, without blocking the calling thread. Coroutines are a natural fit for any sort of IO-bound asynchronous operations, such as D-Bus communication. This PR introduces native coroutine support in sdbus-c++ by the means of the Awaitable<T> class, a type that implements the C++20 awaitable protocol, allowing D-Bus method calls to be awaited in coroutines. Summary of the changes: Core library: * New Awaitable<T> type that can be co_awaited, which suspends a running coroutine. When the result or error of a method call arrives, the coroutine is resumed and the result/error is returned. Implementation is done in a thread-safe way, meaning there are no race conditions between the awaitable object returned by asynchronous calls and the callback invoked by the event loop upon arrival of a message. Naturally, it works in the single-threaded scenario as well, where the connection event loop may be driven externally and/or integrated in a full fledged coroutine runtime. * Low-level API: new callMethodAsync() overloads accepting with_awaitable_t tag and returning Awaitable<MethodReply>. * High-level API: new getResultAsAwaitable methods in the relevant high-level API helpers, covering methods and properties. StandardInterfaces.h classes have also been updated to expose awaitable-based methods. * Integration tests for both low-level and high-level APIs. Codegen: * Added support for generating awaitable-based async methods in the xml2cpp tool through new values for the org.freedesktop.DBus.Method.Async.ClientImpl and org.freedesktop.DBus.Property.[Get/Set].Async.ClientImpl annotations. --------- Co-authored-by: Stanislav Angelovič <stanislav.angelovic@protonmail.com>
2 participants
C++ 20 introduced coroutine support to the language, enabling writing asynchronous code using the
co_await,co_returnandco_yieldoperators. Similar to other languages, coroutines allow functions to suspend execution at certain points and later resume from where they left off, without blocking the calling thread.Coroutines are a natural fit for any sort of IO-bound asynchronous operations, such as D-Bus communication.
This PR introduces native coroutine support in sdbus-c++ by the means of the
Awaitable<T>class, a type that implements the C++20 awaitable protocol, allowing D-Bus method calls to be awaited in coroutines.Summary of the changes:
Core library
Awaitable<T>type that can beco_awaited, which suspends a running coroutine. When the result or error of a method call arrives, the coroutine is resumed and the result/error is returned.Implementation is done in a thread-safe way, meaning there are no race conditions between the awaitable object returned by asynchronous calls and the callback invoked by the event loop upon arrival of a message. Naturally, it works in the single-threaded scenario as well, where the connection event loop may be driven externally and/or integrated in a full fledged coroutine runtime.
callMethodAsync()overloads acceptingwith_awaitable_ttag and returningAwaitable<MethodReply>.getResultAsAwaitablemethods in the relevant high-level API helpers, covering methods and properties. StandardInterfaces.h classes have also been updated to expose awaitable-based methods.Codegen
org.freedesktop.DBus.Method.Async.ClientImplandorg.freedesktop.DBus.Property.[Get/Set].Async.ClientImplannotations.