From patchwork Fri Jul 31 13:39:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 9089 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 61267BD878 for ; Fri, 31 Jul 2020 13:39:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2F85C61ED2; Fri, 31 Jul 2020 15:39:59 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="bzez5/Wr"; dkim-atps=neutral Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D133F616FF for ; Fri, 31 Jul 2020 15:39:56 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc: content-transfer-encoding:content-type; s=s1; bh=MGQPbAd0XKQDUt1FWMgX/SrB2ThHIz3/DdC4y9z43Y0=; b=bzez5/Wr5Y24p/0Tq4D6HXf1JOip9Orb9OAhMZTFUZh/mHVp7DGz+wBuAlhSSbZ4gD9Y nC2VXEH1nkq2sMov9m5KHCs20j1j7NFJBGc7ty+dsspwRHSdVlvBaDQHLMh16/Cw8rAEX3 TBRVWPy7+RpRRNogXPNm7WRzFzppxzq70= Received: by filterdrecv-p3mdw1-7ff865655c-hpdqc with SMTP id filterdrecv-p3mdw1-7ff865655c-hpdqc-19-5F241F2B-25 2020-07-31 13:39:55.564770342 +0000 UTC m=+156224.759476860 Received: from mail.uajain.com (unknown) by ismtpd0005p1maa1.sendgrid.net (SG) with ESMTP id mQXGQXJBQDqYv704DpU31Q Fri, 31 Jul 2020 13:39:54.679 +0000 (UTC) From: Umang Jain Date: Fri, 31 Jul 2020 13:39:55 +0000 (UTC) Message-Id: <20200731133947.84258-3-email@uajain.com> In-Reply-To: <20200731133947.84258-1-email@uajain.com> References: <20200731133947.84258-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcu2AmDZxpR9j2S9uUCg9Ys7krDPmWy2OqGnsRcgal8sKxTv5UymibaUbsW/VyAEP3afoHjebEFcdoq9MaUEQ8Vjg2pT6uEzAwz8VC3pVmHhnIHavlLtoAFaZIIKh/LrmDc3CL7P59uC2Ww54f0zJ/t9hbwnFxIQJ9DIVzssOmx71A7rSaY3zYv8X+oOPFhaNRIKOMc95GpJJHUc+5nnBy8w== To: libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v3 2/3] libcamera: object: Add deleteLater() support X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" This commit adds support to schedule the deletion of an Object to the thread it is bound to (similar to [1]). An Object getting destroyed by a different thread is considered as a violation as per the libcamera threading model. This will be useful for an Object where its ownership is shared via shared pointers in different threads. If the thread which drops the last reference of the Object is a different thread, the destructors get called in that particular thread, not the one Object is bound to. Hence, in order to resolve this kind of situation, the creation of shared pointer can be accompanied by a custom deleter which in turns use deleteLater() to ensure the Object is destroyed in its own thread. [1] https://doc.qt.io/qt-5/qobject.html#deleteLater Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart --- include/libcamera/internal/message.h | 1 + include/libcamera/object.h | 2 + src/libcamera/message.cpp | 2 + src/libcamera/object.cpp | 48 ++++++++++ test/meson.build | 1 + test/object-delete.cpp | 125 +++++++++++++++++++++++++++ 6 files changed, 179 insertions(+) create mode 100644 test/object-delete.cpp diff --git a/include/libcamera/internal/message.h b/include/libcamera/internal/message.h index 92ea64a..98e5a28 100644 --- a/include/libcamera/internal/message.h +++ b/include/libcamera/internal/message.h @@ -25,6 +25,7 @@ public: None = 0, InvokeMessage = 1, ThreadMoveMessage = 2, + DeferredDelete=3, UserMessage = 1000, }; diff --git a/include/libcamera/object.h b/include/libcamera/object.h index 9a3dd07..a1882f0 100644 --- a/include/libcamera/object.h +++ b/include/libcamera/object.h @@ -27,6 +27,8 @@ public: Object(Object *parent = nullptr); virtual ~Object(); + void deleteLater(); + void postMessage(std::unique_ptr msg); templateparent_ = nullptr; } +/** + * \brief Schedule deletion of the instance in the thread it belongs to + * + * This function schedules deletion of the Object when control returns to the + * event loop that the object belongs to. This ensures the object is destroyed + * from the right context, as required by the libcamera threading model. + * + * If this function is called before the thread's event loop is started, the + * object will be deleted when the event loop starts. + * + * Deferred deletion can be used to control the destruction context with shared + * pointers. An object managed with shared pointers is deleted when the last + * reference is destroyed, which makes difficult to ensure through software + * design which context the deletion will take place in. With a custom deleter + * for the shared pointer using deleteLater(), the deletion can be guaranteed to + * happen in the thread the object is bound to. + * + * \code{.cpp} + * std::shared_ptr createObject() + * { + * struct Deleter : std::default_delete { + * void operator()(MyObject *obj) + * { + * delete obj; + * } + * }; + * + * MyObject *obj = new MyObject(); + * + * return std::shared_ptr(obj, Deleter()); + * } + * \endcode + * + * \context This function is \threadsafe. + */ +void Object::deleteLater() +{ + postMessage(std::make_unique(Message::DeferredDelete)); +} + /** * \brief Post a message to the object's thread * \param[in] msg The message @@ -144,6 +188,10 @@ void Object::message(Message *msg) break; } + case Message::DeferredDelete: + delete this; + break; + default: break; } diff --git a/test/meson.build b/test/meson.build index f41d6e7..f5aa144 100644 --- a/test/meson.build +++ b/test/meson.build @@ -34,6 +34,7 @@ internal_tests = [ ['hotplug-cameras', 'hotplug-cameras.cpp'], ['message', 'message.cpp'], ['object', 'object.cpp'], + ['object-delete', 'object-delete.cpp'], ['object-invoke', 'object-invoke.cpp'], ['pixel-format', 'pixel-format.cpp'], ['signal-threads', 'signal-threads.cpp'], diff --git a/test/object-delete.cpp b/test/object-delete.cpp new file mode 100644 index 0000000..6cd1a1e --- /dev/null +++ b/test/object-delete.cpp @@ -0,0 +1,125 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020, Umang Jain + * + * object.cpp - Object deletion tests + */ + +#include + +#include +#include + +#include "libcamera/internal/thread.h" + +#include "test.h" + +using namespace std; +using namespace libcamera; + +/* + * Global flag to check if object is on its way to deletion from it's + * own thread only. + */ +bool ObjectDeleted = false; + +class NewThread : public Thread +{ +public: + NewThread(Object *obj = nullptr) + : object_(obj), deleteScheduled_(false) + { + } + + bool objectScheduledForDeletion() { return deleteScheduled_; } + +protected: + void run() + { + object_->deleteLater(); + deleteScheduled_ = true; + } + +private: + Object *object_; + bool deleteScheduled_; +}; + + +class TestObject : public Object +{ +public: + TestObject() + { + } + + ~TestObject() + { + if (thread() == Thread::current()) + ObjectDeleted = true; + } +}; + +class ObjectDeleteTest : public Test +{ +protected: + int init() + { + isDeleteScheduled_ = false; + + return 0; + } + + int run() + { + TestObject *obj; + + obj = new TestObject(); + NewThread thread(obj); + thread.start(); + Timer timer; + timer.start(100); + + while (timer.isRunning() && !isDeleteScheduled_) + isDeleteScheduled_ = thread.objectScheduledForDeletion(); + + thread.exit(); + thread.wait(); + + if (!isDeleteScheduled_) { + cout << "Failed to queue object for deletion from another thread" << endl; + return TestFail; + } + + Thread::current()->dispatchMessages(Message::Type::DeferredDelete); + + if (!ObjectDeleted) { + cout << "Failed to delete object queued from another thread" << endl; + return TestFail; + } + + ObjectDeleted = false; + obj = new TestObject(); + obj->deleteLater(); + /* + * \todo Devise a check for multiple deleteLater() calls to a object. + * It should be checked that the object is deleted only once. + */ + Thread::current()->dispatchMessages(Message::Type::DeferredDelete); + if(!ObjectDeleted) { + cout << "Failed to delete object"; + return TestFail; + } + + return TestPass; + } + + void cleanup() + { + } + +private: + bool isDeleteScheduled_; +}; + +TEST_REGISTER(ObjectDeleteTest)