[libcamera-devel] [PATCH 09/19] libcamera: signal: Make slots list private
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jan 20 01:24:27 CET 2020
The slots list is touched from most of the Signal template functions. In
order to prepare for thread-safety, move handling of the lists list to a
small number of non-template functions in the SignalBase class.
This incidently fixes a bug in signal disconnection handling where the
signal wasn't removed from the object's signals list, as pointed out by
the signals unit test.
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
include/libcamera/object.h | 4 +-
include/libcamera/signal.h | 88 ++++++++++++++++----------------------
src/libcamera/object.cpp | 7 ++-
src/libcamera/signal.cpp | 36 ++++++++++++++++
4 files changed, 81 insertions(+), 54 deletions(-)
diff --git a/include/libcamera/object.h b/include/libcamera/object.h
index 9344af30bc79..4d16f3f2b610 100644
--- a/include/libcamera/object.h
+++ b/include/libcamera/object.h
@@ -48,9 +48,7 @@ protected:
virtual void message(Message *msg);
private:
- template<typename... Args>
- friend class Signal;
- friend class BoundMethodBase;
+ friend class SignalBase;
friend class Thread;
void notifyThreadMove();
diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h
index 432d95d0ed1c..c13bb30f0636 100644
--- a/include/libcamera/signal.h
+++ b/include/libcamera/signal.h
@@ -7,6 +7,7 @@
#ifndef __LIBCAMERA_SIGNAL_H__
#define __LIBCAMERA_SIGNAL_H__
+#include <functional>
#include <list>
#include <type_traits>
#include <vector>
@@ -19,23 +20,18 @@ namespace libcamera {
class SignalBase
{
public:
- template<typename T>
- void disconnect(T *obj)
- {
- for (auto iter = slots_.begin(); iter != slots_.end(); ) {
- BoundMethodBase *slot = *iter;
- if (slot->match(obj)) {
- iter = slots_.erase(iter);
- delete slot;
- } else {
- ++iter;
- }
- }
- }
+ void disconnect(Object *object);
protected:
- friend class Object;
- std::list<BoundMethodBase *> slots_;
+ using SlotList = std::list<BoundMethodBase *>;
+
+ void connect(BoundMethodBase *slot);
+ void disconnect(std::function<bool(SlotList::iterator &)> match);
+
+ SlotList slots();
+
+private:
+ SlotList slots_;
};
template<typename... Args>
@@ -45,12 +41,7 @@ public:
Signal() {}
~Signal()
{
- for (BoundMethodBase *slot : slots_) {
- Object *object = slot->object();
- if (object)
- object->disconnect(this);
- delete slot;
- }
+ disconnect();
}
#ifndef __DOXYGEN__
@@ -59,8 +50,7 @@ public:
ConnectionType type = ConnectionTypeAuto)
{
Object *object = static_cast<Object *>(obj);
- object->connect(this);
- slots_.push_back(new BoundMethodMember<T, void, Args...>(obj, object, func, type));
+ SignalBase::connect(new BoundMethodMember<T, void, Args...>(obj, object, func, type));
}
template<typename T, typename R, typename std::enable_if<!std::is_base_of<Object, T>::value>::type * = nullptr>
@@ -69,63 +59,62 @@ public:
#endif
void connect(T *obj, R (T::*func)(Args...))
{
- slots_.push_back(new BoundMethodMember<T, R, Args...>(obj, nullptr, func));
+ SignalBase::connect(new BoundMethodMember<T, R, Args...>(obj, nullptr, func));
}
template<typename R>
void connect(R (*func)(Args...))
{
- slots_.push_back(new BoundMethodStatic<R, Args...>(func));
+ SignalBase::connect(new BoundMethodStatic<R, Args...>(func));
}
void disconnect()
{
- for (BoundMethodBase *slot : slots_)
- delete slot;
- slots_.clear();
+ SignalBase::disconnect([](SlotList::iterator &iter) {
+ return true;
+ });
}
template<typename T>
void disconnect(T *obj)
{
- SignalBase::disconnect(obj);
+ SignalBase::disconnect([obj](SlotList::iterator &iter) {
+ return (*iter)->match(obj);
+ });
}
template<typename T, typename R>
void disconnect(T *obj, R (T::*func)(Args...))
{
- for (auto iter = slots_.begin(); iter != slots_.end(); ) {
+ SignalBase::disconnect([obj, func](SlotList::iterator &iter) {
BoundMethodArgs<R, Args...> *slot =
static_cast<BoundMethodArgs<R, Args...> *>(*iter);
+
+ if (!slot->match(obj))
+ return false;
+
/*
* If the object matches the slot, the slot is
* guaranteed to be a member slot, so we can safely
* cast it to BoundMethodMember<T, Args...> to match
* func.
*/
- if (slot->match(obj) &&
- static_cast<BoundMethodMember<T, R, Args...> *>(slot)->match(func)) {
- iter = slots_.erase(iter);
- delete slot;
- } else {
- ++iter;
- }
- }
+ return static_cast<BoundMethodMember<T, R, Args...> *>(slot)->match(func);
+ });
}
template<typename R>
void disconnect(R (*func)(Args...))
{
- for (auto iter = slots_.begin(); iter != slots_.end(); ) {
- BoundMethodArgs<R, Args...> *slot = *iter;
- if (slot->match(nullptr) &&
- static_cast<BoundMethodStatic<R, Args...> *>(slot)->match(func)) {
- iter = slots_.erase(iter);
- delete slot;
- } else {
- ++iter;
- }
- }
+ SignalBase::disconnect([func](SlotList::iterator &iter) {
+ BoundMethodArgs<R, Args...> *slot =
+ static_cast<BoundMethodArgs<R, Args...> *>(*iter);
+
+ if (!slot->match(nullptr))
+ return false;
+
+ return static_cast<BoundMethodStatic<R, Args...> *>(slot)->match(func);
+ });
}
void emit(Args... args)
@@ -134,8 +123,7 @@ public:
* Make a copy of the slots list as the slot could call the
* disconnect operation, invalidating the iterator.
*/
- std::vector<BoundMethodBase *> slots{ slots_.begin(), slots_.end() };
- for (BoundMethodBase *slot : slots)
+ for (BoundMethodBase *slot : slots())
static_cast<BoundMethodArgs<void, Args...> *>(slot)->activate(args...);
}
};
diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
index 21aad5652b38..f2a8be172547 100644
--- a/src/libcamera/object.cpp
+++ b/src/libcamera/object.cpp
@@ -76,7 +76,12 @@ Object::Object(Object *parent)
*/
Object::~Object()
{
- for (SignalBase *signal : signals_)
+ /*
+ * Move signals to a private list to avoid concurrent iteration and
+ * deletion of items from Signal::disconnect().
+ */
+ std::list<SignalBase *> signals(std::move(signals_));
+ for (SignalBase *signal : signals)
signal->disconnect(this);
if (pendingMessages_)
diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp
index 190033317c72..9bb7eca8ed6e 100644
--- a/src/libcamera/signal.cpp
+++ b/src/libcamera/signal.cpp
@@ -14,6 +14,42 @@
namespace libcamera {
+void SignalBase::connect(BoundMethodBase *slot)
+{
+ Object *object = slot->object();
+ if (object)
+ object->connect(this);
+ slots_.push_back(slot);
+}
+
+void SignalBase::disconnect(Object *object)
+{
+ disconnect([object](SlotList::iterator &iter) {
+ return (*iter)->match(object);
+ });
+}
+
+void SignalBase::disconnect(std::function<bool(SlotList::iterator &)> match)
+{
+ for (auto iter = slots_.begin(); iter != slots_.end(); ) {
+ if (match(iter)) {
+ Object *object = (*iter)->object();
+ if (object)
+ object->disconnect(this);
+
+ delete *iter;
+ iter = slots_.erase(iter);
+ } else {
+ ++iter;
+ }
+ }
+}
+
+SignalBase::SlotList SignalBase::slots()
+{
+ return slots_;
+}
+
/**
* \class Signal
* \brief Generic signal and slot communication mechanism
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list