[libcamera-devel] [PATCH v2 08/13] libcamera: signal: Make connection and disconnection thread-safe

[Laurent Pinchart]

Make the signal connection and disconnection thread-safe, and document
them as such. This is required to make objects connectable from
different threads.

The connect(), disconnect() and emit() methods are now all protected by
a global mutex, which may generate a high lock contention. This could be
improved with finer-grained locks or with a pool of mutexes.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
---
 src/libcamera/signal.cpp | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp
index 9bb7eca8ed6e..6eab1fa74d42 100644
--- a/src/libcamera/signal.cpp
+++ b/src/libcamera/signal.cpp
@@ -7,6 +7,8 @@
 
 #include <libcamera/signal.h>
 
+#include "thread.h"
+
 /**
  * \file signal.h
  * \brief Signal & slot implementation
@@ -14,8 +16,21 @@
 
 namespace libcamera {
 
+namespace {
+
+/*
+ * Mutex to protect the SignalBase::slots_ and Object::signals_ lists. If lock
+ * contention needs to be decreased, this could be replaced with locks in
+ * Object and SignalBase, or with a mutex pool.
+ */
+Mutex signalsLock;
+
+} /* namespace */
+
 void SignalBase::connect(BoundMethodBase *slot)
 {
+	MutexLocker locker(signalsLock);
+
 	Object *object = slot->object();
 	if (object)
 		object->connect(this);
@@ -31,6 +46,8 @@ void SignalBase::disconnect(Object *object)
 
 void SignalBase::disconnect(std::function<bool(SlotList::iterator &)> match)
 {
+	MutexLocker locker(signalsLock);
+
 	for (auto iter = slots_.begin(); iter != slots_.end(); ) {
 		if (match(iter)) {
 			Object *object = (*iter)->object();
@@ -47,6 +64,7 @@ void SignalBase::disconnect(std::function<bool(SlotList::iterator &)> match)
 
 SignalBase::SlotList SignalBase::slots()
 {
+	MutexLocker locker(signalsLock);
 	return slots_;
 }
 
@@ -99,23 +117,31 @@ SignalBase::SlotList SignalBase::slots()
  * disconnected from the \a func slot of \a object when \a object is destroyed.
  * Otherwise the caller shall disconnect signals manually before destroying \a
  * object.
+ *
+ * \context This function is \threadsafe.
  */
 
 /**
  * \fn Signal::connect(R (*func)(Args...))
  * \brief Connect the signal to a static function slot
  * \param[in] func The slot static function
+ *
+ * \context This function is \threadsafe.
  */
 
 /**
  * \fn Signal::disconnect()
  * \brief Disconnect the signal from all slots
+ *
+ * \context This function is \threadsafe.
  */
 
 /**
  * \fn Signal::disconnect(T *object)
  * \brief Disconnect the signal from all slots of the \a object
  * \param[in] object The object pointer whose slots to disconnect
+ *
+ * \context This function is \threadsafe.
  */
 
 /**
@@ -123,12 +149,16 @@ SignalBase::SlotList SignalBase::slots()
  * \brief Disconnect the signal from the \a object slot member function \a func
  * \param[in] object The object pointer whose slots to disconnect
  * \param[in] func The slot member function to disconnect
+ *
+ * \context This function is \threadsafe.
  */
 
 /**
  * \fn Signal::disconnect(R (*func)(Args...))
  * \brief Disconnect the signal from the slot static function \a func
  * \param[in] func The slot static function to disconnect
+ *
+ * \context This function is \threadsafe.
  */
 
 /**
@@ -141,6 +171,9 @@ SignalBase::SlotList SignalBase::slots()
  * function are passed to the slot functions unchanged. If a slot modifies one
  * of the arguments (when passed by pointer or reference), the modification is
  * thus visible to all subsequently called slots.
+ *
+ * This function is not \threadsafe, but thread-safety is guaranteed against
+ * concurrent connect() and disconnect() calls.
  */
 
 } /* namespace libcamera */
-- 
Regards,

Laurent Pinchart