[RFC PATCH v3 07/21] apps: common: event_loop: Make it possible to exit with exception

[Jacopo Mondi]

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 at 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
>
>