From b66716d27854db330eb0201fbcba0264a03d6055 Mon Sep 17 00:00:00 2001 From: Sebastian Sumpf Date: Tue, 14 Feb 2017 17:00:53 +0100 Subject: [PATCH] base: entrypoint 'wait_and_dispatch_one_signal' There existed a race when 'wait_and_dispatch_one_signal' is called form a RPC context, because the 'signal_proxy' or 'main' will block and the signal semaphore, when the EP then calls 'wait_and_dispatch_one_signal', the signal proxy is woken up ands sends an RPC to the EP, leading to a dead lock if no further signal arrive, because the EP will then remain blocked in the signal semaphore. Therefore, for this case, the signal proxy will now perform a semaphore up operation and does not perform an RPC if the EP is within 'wait_and_dispatch_one_signal'. --- repos/base-hw/src/lib/base/signal.cc | 9 +++- repos/base/include/base/entrypoint.h | 14 +++--- repos/base/include/base/signal.h | 6 +++ repos/base/lib/symbols/ld | 1 + repos/base/src/lib/base/entrypoint.cc | 64 ++++++++++++++++++++++----- repos/base/src/lib/base/signal.cc | 6 +++ 6 files changed, 79 insertions(+), 21 deletions(-) diff --git a/repos/base-hw/src/lib/base/signal.cc b/repos/base-hw/src/lib/base/signal.cc index 4e81d365f9..e72bab88d2 100644 --- a/repos/base-hw/src/lib/base/signal.cc +++ b/repos/base-hw/src/lib/base/signal.cc @@ -114,7 +114,7 @@ void Signal_receiver::block_for_signal() { /* wait for a signal */ if (Kernel::await_signal(Capability_space::capid(_cap))) { - Genode::error("failed to receive signal"); + /* canceled */ return; } /* read signal data */ @@ -133,4 +133,11 @@ void Signal_receiver::block_for_signal() } +void Signal_receiver::unblock_signal_waiter(Rpc_entrypoint &rpc_ep) +{ + /* force ep out of 'await_signal' in 'block_for_signal' */ + rpc_ep.cancel_blocking(); +} + + void Signal_receiver::local_submit(Signal::Data) { Genode::error("not implemented"); } diff --git a/repos/base/include/base/entrypoint.h b/repos/base/include/base/entrypoint.h index 2c47f08451..4b4e351bcf 100644 --- a/repos/base/include/base/entrypoint.h +++ b/repos/base/include/base/entrypoint.h @@ -89,6 +89,11 @@ class Genode::Entrypoint : Genode::Noncopyable void (*_suspended_callback) () = nullptr; void (*_resumed_callback) () = nullptr; + enum Signal_recipient { + NONE = 0, ENTRYPOINT = 1, SIGNAL_PROXY = 2 + }; + + int _signal_recipient { NONE }; Post_signal_hook *_post_signal_hook = nullptr; void _execute_post_signal_hook() @@ -164,14 +169,7 @@ class Genode::Entrypoint : Genode::Noncopyable * receiver belongs to the calling entrypoint. Alternatively, * remove it. */ - void wait_and_dispatch_one_signal() - { - { - Signal sig = _sig_rec->wait_for_signal(); - _dispatch_signal(sig); - } - _execute_post_signal_hook(); - } + void wait_and_dispatch_one_signal(); /** * Return RPC entrypoint diff --git a/repos/base/include/base/signal.h b/repos/base/include/base/signal.h index 8501651181..3b563704ea 100644 --- a/repos/base/include/base/signal.h +++ b/repos/base/include/base/signal.h @@ -28,6 +28,7 @@ namespace Kernel { struct Signal_receiver; } namespace Genode { class Entrypoint; + class Rpc_entrypoint; class Signal_source; class Signal_receiver; @@ -264,6 +265,11 @@ class Genode::Signal_receiver : Noncopyable */ void block_for_signal(); + /** + * Unblock signal waiter + */ + void unblock_signal_waiter(Rpc_entrypoint &rpc_ep); + /** * Retrieve pending signal * diff --git a/repos/base/lib/symbols/ld b/repos/base/lib/symbols/ld index 26565e6283..0344baca31 100644 --- a/repos/base/lib/symbols/ld +++ b/repos/base/lib/symbols/ld @@ -49,6 +49,7 @@ _Z22__ldso_raise_exceptionv T _ZN6Genode10Entrypoint16_dispatch_signalERNS_6SignalE T _ZN6Genode10Entrypoint16schedule_suspendEPFvvES2_ T _ZN6Genode10Entrypoint25_process_incoming_signalsEv T +_ZN6Genode10Entrypoint28wait_and_dispatch_one_signalEv T _ZN6Genode10Entrypoint6manageERNS_22Signal_dispatcher_baseE T _ZN6Genode10Entrypoint8dissolveERNS_22Signal_dispatcher_baseE T _ZN6Genode10EntrypointC1ERNS_3EnvE T diff --git a/repos/base/src/lib/base/entrypoint.cc b/repos/base/src/lib/base/entrypoint.cc index b84dce1e6e..31234866fe 100644 --- a/repos/base/src/lib/base/entrypoint.cc +++ b/repos/base/src/lib/base/entrypoint.cc @@ -16,6 +16,8 @@ #include #include +#include + #define INCLUDED_BY_ENTRYPOINT_CC /* prevent "deprecated" warning */ #include #undef INCLUDED_BY_ENTRYPOINT_CC @@ -63,17 +65,28 @@ void Entrypoint::_process_incoming_signals() do { _sig_rec->block_for_signal(); - /* - * It might happen that we try to forward a signal to the - * entrypoint, while the context of that signal is already - * destroyed. In that case we will get an ipc error exception - * as result, which has to be caught. - */ - retry( - [&] () { _signal_proxy_cap.call(); }, - [] () { warning("blocking canceled during signal processing"); } - ); + int success = cmpxchg(&_signal_recipient, NONE, SIGNAL_PROXY); + /* common case, entrypoint is not in 'wait_and_dispatch_one_signal' */ + if (success) { + /* + * It might happen that we try to forward a signal to the + * entrypoint, while the context of that signal is already + * destroyed. In that case we will get an ipc error exception + * as result, which has to be caught. + */ + retry( + [&] () { _signal_proxy_cap.call(); }, + [] () { warning("blocking canceled during signal processing"); }); + + cmpxchg(&_signal_recipient, SIGNAL_PROXY, NONE); + } else { + /* + * Entrypoint is in 'wait_and_dispatch_one_signal', wakup it up and + * block for next signal + */ + _sig_rec->unblock_signal_waiter(*_rpc_ep); + } } while (!_suspended); _suspend_dispatcher.destruct(); @@ -107,6 +120,33 @@ void Entrypoint::_process_incoming_signals() } +void Entrypoint::wait_and_dispatch_one_signal() +{ + for (;;) { + + try { + + { + cmpxchg(&_signal_recipient, NONE, ENTRYPOINT); + + Signal sig =_sig_rec->pending_signal(); + + cmpxchg(&_signal_recipient, ENTRYPOINT, NONE); + + _dispatch_signal(sig); + } + + _execute_post_signal_hook(); + + return; + + } catch (Signal_receiver::Signal_not_pending) { + _sig_rec->block_for_signal(); + } + } +} + + void Entrypoint::schedule_suspend(void (*suspended)(), void (*resumed)()) { _suspended_callback = suspended; @@ -119,7 +159,7 @@ void Entrypoint::schedule_suspend(void (*suspended)(), void (*resumed)()) _suspend_dispatcher.construct(*this, *this, &Entrypoint::_handle_suspend); /* trigger wakeup of the signal-dispatch loop for suspend */ - Genode::Signal_transmitter(*_suspend_dispatcher).submit(); + Signal_transmitter(*_suspend_dispatcher).submit(); } @@ -186,7 +226,7 @@ Entrypoint::Entrypoint(Env &env) try { constructor_cap.call(); - } catch (Genode::Blocking_canceled) { + } catch (Blocking_canceled) { warning("blocking canceled in entrypoint constructor"); } diff --git a/repos/base/src/lib/base/signal.cc b/repos/base/src/lib/base/signal.cc index 45201f8057..9a87a925de 100644 --- a/repos/base/src/lib/base/signal.cc +++ b/repos/base/src/lib/base/signal.cc @@ -255,6 +255,12 @@ void Signal_receiver::block_for_signal() } +void Signal_receiver::unblock_signal_waiter(Rpc_entrypoint &) +{ + _signal_available.up(); +} + + void Signal_receiver::local_submit(Signal::Data ns) { Signal_context *context = ns.context;