From 50e0f3b977c06fccfc90aac6dc2fe6097b90d3b6 Mon Sep 17 00:00:00 2001 From: Christian Prochaska Date: Fri, 16 Oct 2020 18:56:43 +0200 Subject: [PATCH] base: don't throw exceptions in 'Signal_receiver::pending_signal()' Issue #3922 --- repos/base-hw/src/lib/base/signal_receiver.cc | 6 +- repos/base/include/base/signal.h | 18 +++-- repos/base/src/core/signal_receiver.cc | 2 +- repos/base/src/lib/base/entrypoint.cc | 74 ++++++++++--------- repos/base/src/lib/base/signal.cc | 10 +-- repos/base/src/lib/base/signal_common.cc | 7 +- repos/ports/src/virtualbox5/thread.cc | 2 - 7 files changed, 61 insertions(+), 58 deletions(-) diff --git a/repos/base-hw/src/lib/base/signal_receiver.cc b/repos/base-hw/src/lib/base/signal_receiver.cc index 2542406714..fd38e0bb2a 100644 --- a/repos/base-hw/src/lib/base/signal_receiver.cc +++ b/repos/base-hw/src/lib/base/signal_receiver.cc @@ -158,7 +158,7 @@ Signal Signal_receiver::pending_signal() Signal::Data result; _contexts.for_each_locked([&] (Signal_context &context) { - if (!context._pending) return; + if (!context._pending) return false; _contexts.head(context._next); context._pending = false; @@ -166,7 +166,7 @@ Signal Signal_receiver::pending_signal() context._curr_signal = Signal::Data(0, 0); Trace::Signal_received trace_event(context, result.num); - throw Context_ring::Break_for_each(); + return true; }); if (result.context) { Mutex::Guard context_guard(result.context->_mutex); @@ -178,7 +178,7 @@ Signal Signal_receiver::pending_signal() /* look for pending signals */ if (Kernel::pending_signal(Capability_space::capid(_cap)) != 0) { - throw Signal_not_pending(); + return Signal(); } /* read signal data */ diff --git a/repos/base/include/base/signal.h b/repos/base/include/base/signal.h index 097851ea6f..11e7d88140 100644 --- a/repos/base/include/base/signal.h +++ b/repos/base/include/base/signal.h @@ -88,7 +88,12 @@ class Genode::Signal */ Data() : context(0), num(0) { } - } _data; + } _data { }; + + /** + * Constructor for invalid signal + */ + Signal() { }; /** * Constructor @@ -117,6 +122,7 @@ class Genode::Signal Signal_context *context() { return _data.context; } unsigned num() const { return _data.num; } + bool valid() const { return _data.context != nullptr; } }; @@ -299,9 +305,7 @@ class Genode::Signal_receiver : Noncopyable do { Mutex::Guard mutex_guard(context->_mutex); - try { - functor(*context); - } catch (Break_for_each) { return; } + if (functor(*context)) return; context = context->_next; } while (context != _head); } @@ -352,7 +356,6 @@ class Genode::Signal_receiver : Noncopyable */ class Context_already_in_use { }; class Context_not_associated { }; - class Signal_not_pending { }; /** * Constructor @@ -401,10 +404,9 @@ class Genode::Signal_receiver : Noncopyable void unblock_signal_waiter(Rpc_entrypoint &rpc_ep); /** - * Retrieve pending signal + * Retrieve pending signal * - * \throw 'Signal_not_pending' no pending signal found - * \return received signal + * \return received signal (invalid if no pending signal found) */ Signal pending_signal(); diff --git a/repos/base/src/core/signal_receiver.cc b/repos/base/src/core/signal_receiver.cc index d26f9611f1..1fc0d2285b 100644 --- a/repos/base/src/core/signal_receiver.cc +++ b/repos/base/src/core/signal_receiver.cc @@ -50,6 +50,6 @@ void Signal_receiver::block_for_signal() } Signal Signal_receiver::pending_signal() { - throw Signal_not_pending(); } + return Signal(); } void Signal_receiver::local_submit(Signal::Data) { ASSERT_NEVER_CALLED; } diff --git a/repos/base/src/lib/base/entrypoint.cc b/repos/base/src/lib/base/entrypoint.cc index 775887770b..827f304009 100644 --- a/repos/base/src/lib/base/entrypoint.cc +++ b/repos/base/src/lib/base/entrypoint.cc @@ -55,15 +55,17 @@ void Entrypoint::Signal_proxy_component::signal() * Note, we handle only one signal here to ensure fairness between RPCs and * signals. */ - try { - Signal sig = ep._sig_rec->pending_signal(); + + Signal sig = ep._sig_rec->pending_signal(); + + if (sig.valid()) { ep._dispatch_signal(sig); if (sig.context()->level() == Signal_context::Level::Io) { /* trigger the progress handler */ io_progress = true; } - } catch (Signal_receiver::Signal_not_pending) { } + } if (io_progress) ep._handle_io_progress(); @@ -185,8 +187,9 @@ bool Entrypoint::_wait_and_dispatch_one_io_signal(bool const dont_block) for (;;) { - try { - Signal sig =_sig_rec->pending_signal(); + Signal sig = _sig_rec->pending_signal(); + + if (sig.valid()) { /* defer application-level signals */ if (sig.context()->level() == Signal_context::Level::App) { @@ -196,40 +199,39 @@ bool Entrypoint::_wait_and_dispatch_one_io_signal(bool const dont_block) _dispatch_signal(sig); break; + } - } catch (Signal_receiver::Signal_not_pending) { - if (dont_block) - return false; + if (dont_block) + return false; - { - /* - * The signal-proxy thread as well as the entrypoint via - * 'wait_and_dispatch_one_io_signal' never call - * 'block_for_signal()' without the '_block_for_signal_mutex' - * aquired. The signal-proxy thread also flags when it was - * unblocked by an incoming signal and delivers the signal via - * RPC in '_signal_proxy_delivers_signal'. - */ - Mutex::Guard guard { _block_for_signal_mutex }; + { + /* + * The signal-proxy thread as well as the entrypoint via + * 'wait_and_dispatch_one_io_signal' never call + * 'block_for_signal()' without the '_block_for_signal_mutex' + * aquired. The signal-proxy thread also flags when it was + * unblocked by an incoming signal and delivers the signal via + * RPC in '_signal_proxy_delivers_signal'. + */ + Mutex::Guard guard { _block_for_signal_mutex }; - /* - * If the signal proxy is blocked in the signal-delivery - * RPC but the call did not yet arrive in the entrypoint - * (_signal_proxy_delivers_signal == true), we acknowledge the - * delivery here (like in 'Signal_proxy_component::signal()') and - * retry to fetch one pending signal at the beginning of the - * loop above. Otherwise, we block for the next incoming - * signal. - * - * There exist cases were we already processed the signal - * flagged in '_signal_proxy_delivers_signal' and will end - * up here again. In these cases we also 'block_for_signal()'. - */ - if (_signal_proxy_delivers_signal) - _signal_proxy_delivers_signal = false; - else - _sig_rec->block_for_signal(); - } + /* + * If the signal proxy is blocked in the signal-delivery + * RPC but the call did not yet arrive in the entrypoint + * (_signal_proxy_delivers_signal == true), we acknowledge the + * delivery here (like in 'Signal_proxy_component::signal()') and + * retry to fetch one pending signal at the beginning of the + * loop above. Otherwise, we block for the next incoming + * signal. + * + * There exist cases were we already processed the signal + * flagged in '_signal_proxy_delivers_signal' and will end + * up here again. In these cases we also 'block_for_signal()'. + */ + if (_signal_proxy_delivers_signal) + _signal_proxy_delivers_signal = false; + else + _sig_rec->block_for_signal(); } } diff --git a/repos/base/src/lib/base/signal.cc b/repos/base/src/lib/base/signal.cc index e8e4733cd2..e98f0cc0d5 100644 --- a/repos/base/src/lib/base/signal.cc +++ b/repos/base/src/lib/base/signal.cc @@ -235,13 +235,14 @@ void Signal_receiver::block_for_signal() _signal_available.down(); } + Signal Signal_receiver::pending_signal() { Mutex::Guard contexts_guard(_contexts_mutex); Signal::Data result; - _contexts.for_each_locked([&] (Signal_context &context) { + _contexts.for_each_locked([&] (Signal_context &context) -> bool { - if (!context._pending) return; + if (!context._pending) return false; _contexts.head(context._next); context._pending = false; @@ -249,7 +250,7 @@ Signal Signal_receiver::pending_signal() context._curr_signal = Signal::Data(0, 0); Trace::Signal_received trace_event(context, result.num); - throw Context_ring::Break_for_each(); + return true; }); if (result.context) { Mutex::Guard context_guard(result.context->_mutex); @@ -268,10 +269,9 @@ Signal Signal_receiver::pending_signal() * signal, we may have increased the semaphore already. In this case * the signal-causing context is absent from the list. */ - throw Signal_not_pending(); + return Signal(); } - void Signal_receiver::unblock_signal_waiter(Rpc_entrypoint &) { _signal_available.up(); diff --git a/repos/base/src/lib/base/signal_common.cc b/repos/base/src/lib/base/signal_common.cc index 8d3c15b915..e1eb8bc1a9 100644 --- a/repos/base/src/lib/base/signal_common.cc +++ b/repos/base/src/lib/base/signal_common.cc @@ -149,9 +149,10 @@ Signal Signal_receiver::wait_for_signal() { for (;;) { - try { - return pending_signal(); - } catch (Signal_not_pending) { } + Signal sig = pending_signal(); + + if (sig.valid()) + return sig; /* block until the receiver has received a signal */ block_for_signal(); diff --git a/repos/ports/src/virtualbox5/thread.cc b/repos/ports/src/virtualbox5/thread.cc index 0cfee3c1f8..358c2a7a89 100644 --- a/repos/ports/src/virtualbox5/thread.cc +++ b/repos/ports/src/virtualbox5/thread.cc @@ -160,8 +160,6 @@ extern "C" int pthread_create(pthread_t *thread, const pthread_attr_t *attr, log("Upgrading memory for creation of " "thread '", Cstring(rtthread->szName), "'"); cpu_connection(rtthread->enmType)->upgrade_ram(4096); - } catch (Genode::Signal_receiver::Signal_not_pending) { - error("signal not pending ?"); } catch (Out_of_caps) { error("out of caps ..."); }