From patchwork Tue Feb 12 22:37:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 561 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 786C160B21 for ; Tue, 12 Feb 2019 23:37:09 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EB56585 for ; Tue, 12 Feb 2019 23:37:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1550011029; bh=SlcslwDe9qmust3GNJIXAERP23Vo29Ja8Bw4muq/2yw=; h=From:To:Subject:Date:From; b=f1IY5svNm2eM4nzsK4nIyYbm7uUj6EfhvP5GCK3Sa6cID4YRmO+2FzcC82nutcEsF wKJUVFgVRkMIoL52eqTZO0mChXXx7dsKWH3u96nZGwoMHTwzDPIUI34FjnxmBLRs2C 53aMnCRYFNuHeqISKRdBN0Iw0egVeIHZnGb57WiU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Wed, 13 Feb 2019 00:37:00 +0200 Message-Id: <20190212223702.9582-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/3] libcamera: pipeline_handler: Disconnect MediaDevice::disconnected signal X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Feb 2019 22:37:09 -0000 The pipeline handler connects the disconnected signal of MediaDevice instances registered for hotplug handling to a member slot. Disconnect the signal when the slot is called, as the pipeline handler will be deleted. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline_handler.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 4e111d6d2f55..616838fed702 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -241,6 +241,8 @@ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) */ void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media) { + media->disconnected.disconnect(this); + if (cameras_.empty()) return; From patchwork Tue Feb 12 22:37:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 562 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AC5FD60B21 for ; Tue, 12 Feb 2019 23:37:09 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3B85D2CF for ; Tue, 12 Feb 2019 23:37:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1550011029; bh=y20Of7z8P4SNb2vd8O1slHz15AWNFkxOLw4j0PDHJUQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ErbFiisg9k8Vjlst9GnYU+O/MyAZEACsqYVhBJammsCS3rSvyLxDrCnW7VlCAWyF/ J1OCMyWxuJkK+ixqQvcRU8aWUNzwK9tL76a/zntKWQOPbyMiTM6SDuHS7jT6mwPs3M muCIJX6FHKGODCkOXEjqZ3cqh8hIxZDJAT41h43s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Wed, 13 Feb 2019 00:37:01 +0200 Message-Id: <20190212223702.9582-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190212223702.9582-1-laurent.pinchart@ideasonboard.com> References: <20190212223702.9582-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/3] libcamera: signal: Fix coding style issues X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Feb 2019 22:37:09 -0000 Fix issues reported by checkstyle.py in preparation for further changes. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Changes since v1: - Don't add a space after the template keyword --- include/libcamera/signal.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h index c375b0a878af..85dc481d6957 100644 --- a/include/libcamera/signal.h +++ b/include/libcamera/signal.h @@ -34,28 +34,28 @@ template class SlotMember : public SlotBase { public: - SlotMember(T *obj, void(T::*func)(Args...)) + SlotMember(T *obj, void (T::*func)(Args...)) : SlotBase(obj), func_(func) { } void invoke(Args... args) { (static_cast(this->obj_)->*func_)(args...); } private: friend class Signal; - void(T::*func_)(Args...); + void (T::*func_)(Args...); }; template class SlotStatic : public SlotBase { public: - SlotStatic(void(*func)(Args...)) + SlotStatic(void (*func)(Args...)) : SlotBase(nullptr), func_(func) { } void invoke(Args... args) { (*func_)(args...); } private: friend class Signal; - void(*func_)(Args...); + void (*func_)(Args...); }; template @@ -70,12 +70,12 @@ public: } template - void connect(T *object, void(T::*func)(Args...)) + void connect(T *object, void (T::*func)(Args...)) { slots_.push_back(new SlotMember(object, func)); } - void connect(void(*func)(Args...)) + void connect(void (*func)(Args...)) { slots_.push_back(new SlotStatic(func)); } @@ -102,7 +102,7 @@ public: } template - void disconnect(T *object, void(T::*func)(Args...)) + void disconnect(T *object, void (T::*func)(Args...)) { for (auto iter = slots_.begin(); iter != slots_.end(); ) { SlotBase *slot = *iter; @@ -120,7 +120,7 @@ public: } } - void disconnect(void(*func)(Args...)) + void disconnect(void (*func)(Args...)) { for (auto iter = slots_.begin(); iter != slots_.end(); ) { SlotBase *slot = *iter; From patchwork Tue Feb 12 22:37:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 563 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E4E9260B21 for ; Tue, 12 Feb 2019 23:37:09 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7FDE62D6 for ; Tue, 12 Feb 2019 23:37:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1550011029; bh=GaxYGmc7QyxQAgV7D1XW9eE8WnOUmBIoKXYu7KBkymo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=BaNzl5wE2QGbRkoZHqlvNtfNqPYOp3IaFg7k8quElPNaIBbIEU4yyfYiZZfiSGfr5 1VfP4Ptp0ZeNhz3ExEd+wUychbRso9RakdnHngdFGhFttr7NfbL1K5dUiHaUaA5JBY aUfB3Fwnrhqw/fbjmmkB17K014fml+hjsdofROO8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Wed, 13 Feb 2019 00:37:02 +0200 Message-Id: <20190212223702.9582-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190212223702.9582-1-laurent.pinchart@ideasonboard.com> References: <20190212223702.9582-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/3] libcamera: signal: Disconnect signal automatically on slot deletion X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Feb 2019 22:37:10 -0000 When a signal is connected to a member function slot, the slot is not disconnected when the slot object is deleted. This can lead to calling a member function of a deleted object if the signal isn't disconnected manually by the slot object's destructor. Make signal handling easier by implementing a base Object class that tracks all connected signals and disconnects from them automatically when the object is deleted, using template specialization resolution in the Signal class. As inheriting from the Object class may to a too harsh requirement for Signal usage in applications, keep the existing behaviour working if the slot doesn't inherit from the Object class. We may reconsider this later and require all slot objects to inherit from the Object class. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Documentation/Doxyfile.in | 4 +- include/libcamera/meson.build | 1 + include/libcamera/object.h | 35 +++++++++++ include/libcamera/signal.h | 115 +++++++++++++++++++++++----------- src/libcamera/meson.build | 1 + src/libcamera/object.cpp | 50 +++++++++++++++ src/libcamera/signal.cpp | 5 ++ test/signal.cpp | 37 +++++++++++ 8 files changed, 211 insertions(+), 37 deletions(-) create mode 100644 include/libcamera/object.h create mode 100644 src/libcamera/object.cpp diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in index b78fb3a0b0b6..3e2b7fd9da0e 100644 --- a/Documentation/Doxyfile.in +++ b/Documentation/Doxyfile.in @@ -860,7 +860,9 @@ EXCLUDE_PATTERNS = # Note that the wildcards are matched against the file with absolute path, so to # exclude all test directories use the pattern */test/* -EXCLUDE_SYMBOLS = libcamera::SlotBase \ +EXCLUDE_SYMBOLS = libcamera::SignalBase \ + libcamera::SlotArgs \ + libcamera::SlotBase \ libcamera::SlotMember \ libcamera::SlotStatic diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 5788e9bbdf3e..3f4d1e28208b 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -5,6 +5,7 @@ libcamera_api = files([ 'event_dispatcher.h', 'event_notifier.h', 'libcamera.h', + 'object.h', 'request.h', 'signal.h', 'stream.h', diff --git a/include/libcamera/object.h b/include/libcamera/object.h new file mode 100644 index 000000000000..eadd41f9a41f --- /dev/null +++ b/include/libcamera/object.h @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * object.h - Base object + */ +#ifndef __LIBCAMERA_OBJECT_H__ +#define __LIBCAMERA_OBJECT_H__ + +#include + +namespace libcamera { + +class SignalBase; +template +class Signal; + +class Object +{ +public: + virtual ~Object(); + +private: + template + friend class Signal; + + void connect(SignalBase *signal); + void disconnect(SignalBase *signal); + + std::list signals_; +}; + +}; /* namespace libcamera */ + +#endif /* __LIBCAMERA_OBJECT_H__ */ diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h index 85dc481d6957..434fc0109f42 100644 --- a/include/libcamera/signal.h +++ b/include/libcamera/signal.h @@ -10,32 +10,47 @@ #include #include +#include + namespace libcamera { template class Signal; -template class SlotBase { public: - SlotBase(void *obj) - : obj_(obj) { } - virtual ~SlotBase() { } + SlotBase(void *obj, bool isObject) + : obj_(obj), isObject_(isObject) {} + virtual ~SlotBase() {} - virtual void invoke(Args... args) = 0; + void *obj() { return obj_; } + bool isObject() const { return isObject_; } protected: - friend class Signal; void *obj_; + bool isObject_; +}; + +template +class SlotArgs : public SlotBase +{ +public: + SlotArgs(void *obj, bool isObject) + : SlotBase(obj, isObject) {} + + virtual void invoke(Args... args) = 0; + +protected: + friend class Signal; }; template -class SlotMember : public SlotBase +class SlotMember : public SlotArgs { public: - SlotMember(T *obj, void (T::*func)(Args...)) - : SlotBase(obj), func_(func) { } + SlotMember(T *obj, bool isObject, void (T::*func)(Args...)) + : SlotArgs(obj, isObject), func_(func) {} void invoke(Args... args) { (static_cast(this->obj_)->*func_)(args...); } @@ -45,11 +60,11 @@ private: }; template -class SlotStatic : public SlotBase +class SlotStatic : public SlotArgs { public: SlotStatic(void (*func)(Args...)) - : SlotBase(nullptr), func_(func) { } + : SlotArgs(nullptr, false), func_(func) {} void invoke(Args... args) { (*func_)(args...); } @@ -58,21 +73,57 @@ private: void (*func_)(Args...); }; +class SignalBase +{ +public: + template + void disconnect(T *object) + { + for (auto iter = slots_.begin(); iter != slots_.end(); ) { + SlotBase *slot = *iter; + if (slot->obj() == object) { + iter = slots_.erase(iter); + delete slot; + } else { + ++iter; + } + } + } + +protected: + friend class Object; + std::list slots_; +}; + template -class Signal +class Signal : public SignalBase { public: Signal() { } ~Signal() { - for (SlotBase *slot : slots_) + for (SlotBase *slot : slots_) { + if (slot->isObject()) + static_cast(slot->obj())->disconnect(this); delete slot; + } } +#ifndef __DOXYGEN__ + template::value>::type * = nullptr> + void connect(T *object, void (T::*func)(Args...)) + { + object->connect(this); + slots_.push_back(new SlotMember(object, true, func)); + } + + template::value>::type * = nullptr> +#else template +#endif void connect(T *object, void (T::*func)(Args...)) { - slots_.push_back(new SlotMember(object, func)); + slots_.push_back(new SlotMember(object, false, func)); } void connect(void (*func)(Args...)) @@ -82,7 +133,7 @@ public: void disconnect() { - for (SlotBase *slot : slots_) + for (SlotBase *slot : slots_) delete slot; slots_.clear(); } @@ -90,27 +141,21 @@ public: template void disconnect(T *object) { - for (auto iter = slots_.begin(); iter != slots_.end(); ) { - SlotBase *slot = *iter; - if (slot->obj_ == object) { - iter = slots_.erase(iter); - delete slot; - } else { - ++iter; - } - } + SignalBase::disconnect(object); } template void disconnect(T *object, void (T::*func)(Args...)) { for (auto iter = slots_.begin(); iter != slots_.end(); ) { - SlotBase *slot = *iter; + SlotArgs *slot = static_cast *>(*iter); /* - * If the obj_ pointer matches the object types must - * match, so we can safely cast to SlotMember. + * If the obj() pointer matches the object, the slot is + * guaranteed to be a member slot, so we can safely + * cast it to SlotMember and access its + * func_ member. */ - if (slot->obj_ == object && + if (slot->obj() == object && static_cast *>(slot)->func_ == func) { iter = slots_.erase(iter); delete slot; @@ -123,8 +168,8 @@ public: void disconnect(void (*func)(Args...)) { for (auto iter = slots_.begin(); iter != slots_.end(); ) { - SlotBase *slot = *iter; - if (slot->obj_ == nullptr && + SlotArgs *slot = *iter; + if (slot->obj() == nullptr && static_cast *>(slot)->func_ == func) { iter = slots_.erase(iter); delete slot; @@ -140,13 +185,11 @@ public: * Make a copy of the slots list as the slot could call the * disconnect operation, invalidating the iterator. */ - std::vector *> slots{ slots_.begin(), slots_.end() }; - for (SlotBase *slot : slots) - slot->invoke(args...); + std::vector slots{ slots_.begin(), slots_.end() }; + for (SlotBase *slot : slots) { + static_cast *>(slot)->invoke(args...); + } } - -private: - std::list *> slots_; }; } /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index c5354c136563..8384cd0af451 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -10,6 +10,7 @@ libcamera_sources = files([ 'log.cpp', 'media_device.cpp', 'media_object.cpp', + 'object.cpp', 'pipeline_handler.cpp', 'request.cpp', 'signal.cpp', diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp new file mode 100644 index 000000000000..826eed6f9b3a --- /dev/null +++ b/src/libcamera/object.cpp @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * object.cpp - Base object + */ + +#include +#include + +/** + * \file object.h + * \brief Base object to support automatic signal disconnection + */ + +namespace libcamera { + +/** + * \class Object + * \brief Base object to support automatic signal disconnection + * + * The Object class simplifies signal/slot handling for classes implementing + * slots. By inheriting from Object, an object is automatically disconnected + * from all connected signals when it gets destroyed. + * + * \sa Signal + */ + +Object::~Object() +{ + for (SignalBase *signal : signals_) + signal->disconnect(this); +} + +void Object::connect(SignalBase *signal) +{ + signals_.push_back(signal); +} + +void Object::disconnect(SignalBase *signal) +{ + for (auto iter = signals_.begin(); iter != signals_.end(); ) { + if (*iter == signal) + iter = signals_.erase(iter); + else + iter++; + } +} + +}; /* namespace libcamera */ diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp index 8d62b5beeeac..cb7daa11dab1 100644 --- a/src/libcamera/signal.cpp +++ b/src/libcamera/signal.cpp @@ -47,6 +47,11 @@ namespace libcamera { * \brief Connect the signal to a member function slot * \param object The slot object pointer * \param func The slot member function + * + * If the typename T inherits from Object, the signal will be automatically + * 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. */ /** diff --git a/test/signal.cpp b/test/signal.cpp index ab69ca1b663d..19a52c603a4a 100644 --- a/test/signal.cpp +++ b/test/signal.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include "test.h" @@ -22,6 +23,15 @@ static void slotStatic(int value) valueStatic_ = value; } +class SlotObject : public Object +{ +public: + void slot() + { + valueStatic_ = 1; + } +}; + class SignalTest : public Test { protected: @@ -139,6 +149,33 @@ protected: return TestFail; } + /* + * Test automatic disconnection on object deletion. Connect the + * slot twice to ensure all instances are disconnected. + */ + signalVoid_.disconnect(); + + SlotObject *slotObject = new SlotObject(); + signalVoid_.connect(slotObject, &SlotObject::slot); + signalVoid_.connect(slotObject, &SlotObject::slot); + delete slotObject; + valueStatic_ = 0; + signalVoid_.emit(); + if (valueStatic_ != 0) { + cout << "Signal disconnection on object deletion test failed" << endl; + return TestFail; + } + + /* + * Test that signal deletion disconnects objects. This shall + * not generate any valgrind warning. + */ + Signal<> *signal = new Signal<>(); + slotObject = new SlotObject(); + signal->connect(slotObject, &SlotObject::slot); + delete signal; + delete slotObject; + return TestPass; }