async: scope: forward exception correctly
There were test cases to cover an unhandled exception thrown by a
spawned task in the async::context, but I observed some cases where
this exception was silently dropped. The test cases only covered
spawned Senders and not spawned Coroutines. The underlying cause
of this was that in some cases the exception was deleted prior to
it being forwarded along to the async::context's scope.
When a task completes in the async::scope, the operational state
holding the task has to be deleted. There needs to be a careful
handshake to be able to trigger the scope while at the same time
deleting the object which is being executed (the scope_receiver),
so that there is not a memory leak. As part of this, if the task
completed with an unhandled exception, the complete function is
suppose to forward the exception to the scope. It is quite likely
that the exception is actually held by the operational state of the
task, which is going to be deleted prior to triggering the scope, so
we need to save this exception prior to the deletion.
Add a specific test case to cover the throwing coroutine, which
failed prior to this fix.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I7e24529a724c3338b6e155ba0a9b4220e1d2fa50
diff --git a/include/sdbusplus/async/scope.hpp b/include/sdbusplus/async/scope.hpp
index 6fc21f6..cc6c87a 100644
--- a/include/sdbusplus/async/scope.hpp
+++ b/include/sdbusplus/async/scope.hpp
@@ -138,12 +138,14 @@
// Save the scope (since we're going to delete `this`).
auto owning_scope = s;
+ // We also need to save the exception, which is likely owned by `this`.
+ std::exception_ptr ex{std::move(e)};
// Delete the operational state, which triggers deleting this.
delete static_cast<scope_ns::scope_operation_state<Sender>*>(op_state);
// Inform the scope that a task has completed.
- owning_scope->ended_task(std::move(e));
+ owning_scope->ended_task(std::move(ex));
}
// Virtual class to handle the scope completions.
diff --git a/test/async/context.cpp b/test/async/context.cpp
index 7f821ed..7d7f6fa 100644
--- a/test/async/context.cpp
+++ b/test/async/context.cpp
@@ -57,6 +57,22 @@
ctx->run();
}
+TEST_F(Context, SpawnThrowingCoroutine)
+{
+ struct _
+ {
+ static auto one() -> sdbusplus::async::task<>
+ {
+ throw std::logic_error("Oops");
+ co_return;
+ }
+ };
+
+ ctx->spawn(_::one());
+ EXPECT_THROW(runToStop(), std::logic_error);
+ ctx->run();
+};
+
TEST_F(Context, SpawnManyThrowingTasks)
{
static constexpr size_t count = 100;