libcamera: base: object: Prevent the same signal being connected more than once

Objects are not expected to be connected to the same signal more than
once. Doing so likely indicates a bug in the code, and can be
highlighted in debug builds with an assert that performs a lookup on the
signals_ list.

While it is possible to allow the implementation to let objects connect
to a specific signal multiple times, there are no expected use cases for
this in libcamera and this behaviour is restricted to favour defensive
programming by raising an error when this occurs.

Remove the support in the test framework which uses multiple Signal
connections on the same object, and update the test to use a second
Signal.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

src/libcamera/base/object.cpp

@@ -47,6 +47,14 @@ LOG_DEFINE_CATEGORY(Object)
  * object's thread, regardless of whether the signal is emitted in the same or
  * in another thread.
  *
+ * Objects can be connected to multiple signals, but they can only be connected
+ * to each signal once. Attempting to create multiple concurrent connections
+ * between the same signal and the same Object (to either the same or differents
+ * slots of the object) will cause an assertion failure. While it would be
+ * possible to allow the implementation to let objects connect to the same
+ * signal multiple times, there are no expected use cases for this in libcamera
+ * and this behaviour is restricted to favour defensive programming.
+ *
  * \sa Message, Signal, Thread
  */
 
@@ -284,6 +292,12 @@ void Object::notifyThreadMove()
 
 void Object::connect(SignalBase *signal)
 {
+	/*
+	 * Connecting the same signal to an object multiple times is not
+	 * supported.
+	 */
+	ASSERT(std::find(signals_.begin(), signals_.end(), signal) == signals_.end());
+
 	signals_.push_back(signal);
 }
 

src/libcamera/base/signal.cpp

@@ -93,9 +93,7 @@ SignalBase::SlotList SignalBase::slots()
  * Connecting a signal to a slot results in the slot being called with the
  * arguments passed to the emit() function when the signal is emitted. Multiple
  * slots can be connected to the same signal, and multiple signals can connected
- * to the same slot. Duplicate connections between a signal and a slot are
+ * to the same slot.
- * allowed and result in the slot being called multiple times for the same
- * signal emission.
  *
  * When a slot belongs to an instance of the Object class, the slot is called
  * in the context of the thread that the object is bound to. If the signal is
@@ -105,6 +103,9 @@ SignalBase::SlotList SignalBase::slots()
  * loop, after the Signal::emit() function returns, with a copy of the signal's
  * arguments. The emitter shall thus ensure that any pointer or reference
  * passed through the signal will remain valid after the signal is emitted.
+ *
+ * Duplicate connections between a signal and a slot are not expected and use of
+ * the Object class to manage signals will enforce this restriction.
  */
 
 /**

test/signal.cpp

@@ -212,17 +212,19 @@ protected:
 		/* ----------------- Signal -> Object tests ----------------- */
 
 		/*
-		 * Test automatic disconnection on object deletion. Connect the
+		 * Test automatic disconnection on object deletion. Connect two
-		 * slot twice to ensure all instances are disconnected.
+		 * signals to ensure all instances are disconnected.
 		 */
 		signalVoid_.disconnect();
+		signalVoid2_.disconnect();
 
 		SlotObject *slotObject = new SlotObject();
 		signalVoid_.connect(slotObject, &SlotObject::slot);
-		signalVoid_.connect(slotObject, &SlotObject::slot);
+		signalVoid2_.connect(slotObject, &SlotObject::slot);
 		delete slotObject;
 		valueStatic_ = 0;
 		signalVoid_.emit();
+		signalVoid2_.emit();
 		if (valueStatic_ != 0) {
 			cout << "Signal disconnection on object deletion test failed" << endl;
 			return TestFail;
@@ -298,17 +300,19 @@ protected:
 		/* --------- Signal -> Object (multiple inheritance) -------- */
 
 		/*
-		 * Test automatic disconnection on object deletion. Connect the
+		 * Test automatic disconnection on object deletion. Connect two
-		 * slot twice to ensure all instances are disconnected.
+		 * signals to ensure all instances are disconnected.
 		 */
 		signalVoid_.disconnect();
+		signalVoid2_.disconnect();
 
 		SlotMulti *slotMulti = new SlotMulti();
 		signalVoid_.connect(slotMulti, &SlotMulti::slot);
-		signalVoid_.connect(slotMulti, &SlotMulti::slot);
+		signalVoid2_.connect(slotMulti, &SlotMulti::slot);
 		delete slotMulti;
 		valueStatic_ = 0;
 		signalVoid_.emit();
+		signalVoid2_.emit();
 		if (valueStatic_ != 0) {
 			cout << "Signal disconnection on object deletion test failed" << endl;
 			return TestFail;
@@ -345,6 +349,7 @@ protected:
 
 private:
 	Signal<> signalVoid_;
+	Signal<> signalVoid2_;
 	Signal<int> signalInt_;
 	Signal<int, const std::string &> signalMultiArgs_;