Message ID | 20250130115001.1129305-8-pobrn@protonmail.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Hi Barnabás On Thu, Jan 30, 2025 at 11:50:36AM +0000, Barnabás Pőcze wrote: > Instead of taking a single integer in `EventLoop:exit()`, change it > so that an `std::exception_ptr` can also be passed. And in `EventLoop::exec()` > rethrow the stored exception (if any). > > Furthermore, catch exceptions from deferred calls, and stop the event > loop with the caught exception. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > src/apps/common/event_loop.cpp | 35 +++++++++++++++++++++++++++++----- > src/apps/common/event_loop.h | 6 ++++-- > 2 files changed, 34 insertions(+), 7 deletions(-) > > diff --git a/src/apps/common/event_loop.cpp b/src/apps/common/event_loop.cpp > index 0d7a4a024..13c2d6057 100644 > --- a/src/apps/common/event_loop.cpp > +++ b/src/apps/common/event_loop.cpp > @@ -12,6 +12,7 @@ > #include <event2/thread.h> > #include <algorithm> > #include <iostream> > +#include <utility> > > EventLoop *EventLoop::instance_ = nullptr; > > @@ -38,7 +39,14 @@ EventLoop::EventLoop() > self->calls_.pop_front(); > } > > - call(); > + try { > + call(); > + } > + catch (...) { we don't have a preferred syntax for try {} catch () {} but I feel like it should probably be try { } catch () { } to match how we handle if() branches ? > + ::event_active(self->callsTrigger_, 0, 0); > + self->exit(std::current_exception()); > + break; > + } > } > }, this); > assert(callsTrigger_); > @@ -63,14 +71,31 @@ EventLoop *EventLoop::instance() > > int EventLoop::exec() > { > - exitCode_ = -1; > + { > + std::lock_guard locker(lock_); > + result_ = 0; > + } > + > event_base_loop(base_, EVLOOP_NO_EXIT_ON_EMPTY); > - return exitCode_; > + > + auto result = [&] { > + std::lock_guard locker(lock_); > + return std::exchange(result_, 0); > + }(); > + > + if (auto *exc = std::get_if<std::exception_ptr>(&result)) > + std::rethrow_exception(std::move(*exc)); > + > + return std::get<int>(result); All of this is morbidly fascinating as only C++ can be. I understand this whole patch is only needed to support the following hunk in 19/21 - if (queueRequest(request)) - loop_->exit(-EINVAL); + queueRequest(request); iow to remove the need for explicit loop interruptions but to propagate the exceptions thrown by gtest ASSERT_EQ() and friends in the even loop and exit there. All of this is neat but I wonder if the complexity of this patch is necessary just to support that single use case. Maybe it's complex for me but not for others, I'll ask their opinion. > } > > -void EventLoop::exit(int code) > +void EventLoop::exit(std::variant<int, std::exception_ptr> result) > { > - exitCode_ = code; > + { > + std::lock_guard locker(lock_); > + result_ = std::move(result); > + } > + > event_base_loopbreak(base_); > } > > diff --git a/src/apps/common/event_loop.h b/src/apps/common/event_loop.h > index 6d7d0497a..380c3483a 100644 > --- a/src/apps/common/event_loop.h > +++ b/src/apps/common/event_loop.h > @@ -9,11 +9,13 @@ > > #include <chrono> > #include <deque> > +#include <exception> > #include <functional> > #include <list> > #include <memory> > #include <mutex> > #include <optional> > +#include <variant> > > #include <libcamera/base/class.h> > > @@ -35,7 +37,7 @@ public: > static EventLoop *instance(); > > int exec(); > - void exit(int code = 0); > + void exit(std::variant<int, std::exception_ptr> result = 0); > > void callLater(std::function<void()> &&func, std::optional<std::uintptr_t> cookie = {}); > void cancelLater(std::optional<std::uintptr_t> cookie = {}); > @@ -63,7 +65,7 @@ private: > static EventLoop *instance_; > > struct event_base *base_; > - int exitCode_; > + std::variant<int, std::exception_ptr> result_ = 0; > > std::deque<std::pair<std::function<void()>, std::optional<std::uintptr_t>>> calls_; > ::event *callsTrigger_ = nullptr; > -- > 2.48.1 > >
diff --git a/src/apps/common/event_loop.cpp b/src/apps/common/event_loop.cpp index 0d7a4a024..13c2d6057 100644 --- a/src/apps/common/event_loop.cpp +++ b/src/apps/common/event_loop.cpp @@ -12,6 +12,7 @@ #include <event2/thread.h> #include <algorithm> #include <iostream> +#include <utility> EventLoop *EventLoop::instance_ = nullptr; @@ -38,7 +39,14 @@ EventLoop::EventLoop() self->calls_.pop_front(); } - call(); + try { + call(); + } + catch (...) { + ::event_active(self->callsTrigger_, 0, 0); + self->exit(std::current_exception()); + break; + } } }, this); assert(callsTrigger_); @@ -63,14 +71,31 @@ EventLoop *EventLoop::instance() int EventLoop::exec() { - exitCode_ = -1; + { + std::lock_guard locker(lock_); + result_ = 0; + } + event_base_loop(base_, EVLOOP_NO_EXIT_ON_EMPTY); - return exitCode_; + + auto result = [&] { + std::lock_guard locker(lock_); + return std::exchange(result_, 0); + }(); + + if (auto *exc = std::get_if<std::exception_ptr>(&result)) + std::rethrow_exception(std::move(*exc)); + + return std::get<int>(result); } -void EventLoop::exit(int code) +void EventLoop::exit(std::variant<int, std::exception_ptr> result) { - exitCode_ = code; + { + std::lock_guard locker(lock_); + result_ = std::move(result); + } + event_base_loopbreak(base_); } diff --git a/src/apps/common/event_loop.h b/src/apps/common/event_loop.h index 6d7d0497a..380c3483a 100644 --- a/src/apps/common/event_loop.h +++ b/src/apps/common/event_loop.h @@ -9,11 +9,13 @@ #include <chrono> #include <deque> +#include <exception> #include <functional> #include <list> #include <memory> #include <mutex> #include <optional> +#include <variant> #include <libcamera/base/class.h> @@ -35,7 +37,7 @@ public: static EventLoop *instance(); int exec(); - void exit(int code = 0); + void exit(std::variant<int, std::exception_ptr> result = 0); void callLater(std::function<void()> &&func, std::optional<std::uintptr_t> cookie = {}); void cancelLater(std::optional<std::uintptr_t> cookie = {}); @@ -63,7 +65,7 @@ private: static EventLoop *instance_; struct event_base *base_; - int exitCode_; + std::variant<int, std::exception_ptr> result_ = 0; std::deque<std::pair<std::function<void()>, std::optional<std::uintptr_t>>> calls_; ::event *callsTrigger_ = nullptr;
Instead of taking a single integer in `EventLoop:exit()`, change it so that an `std::exception_ptr` can also be passed. And in `EventLoop::exec()` rethrow the stored exception (if any). Furthermore, catch exceptions from deferred calls, and stop the event loop with the caught exception. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- src/apps/common/event_loop.cpp | 35 +++++++++++++++++++++++++++++----- src/apps/common/event_loop.h | 6 ++++-- 2 files changed, 34 insertions(+), 7 deletions(-)