Patch Detail
Show a patch.
GET /api/1.1/patches/9085/?format=api
{ "id": 9085, "url": "https://patchwork.libcamera.org/api/1.1/patches/9085/?format=api", "web_url": "https://patchwork.libcamera.org/patch/9085/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200731105335.62014-3-email@uajain.com>", "date": "2020-07-31T10:53:42", "name": "[libcamera-devel,v2,2/3] libcamera: object: Add deleteLater() support", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "8da15f2f4b5e5eec33d948dad0fe49ae2886942b", "submitter": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/people/1/?format=api", "name": "Umang Jain", "email": "email@uajain.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/9085/mbox/", "series": [ { "id": 1167, "url": "https://patchwork.libcamera.org/api/1.1/series/1167/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1167", "date": "2020-07-31T10:53:41", "name": "Add Object::deleteLater() support", "version": 2, "mbox": "https://patchwork.libcamera.org/series/1167/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/9085/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/9085/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6B5DABD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 31 Jul 2020 10:53:48 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 34EB161DE1;\n\tFri, 31 Jul 2020 12:53:48 +0200 (CEST)", "from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7B5361988\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 31 Jul 2020 12:53:43 +0200 (CEST)", "by filterdrecv-p3iad2-d8cc6d457-jqj7p with SMTP id\n\tfilterdrecv-p3iad2-d8cc6d457-jqj7p-18-5F23F836-20\n\t2020-07-31 10:53:42.622161326 +0000 UTC m=+146724.453296916", "from mail.uajain.com (unknown)\n\tby ismtpd0001p1maa1.sendgrid.net (SG) with ESMTP\n\tid tIcws8jzS4mwFnmpr3Ogow Fri, 31 Jul 2020 10:53:41.898 +0000 (UTC)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"Fj/BEi5h\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=from:subject:in-reply-to:references:mime-version:to:cc:\n\tcontent-transfer-encoding:content-type;\n\ts=s1; bh=JrUmiymC4o9tof+s4KHbfmcsApVDZTu4G1cxMyAHBRU=;\n\tb=Fj/BEi5h+Th22CcLMRDqmGvZ8jyS58jrRZVa3WkKBNQAWwrBsgZh5dxQGQEp6ytHaaZH\n\tasWvOMMaT0k+JTYHqp45e6/JbsRwhUWNWhHFsm0ZfiSqiJMpLTkTJmfPBgbXBDwgDQbbzk\n\tY0u0Bv9eCCuwPpxTB0NSOCk2Px+SsC1Zo=", "From": "Umang Jain <email@uajain.com>", "Date": "Fri, 31 Jul 2020 10:53:42 +0000 (UTC)", "Message-Id": "<20200731105335.62014-3-email@uajain.com>", "In-Reply-To": "<20200731105335.62014-1-email@uajain.com>", "References": "<20200731105335.62014-1-email@uajain.com>", "Mime-Version": "1.0", "X-SG-EID": "1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPc+BcW/nTZaHvUFD9w8u96rp9eFG0+/ILAMuChoGefdnk1+uePLUnDkefJPXUZqBWmUnyAErqjiYJguGNkXO82Fi7jS4PU0IFlvVVCIg3kcDu4mFBy3jaW2Br6ctlD3k3niFuJIgNcqkG9pNuCJ0Ct3nBrNh9hXZoLP3PA/m925CO9XPfWB8+OJyY+F3IKV/2RM4xJLRXH+hm1iL+On/ckLw==", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[libcamera-devel] [PATCH v2 2/3] libcamera: object: Add\n\tdeleteLater() support", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "This commit adds support to schedule the deletion of an Object to the\nthread it is bound to (similar to [1]). An Object getting destroyed\nby a different thread is considered as a violation as per the\nlibcamera threading model.\n\nThis will be useful for an Object where its ownership is shared via\nshared pointers in different threads. If the thread which drops the\nlast reference of the Object is a different thread, the destructors\nget called in that particular thread, not the one Object is bound to.\nHence, in order to resolve this kind of situation, the creation of\nshared pointer can be accompanied by a custom deleter which in turns\nuse deleteLater() to ensure the Object is destroyed in its own thread.\n\n[1] https://doc.qt.io/qt-5/qobject.html#deleteLater\n\nSigned-off-by: Umang Jain <email@uajain.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/internal/message.h | 1 +\n include/libcamera/object.h | 2 +\n src/libcamera/message.cpp | 2 +\n src/libcamera/object.cpp | 48 ++++++++++\n test/meson.build | 1 +\n test/object-delete.cpp | 125 +++++++++++++++++++++++++++\n 6 files changed, 179 insertions(+)\n create mode 100644 test/object-delete.cpp", "diff": "diff --git a/include/libcamera/internal/message.h b/include/libcamera/internal/message.h\nindex b8d9866..6e19822 100644\n--- a/include/libcamera/internal/message.h\n+++ b/include/libcamera/internal/message.h\n@@ -25,6 +25,7 @@ public:\n \t\tNone = 0,\n \t\tInvokeMessage = 1,\n \t\tThreadMoveMessage = 2,\n+\t\tDeferredDelete = 3,\n \t\tCatchAll = 999,\n \t\tUserMessage = 1000,\n \t};\ndiff --git a/include/libcamera/object.h b/include/libcamera/object.h\nindex 9a3dd07..a1882f0 100644\n--- a/include/libcamera/object.h\n+++ b/include/libcamera/object.h\n@@ -27,6 +27,8 @@ public:\n \tObject(Object *parent = nullptr);\n \tvirtual ~Object();\n \n+\tvoid deleteLater();\n+\n \tvoid postMessage(std::unique_ptr<Message> msg);\n \n \ttemplate<typename T, typename R, typename... FuncArgs, typename... Args,\ndiff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp\nindex e462f90..770e226 100644\n--- a/src/libcamera/message.cpp\n+++ b/src/libcamera/message.cpp\n@@ -49,6 +49,8 @@ std::atomic_uint Message::nextUserType_{ Message::UserMessage };\n * \\brief Asynchronous method invocation across threads\n * \\var Message::ThreadMoveMessage\n * \\brief Object is being moved to a different thread\n+ * \\var Message::DeferredDelete\n+ * \\brief Object is scheduled for deletion\n * \\var Message::CatchAll\n * \\brief Helper to match message of any type\n * \\var Message::UserMessage\ndiff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\nindex 1544a23..74e2268 100644\n--- a/src/libcamera/object.cpp\n+++ b/src/libcamera/object.cpp\n@@ -73,6 +73,10 @@ Object::Object(Object *parent)\n * Deleting an Object automatically disconnects all signals from the Object's\n * slots. All the Object's children are made orphan, but stay bound to their\n * current thread.\n+ *\n+ * Object instances shall be destroyed from the thread they are bound to,\n+ * otherwise undefined behaviour may occur. If deletion of an Object needs to\n+ * be scheduled from a different thread, deleteLater() shall be used.\n */\n Object::~Object()\n {\n@@ -98,6 +102,46 @@ Object::~Object()\n \t\tchild->parent_ = nullptr;\n }\n \n+/**\n+ * \\brief Schedule deletion of the instance in the thread it belongs to\n+ *\n+ * This function schedules deletion of the Object when control returns to the\n+ * event loop that the object belongs to. This ensures the object is destroyed\n+ * from the right context, as required by the libcamera threading model.\n+ *\n+ * If this function is called before the thread's event loop is started, the\n+ * object will be deleted when the event loop starts.\n+ *\n+ * Deferred deletion can be used to control the destruction context with shared\n+ * pointers. An object managed with shared pointers is deleted when the last\n+ * reference is destroyed, which makes difficult to ensure through software\n+ * design which context the deletion will take place in. With a custom deleter\n+ * for the shared pointer using deleteLater(), the deletion can be guaranteed to\n+ * happen in the thread the object is bound to.\n+ *\n+ * \\code{.cpp}\n+ * std::shared_ptr<MyObject> createObject()\n+ * {\n+ * struct Deleter : std::default_delete<MyObject> {\n+ * void operator()(MyObject *obj)\n+ * {\n+ * delete obj;\n+ * }\n+ * };\n+ *\n+ * MyObject *obj = new MyObject();\n+ *\n+ * return std::shared_ptr<MyObject>(obj, Deleter());\n+ * }\n+ * \\endcode\n+ *\n+ * \\context This function is \\threadsafe.\n+ */\n+void Object::deleteLater()\n+{\n+ postMessage(std::make_unique<Message>(Message::DeferredDelete));\n+}\n+\n /**\n * \\brief Post a message to the object's thread\n * \\param[in] msg The message\n@@ -144,6 +188,10 @@ void Object::message(Message *msg)\n \t\tbreak;\n \t}\n \n+\tcase Message::DeferredDelete:\n+\t\tdelete this;\n+\t\tbreak;\n+\n \tdefault:\n \t\tbreak;\n \t}\ndiff --git a/test/meson.build b/test/meson.build\nindex f41d6e7..f5aa144 100644\n--- a/test/meson.build\n+++ b/test/meson.build\n@@ -34,6 +34,7 @@ internal_tests = [\n ['hotplug-cameras', 'hotplug-cameras.cpp'],\n ['message', 'message.cpp'],\n ['object', 'object.cpp'],\n+ ['object-delete', 'object-delete.cpp'],\n ['object-invoke', 'object-invoke.cpp'],\n ['pixel-format', 'pixel-format.cpp'],\n ['signal-threads', 'signal-threads.cpp'],\ndiff --git a/test/object-delete.cpp b/test/object-delete.cpp\nnew file mode 100644\nindex 0000000..6cd1a1e\n--- /dev/null\n+++ b/test/object-delete.cpp\n@@ -0,0 +1,125 @@\n+/* SPDX-License-Identifier: GPL-2.0-or-later */\n+/*\n+ * Copyright (C) 2020, Umang Jain <email@uajain.com>\n+ *\n+ * object.cpp - Object deletion tests\n+ */\n+\n+#include <iostream>\n+\n+#include <libcamera/object.h>\n+#include <libcamera/timer.h>\n+\n+#include \"libcamera/internal/thread.h\"\n+\n+#include \"test.h\"\n+\n+using namespace std;\n+using namespace libcamera;\n+\n+/*\n+ * Global flag to check if object is on its way to deletion from it's\n+ * own thread only.\n+ */\n+bool ObjectDeleted = false;\n+\n+class NewThread : public Thread\n+{\n+public:\n+\tNewThread(Object *obj = nullptr)\n+\t\t: object_(obj), deleteScheduled_(false)\n+\t{\n+\t}\n+\n+\tbool objectScheduledForDeletion() { return deleteScheduled_; }\n+\n+protected:\n+ void run()\n+ {\n+\t\tobject_->deleteLater();\n+\t\tdeleteScheduled_ = true;\n+ }\n+\n+private:\n+\tObject *object_;\n+\tbool deleteScheduled_;\n+};\n+\n+\n+class TestObject : public Object\n+{\n+public:\n+\tTestObject()\n+\t{\n+\t}\n+\n+\t~TestObject()\n+\t{\n+\t\tif (thread() == Thread::current())\n+\t\t\tObjectDeleted = true;\n+\t}\n+};\n+\n+class ObjectDeleteTest : public Test\n+{\n+protected:\n+\tint init()\n+\t{\n+\t\tisDeleteScheduled_ = false;\n+\n+\t\treturn 0;\n+\t}\n+\n+\tint run()\n+\t{\n+\t\tTestObject *obj;\n+\n+\t\tobj = new TestObject();\n+\t\tNewThread thread(obj);\n+\t\tthread.start();\n+\t\tTimer timer;\n+\t\ttimer.start(100);\n+\n+\t\twhile (timer.isRunning() && !isDeleteScheduled_)\n+\t\t\tisDeleteScheduled_ = thread.objectScheduledForDeletion();\n+\n+\t\tthread.exit();\n+\t\tthread.wait();\n+\n+\t\tif (!isDeleteScheduled_) {\n+\t\t\tcout << \"Failed to queue object for deletion from another thread\" << endl;\n+\t\t\treturn TestFail;\n+\t\t}\n+\n+\t\tThread::current()->dispatchMessages(Message::Type::DeferredDelete);\n+\n+\t\tif (!ObjectDeleted) {\n+\t\t\tcout << \"Failed to delete object queued from another thread\" << endl;\n+\t\t\treturn TestFail;\n+\t\t}\n+\n+\t\tObjectDeleted = false;\n+\t\tobj = new TestObject();\n+\t\tobj->deleteLater();\n+\t\t/*\n+\t\t * \\todo Devise a check for multiple deleteLater() calls to a object.\n+\t\t * It should be checked that the object is deleted only once.\n+\t\t */\n+\t\tThread::current()->dispatchMessages(Message::Type::DeferredDelete);\n+\t\tif(!ObjectDeleted) {\n+\t\t\tcout << \"Failed to delete object\";\n+\t\t\treturn TestFail;\n+\t\t}\n+\n+\t\treturn TestPass;\n+\t}\n+\n+\tvoid cleanup()\n+\t{\n+\t}\n+\n+private:\n+\tbool isDeleteScheduled_;\n+};\n+\n+TEST_REGISTER(ObjectDeleteTest)\n", "prefixes": [ "libcamera-devel", "v2", "2/3" ] }