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

@@ -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"); }

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

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
 		 *

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

repos/base/src/lib/base/entrypoint.cc

@@ -16,6 +16,8 @@
 #include <base/entrypoint.h>
 #include <base/component.h>
 
+#include <cpu/atomic.h>
+
 #define INCLUDED_BY_ENTRYPOINT_CC  /* prevent "deprecated" warning */
 #include <cap_session/connection.h>
 #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<Genode::Blocking_canceled>(
-				[&] () { _signal_proxy_cap.call<Signal_proxy::Rpc_signal>(); },
-				[]  () { 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<Blocking_canceled>(
+					[&] () { _signal_proxy_cap.call<Signal_proxy::Rpc_signal>(); },
+					[]  () { 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<Constructor::Rpc_construct>();
-	} catch (Genode::Blocking_canceled) {
+	} catch (Blocking_canceled) {
 		warning("blocking canceled in entrypoint constructor");
 	}
 

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;