Patch Detail
Show a patch.
GET /api/1.1/patches/2505/?format=api
{ "id": 2505, "url": "https://patchwork.libcamera.org/api/1.1/patches/2505/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2505/", "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": "<20200104050947.7673-10-laurent.pinchart@ideasonboard.com>", "date": "2020-01-04T05:09:42", "name": "[libcamera-devel,09/14] libcamera: bound_method: Manage BoundMethodPack through std::shared_ptr", "commit_ref": "b0135a1522ed4217a8deb7929fdb36276e58161b", "pull_url": null, "state": "accepted", "archived": false, "hash": "3bed665dfb71773c755c432f03077844c473ed85", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2505/mbox/", "series": [ { "id": 600, "url": "https://patchwork.libcamera.org/api/1.1/series/600/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=600", "date": "2020-01-04T05:09:33", "name": "object: Propagate return value of invoked method", "version": 1, "mbox": "https://patchwork.libcamera.org/series/600/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2505/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2505/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "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 E600960461\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 4 Jan 2020 06:10:05 +0100 (CET)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 85B30A49\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 4 Jan 2020 06:10:05 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578114605;\n\tbh=K6K82HbM2NYoN+Iwsuyf+Rhna/oKWFyF6tyy8l9R1cs=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=b9h8g+Fa1PY9+UuPdj1oM4gJhM9onplvndeHj2N4f312sVAfhhGW3ygabn/P2KdDV\n\tvJXixB+SIm2/dXuHeJBEs+MuFUoZYC2ar0aJgQiVWdER0yoLF9um/fDtByLria03b5\n\t/+beCpKQfgwyDx3Z1oNreRU5BGSlZVIZyoLHORRE=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 4 Jan 2020 07:09:42 +0200", "Message-Id": "<20200104050947.7673-10-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200104050947.7673-1-laurent.pinchart@ideasonboard.com>", "References": "<20200104050947.7673-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 09/14] libcamera: bound_method: Manage\n\tBoundMethodPack through std::shared_ptr", "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>", "X-List-Received-Date": "Sat, 04 Jan 2020 05:10:08 -0000" }, "content": "The bound method arguments pack will need to be accessed by the method\ninvoker in order to retrieve the method return value when using a\nblocking connection type. We thus can't delete the pack unconditionally\nin the bound method target thread. We also can't delete it\nunconditionally in the invoker's thread, as for queued connections the\npack will be used in the target thread after the invoker completes.\n\nThis shows that ownership of the arguments pack is shared between two\ncontexts. As a result, manage it using std::shared_ptr<>.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n Documentation/Doxyfile.in | 1 +\n include/libcamera/bound_method.h | 54 +++++++++++++++++++-------------\n src/libcamera/bound_method.cpp | 5 +--\n src/libcamera/include/message.h | 5 +--\n src/libcamera/message.cpp | 5 +--\n 5 files changed, 43 insertions(+), 27 deletions(-)", "diff": "diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\nindex 9e5efae33919..5ae8773bd3ad 100644\n--- a/Documentation/Doxyfile.in\n+++ b/Documentation/Doxyfile.in\n@@ -874,6 +874,7 @@ EXCLUDE_SYMBOLS = libcamera::BoundMemberMethod \\\n libcamera::BoundMethodArgs \\\n libcamera::BoundMethodBase \\\n libcamera::BoundMethodPack \\\n+ libcamera::BoundMethodPackBase \\\n libcamera::BoundStaticMethod \\\n libcamera::SignalBase \\\n std::*\ndiff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h\nindex b50072ffc098..a74d2c503cdb 100644\n--- a/include/libcamera/bound_method.h\n+++ b/include/libcamera/bound_method.h\n@@ -7,6 +7,7 @@\n #ifndef __LIBCAMERA_BOUND_METHOD_H__\n #define __LIBCAMERA_BOUND_METHOD_H__\n \n+#include <memory>\n #include <tuple>\n #include <type_traits>\n \n@@ -21,6 +22,24 @@ enum ConnectionType {\n \tConnectionTypeBlocking,\n };\n \n+class BoundMethodPackBase\n+{\n+public:\n+\tvirtual ~BoundMethodPackBase() {}\n+};\n+\n+template<typename... Args>\n+class BoundMethodPack : public BoundMethodPackBase\n+{\n+public:\n+\tBoundMethodPack(const Args &... args)\n+\t\t: args_(args...)\n+\t{\n+\t}\n+\n+\tstd::tuple<typename std::remove_reference<Args>::type...> args_;\n+};\n+\n class BoundMethodBase\n {\n public:\n@@ -36,7 +55,7 @@ public:\n \n \tObject *object() const { return object_; }\n \n-\tvirtual void invokePack(void *pack) = 0;\n+\tvirtual void invokePack(BoundMethodPackBase *pack) = 0;\n \n protected:\n #ifndef __DOXYGEN__\n@@ -58,7 +77,8 @@ protected:\n \t};\n #endif\n \n-\tvoid activatePack(void *pack, bool deleteMethod);\n+\tvoid activatePack(std::shared_ptr<BoundMethodPackBase> pack,\n+\t\t\t bool deleteMethod);\n \n \tvoid *obj_;\n \tObject *object_;\n@@ -67,18 +87,6 @@ private:\n \tConnectionType connectionType_;\n };\n \n-template<typename... Args>\n-class BoundMethodPack\n-{\n-public:\n-\tBoundMethodPack(const Args &... args)\n-\t\t: args_(args...)\n-\t{\n-\t}\n-\n-\tstd::tuple<typename std::remove_reference<Args>::type...> args_;\n-};\n-\n template<typename R, typename... Args>\n class BoundMethodArgs : public BoundMethodBase\n {\n@@ -87,18 +95,18 @@ public:\n \n private:\n \ttemplate<int... S>\n-\tvoid invokePack(void *pack, BoundMethodBase::sequence<S...>)\n+\tvoid invokePack(BoundMethodPackBase *pack, BoundMethodBase::sequence<S...>)\n \t{\n-\t\tPackType *args = static_cast<PackType *>(pack);\n+\t\t/* args is effectively unused when the sequence S is empty. */\n+\t\tPackType *args [[gnu::unused]] = static_cast<PackType *>(pack);\n \t\tinvoke(std::get<S>(args->args_)...);\n-\t\tdelete args;\n \t}\n \n public:\n \tBoundMethodArgs(void *obj, Object *object, ConnectionType type)\n \t\t: BoundMethodBase(obj, object, type) {}\n \n-\tvoid invokePack(void *pack) override\n+\tvoid invokePack(BoundMethodPackBase *pack) override\n \t{\n \t\tinvokePack(pack, typename BoundMethodBase::generator<sizeof...(Args)>::type());\n \t}\n@@ -123,10 +131,14 @@ public:\n \n \tvoid activate(Args... args, bool deleteMethod = false) override\n \t{\n-\t\tif (this->object_)\n-\t\t\tBoundMethodBase::activatePack(new PackType{ args... }, deleteMethod);\n-\t\telse\n+\t\tif (!this->object_) {\n \t\t\t(static_cast<T *>(this->obj_)->*func_)(args...);\n+\t\t\treturn;\n+\t\t}\n+\n+\t\tstd::shared_ptr<BoundMethodPackBase> pack =\n+\t\t\tstd::make_shared<typename BoundMemberMethod<T, R, Args...>::PackType>(args...);\n+\t\tBoundMethodBase::activatePack(pack, deleteMethod);\n \t}\n \n \tvoid invoke(Args... args) override\ndiff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp\nindex 45c765774801..82339b7e9c95 100644\n--- a/src/libcamera/bound_method.cpp\n+++ b/src/libcamera/bound_method.cpp\n@@ -48,7 +48,8 @@ namespace libcamera {\n * deadlock will occur.\n */\n \n-void BoundMethodBase::activatePack(void *pack, bool deleteMethod)\n+void BoundMethodBase::activatePack(std::shared_ptr<BoundMethodPackBase> pack,\n+\t\t\t\t bool deleteMethod)\n {\n \tConnectionType type = connectionType_;\n \tif (type == ConnectionTypeAuto) {\n@@ -61,7 +62,7 @@ void BoundMethodBase::activatePack(void *pack, bool deleteMethod)\n \tswitch (type) {\n \tcase ConnectionTypeDirect:\n \tdefault:\n-\t\tinvokePack(pack);\n+\t\tinvokePack(pack.get());\n \t\tif (deleteMethod)\n \t\t\tdelete this;\n \t\tbreak;\ndiff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h\nindex 311755cc60fe..8e8b013dcd18 100644\n--- a/src/libcamera/include/message.h\n+++ b/src/libcamera/include/message.h\n@@ -48,7 +48,8 @@ private:\n class InvokeMessage : public Message\n {\n public:\n-\tInvokeMessage(BoundMethodBase *method, void *pack,\n+\tInvokeMessage(BoundMethodBase *method,\n+\t\t std::shared_ptr<BoundMethodPackBase> pack,\n \t\t Semaphore *semaphore = nullptr,\n \t\t bool deleteMethod = false);\n \t~InvokeMessage();\n@@ -59,7 +60,7 @@ public:\n \n private:\n \tBoundMethodBase *method_;\n-\tvoid *pack_;\n+\tstd::shared_ptr<BoundMethodPackBase> pack_;\n \tSemaphore *semaphore_;\n \tbool deleteMethod_;\n };\ndiff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp\nindex c35bb33dfeda..77f2bdd5fbac 100644\n--- a/src/libcamera/message.cpp\n+++ b/src/libcamera/message.cpp\n@@ -123,7 +123,8 @@ Message::Type Message::registerMessageType()\n * \\param[in] deleteMethod True to delete the \\a method when the message is\n * destroyed\n */\n-InvokeMessage::InvokeMessage(BoundMethodBase *method, void *pack,\n+InvokeMessage::InvokeMessage(BoundMethodBase *method,\n+\t\t\t std::shared_ptr<BoundMethodPackBase> pack,\n \t\t\t Semaphore *semaphore, bool deleteMethod)\n \t: Message(Message::InvokeMessage), method_(method), pack_(pack),\n \t semaphore_(semaphore), deleteMethod_(deleteMethod)\n@@ -148,7 +149,7 @@ InvokeMessage::~InvokeMessage()\n */\n void InvokeMessage::invoke()\n {\n-\tmethod_->invokePack(pack_);\n+\tmethod_->invokePack(pack_.get());\n }\n \n /**\n", "prefixes": [ "libcamera-devel", "09/14" ] }