base: fix deadlock during signal-context dissolve

This patch moves the removal of the signal context from the
'_platform_finish_dissolve' to the '_platform_begin_dissolve'
method. This is needed because the removal involves taking
the signal-registry lock. The latter must adhere the same
locking order as the code path used for signal delivery.

Fixes #3109

repos/base-hw/src/lib/base/signal_receiver.cc

@@ -82,8 +82,11 @@ void Signal_receiver::_platform_begin_dissolve(Signal_context * const c)
 	 * from taking the lock, and set an invalid context to prevent further
 	 * processing
 	 */
+	{
+		Lock::Guard context_guard(c->_lock);
 		c->_pending     = true;
 		c->_curr_signal = Signal::Data(nullptr, 0);
+	}
 	Kernel::kill_signal_context(Capability_space::capid(c->_cap));
 }
 

repos/base/src/lib/base/signal.cc

@@ -318,11 +318,18 @@ void Signal_receiver::dispatch_signals(Signal_source *signal_source)
 }
 
 
-void Signal_receiver::_platform_begin_dissolve(Signal_context *) { }
+void Signal_receiver::_platform_begin_dissolve(Signal_context *context)
+{
+	/*
+	 * Because the 'remove' operation takes the registry lock, the context
+	 * must not be locked when calling this method. See the comment in
+	 * 'Signal_receiver::dissolve'.
+	 */
+	signal_context_registry()->remove(&context->_registry_le);
+}
 
 
-void Signal_receiver::_platform_finish_dissolve(Signal_context * const c) {
+void Signal_receiver::_platform_finish_dissolve(Signal_context *) { }
-	signal_context_registry()->remove(&c->_registry_le); }
 
 
 void Signal_receiver::_platform_destructor() { }

repos/base/src/lib/base/signal_common.cc

@@ -200,8 +200,11 @@ Signal_receiver::~Signal_receiver()
 	Lock::Guard contexts_lock_guard(_contexts_lock);
 
 	/* disassociate contexts from the receiver */
-	while (_contexts.head())
+	while (Signal_context *context = _contexts.head()) {
-		_unsynchronized_dissolve(_contexts.head());
+		_platform_begin_dissolve(context);
+		_unsynchronized_dissolve(context);
+		_platform_finish_dissolve(context);
+	}
 
 	_platform_destructor();
 }
@@ -209,8 +212,6 @@ Signal_receiver::~Signal_receiver()
 
 void Signal_receiver::_unsynchronized_dissolve(Signal_context * const context)
 {
-	_platform_begin_dissolve(context);
-
 	/* tell core to stop sending signals referring to the context */
 	env_deprecated()->pd_session()->free_context(context->_cap);
 
@@ -220,8 +221,6 @@ void Signal_receiver::_unsynchronized_dissolve(Signal_context * const context)
 
 	/* remove context from context list */
 	_contexts.remove(context);
-
-	_platform_finish_dissolve(context);
 }
 
 
@@ -231,13 +230,26 @@ void Signal_receiver::dissolve(Signal_context *context)
 		throw Context_not_associated();
 
 	{
+		/*
+		 * We must adhere to the following lock-taking order:
+		 *
+		 * 1. Taking the lock for the list of contexts ('_contexts_lock')
+		 * 2. Taking the context-registry lock (this happens inside
+		 *    '_platform_begin_dissolve' on platforms that use such a
+		 *    registry)
+		 * 3. Taking the lock for an individual signal context
+		 */
 		Lock::Guard contexts_lock_guard(_contexts_lock);
 
+		_platform_begin_dissolve(context);
+
 		Lock::Guard context_lock_guard(context->_lock);
 
 		_unsynchronized_dissolve(context);
 	}
 
+	_platform_finish_dissolve(context);
+
 	Lock::Guard context_destroy_lock_guard(context->_destroy_lock);
 }