[{"id":11752,"web_url":"https://patchwork.libcamera.org/comment/11752/","msgid":"<20200731142806.GG6218@pendragon.ideasonboard.com>","date":"2020-07-31T14:28:06","subject":"Re: [libcamera-devel] [PATCH v3 2/3] libcamera: object: Add\n\tdeleteLater() support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Fri, Jul 31, 2020 at 01:39:55PM +0000, Umang Jain wrote:\n> This commit adds support to schedule the deletion of an Object to the\n> thread it is bound to (similar to [1]). An Object getting destroyed\n> by a different thread is considered as a violation as per the\n> libcamera threading model.\n> \n> This will be useful for an Object where its ownership is shared via\n> shared pointers in different threads. If the thread which drops the\n> last reference of the Object is a different thread, the destructors\n> get called in that particular thread, not the one Object is bound to.\n> Hence, in order to resolve this kind of situation, the creation of\n> shared pointer can be accompanied by a custom deleter which in turns\n> use deleteLater() to ensure the Object is destroyed in its own thread.\n> \n> [1] https://doc.qt.io/qt-5/qobject.html#deleteLater\n> \n> Signed-off-by: Umang Jain <email@uajain.com>\n> Reviewed-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\n> \n> diff --git a/include/libcamera/internal/message.h b/include/libcamera/internal/message.h\n> index 92ea64a..98e5a28 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\nMissing spaces.\n\n>  \t\tUserMessage = 1000,\n>  \t};\n>  \n> diff --git a/include/libcamera/object.h b/include/libcamera/object.h\n> index 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,\n> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp\n> index e9b3e73..bc985c0 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::UserMessage\n>   * \\brief First value available for user-defined messages\n>   */\n> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n> index 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\nThis uses spaces instead of tabs for indentation.\n\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}\n> diff --git a/test/meson.build b/test/meson.build\n> index 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'],\n> diff --git a/test/object-delete.cpp b/test/object-delete.cpp\n> new file mode 100644\n> index 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\nThis uses spaces too.\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\nMissing space.\n\nDidn't checkstyle.py warn you ?\n\nI'll fix all this when applying.\n\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)","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 1F1EABD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 31 Jul 2020 14:28:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 90FC960398;\n\tFri, 31 Jul 2020 16:28:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B2DCD60398\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 31 Jul 2020 16:28:16 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0B6CA53C;\n\tFri, 31 Jul 2020 16:28:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"O3kzcq0P\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596205696;\n\tbh=udP5BKclNzqxxx4bdFc9RHQuo2cb1CHl1AfUZvA5Zdc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O3kzcq0PosejYWi2WyPKJv6jW4kencku1FaBdeQEmdSqZYGaL9EGpt31ISy6jSNgn\n\tclj/rPR9EcSLIvPnZZrZCR/NGfyb04VJ37Ij1uFpaSfecRmk/bs9bo/hVWflZ1feZx\n\tB6cDrHgfDiTHtLrrTQpWr4HulwhVn2DtWqoyaRaM=","Date":"Fri, 31 Jul 2020 17:28:06 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <email@uajain.com>","Message-ID":"<20200731142806.GG6218@pendragon.ideasonboard.com>","References":"<20200731133947.84258-1-email@uajain.com>\n\t<20200731133947.84258-3-email@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200731133947.84258-3-email@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH v3 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>","Cc":"libcamera-devel@lists.libcamera.org","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>"}},{"id":11753,"web_url":"https://patchwork.libcamera.org/comment/11753/","msgid":"<20200731150920.GH6218@pendragon.ideasonboard.com>","date":"2020-07-31T15:09:20","subject":"Re: [libcamera-devel] [PATCH v3 2/3] libcamera: object: Add\n\tdeleteLater() support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nLooks like I forgot to review the test case.\n\nOn Fri, Jul 31, 2020 at 01:39:55PM +0000, Umang Jain wrote:\n> This commit adds support to schedule the deletion of an Object to the\n> thread it is bound to (similar to [1]). An Object getting destroyed\n> by a different thread is considered as a violation as per the\n> libcamera threading model.\n> \n> This will be useful for an Object where its ownership is shared via\n> shared pointers in different threads. If the thread which drops the\n> last reference of the Object is a different thread, the destructors\n> get called in that particular thread, not the one Object is bound to.\n> Hence, in order to resolve this kind of situation, the creation of\n> shared pointer can be accompanied by a custom deleter which in turns\n> use deleteLater() to ensure the Object is destroyed in its own thread.\n> \n> [1] https://doc.qt.io/qt-5/qobject.html#deleteLater\n> \n> Signed-off-by: Umang Jain <email@uajain.com>\n> Reviewed-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\n> \n> diff --git a/include/libcamera/internal/message.h b/include/libcamera/internal/message.h\n> index 92ea64a..98e5a28 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\tUserMessage = 1000,\n>  \t};\n>  \n> diff --git a/include/libcamera/object.h b/include/libcamera/object.h\n> index 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,\n> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp\n> index e9b3e73..bc985c0 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::UserMessage\n>   * \\brief First value available for user-defined messages\n>   */\n> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n> index 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}\n> diff --git a/test/meson.build b/test/meson.build\n> index 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'],\n> diff --git a/test/object-delete.cpp b/test/object-delete.cpp\n> new file mode 100644\n> index 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\nThis should be static, and named objectDeleted (all variables start with\na lower case).\n\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\nThis can be a local variable in the run() function.\n\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\tTestObject *obj;\n> +\n> +\t\tobj = new TestObject();\n\nYou can combine these two lines.\n\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\nThis isn't needed, as the thread doesn't run any event loop (the exec()\nfunction isn't called by NewThread::run()).\n\n> +\t\tthread.wait();\n\nI think you can simplify this by dropping the timer and the\nisDeleteScheduled_ variable, we can assume the thread will run (there\nare specific tests for that), so just\n\n\tNewThread thread(obj);\n\tthread.start();\n\tthread.wait();\n\nshould be enough.\n\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\nMaybe \"Failed to dispatch DeferredDelete\" ?\n\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\nHow about just\n\n\t\tobj = new TestObject();\n\t\tobj->deleteLater();\n\t\tobj->deleteLater();\n\nand turning ObjectDeleted into objectDeleteCount, and verifying that the\ncounter is equal to 1 ?\n\nI've ended up doing this while investigating another issue (which turned\nout not to be an issue), so I'll send a patch for the test case shortly.\nI propose merging this patch without the test case, and keeping the test\nin a separate patch (I've retained your authorship of course).\n\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)","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 10A5FBD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 31 Jul 2020 15:09:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 746C661A14;\n\tFri, 31 Jul 2020 17:09:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2DE4D60398\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 31 Jul 2020 17:09:30 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8B9CF53C;\n\tFri, 31 Jul 2020 17:09:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"D9nHYTB/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596208169;\n\tbh=k4A2xQGVaIzRUqAU44Xs/IJ60DWOk52W88lkWOzzrOI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=D9nHYTB/Oowj+DeY2QAII0C+d/5XvXLyIiiH63GZEwVR1qKLrWTERkG5UPRRn9mNo\n\tVUXSo9KcR7+p8IATG9Eyk59tOCnyL3JrWS2HI+leJILAFUBsId2fCw7/A1t/JQRJ+f\n\t81v/31aUaJyk2y7TJCiO+mPQJXHYr3qHY+E8VPQU=","Date":"Fri, 31 Jul 2020 18:09:20 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <email@uajain.com>","Message-ID":"<20200731150920.GH6218@pendragon.ideasonboard.com>","References":"<20200731133947.84258-1-email@uajain.com>\n\t<20200731133947.84258-3-email@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200731133947.84258-3-email@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH v3 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>","Cc":"libcamera-devel@lists.libcamera.org","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>"}}]