Patch Detail
Show a patch.
GET /api/1.1/patches/12764/?format=api
{ "id": 12764, "url": "https://patchwork.libcamera.org/api/1.1/patches/12764/?format=api", "web_url": "https://patchwork.libcamera.org/patch/12764/", "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": "<20210701230741.14320-4-laurent.pinchart@ideasonboard.com>", "date": "2021-07-01T23:07:41", "name": "[libcamera-devel,3/3] base: thread: Fix recursive calls to dispatchMessages()", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "df575b338a0c095a214a78a1aa3bb7ac764ac03e", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": { "id": 14, "url": "https://patchwork.libcamera.org/api/1.1/users/14/?format=api", "username": "pinchartl", "first_name": "Laurent", "last_name": "Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/patch/12764/mbox/", "series": [ { "id": 2198, "url": "https://patchwork.libcamera.org/api/1.1/series/2198/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2198", "date": "2021-07-01T23:07:38", "name": "base: Fix crash with recursive messages dispatch", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2198/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/12764/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/12764/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 9886EC3224\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 1 Jul 2021 23:08:32 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 941AE684FD;\n\tFri, 2 Jul 2021 01:08: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 A6E27684E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 2 Jul 2021 01:08:26 +0200 (CEST)", "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C3ADE53;\n\tFri, 2 Jul 2021 01:08:26 +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=\"L+C2ctZ5\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1625180906;\n\tbh=Glf15EbSOCrKKbh3HM/ycvOlUuHT+xYMVwy/up78d3E=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=L+C2ctZ5Jws6EgP6MJXWu0LFOnMLbAEFl6/SnbLLsNnpr5pWhwIohnVc3zKn59uUY\n\tpsowLNuzwGutbQ5k7sMoYJJ6NDJiOZpsW5UaD+0M6kgA2WKc0yf6UDtd/YYu331JFQ\n\tVzAu8wCjp4Uf+EfenQg4TJTdlLNXrcIma+KsP/Rg=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 2 Jul 2021 02:07:41 +0300", "Message-Id": "<20210701230741.14320-4-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<20210701230741.14320-1-laurent.pinchart@ideasonboard.com>", "References": "<20210701230741.14320-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 3/3] base: thread: Fix recursive calls to\n\tdispatchMessages()", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "There are use cases for calling the dispatchMessages() function\nrecursively, from within a message handler. This can be used, for\ninstance, to force delivery of messages posted to a thread concurrently\nto stopping the thread. This currently causes access, in the outer\ndispatchMessages() call, to iterators that have been invalidated by\nerasing list elements in the recursive call, leading to undefined\nbehaviour (most likely double-free or other crashes).\n\nFix it by only erasing messages from the list at the end of the outer\ncall, identified using a recursion counter. We need to keep track of the\nfirst non-null entry in the posted messages list to restrict the\ndeletion to the\n\nBug: https://bugs.libcamera.org/show_bug.cgi?id=26\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/base/thread.cpp | 43 +++++++++++++++++++++++++++--------\n 1 file changed, 33 insertions(+), 10 deletions(-)", "diff": "diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp\nindex 7f79115222e8..9eb488d46db8 100644\n--- a/src/libcamera/base/thread.cpp\n+++ b/src/libcamera/base/thread.cpp\n@@ -126,6 +126,11 @@ public:\n \t * \\brief Protects the \\ref list_\n \t */\n \tMutex mutex_;\n+\t/**\n+\t * \\brief The recursion level for recursive Thread::dispatchMessages()\n+\t * calls\n+\t */\n+\tunsigned int recursion_ = 0;\n };\n \n /**\n@@ -595,30 +600,34 @@ void Thread::removeMessages(Object *receiver)\n * Messages shall only be dispatched from the current thread, typically within\n * the thread from the run() function. Calling this function outside of the\n * thread results in undefined behaviour.\n+ *\n+ * This function is not thread-safe, but it may be called recursively in the\n+ * same thread from an object's message handler. It guarantees in all cases\n+ * delivery of messages in the order they have been posted.\n */\n void Thread::dispatchMessages(Message::Type type)\n {\n \tASSERT(data_ == ThreadData::current());\n \n+\t++data_->messages_.recursion_;\n+\n \tMutexLocker locker(data_->messages_.mutex_);\n \n \tstd::list<std::unique_ptr<Message>> &messages = data_->messages_.list_;\n \n-\tfor (auto iter = messages.begin(); iter != messages.end(); ) {\n-\t\tstd::unique_ptr<Message> &msg = *iter;\n-\n-\t\tif (!msg) {\n-\t\t\titer = data_->messages_.list_.erase(iter);\n+\tfor (std::unique_ptr<Message> &msg : messages) {\n+\t\tif (!msg)\n \t\t\tcontinue;\n-\t\t}\n \n-\t\tif (type != Message::Type::None && msg->type() != type) {\n-\t\t\t++iter;\n+\t\tif (type != Message::Type::None && msg->type() != type)\n \t\t\tcontinue;\n-\t\t}\n \n+\t\t/*\n+\t\t * Move the message, setting the entry in the list to null. It\n+\t\t * will cause recursive calls to ignore the entry, and the erase\n+\t\t * loop at the end of the function to delete it from the list.\n+\t\t */\n \t\tstd::unique_ptr<Message> message = std::move(msg);\n-\t\titer = data_->messages_.list_.erase(iter);\n \n \t\tObject *receiver = message->receiver_;\n \t\tASSERT(data_ == receiver->thread()->data_);\n@@ -629,6 +638,20 @@ void Thread::dispatchMessages(Message::Type type)\n \t\tmessage.reset();\n \t\tlocker.lock();\n \t}\n+\n+\t/*\n+\t * If the recursion level is 0, erase all null messages in the list. We\n+\t * can't do so during recursion, as it would invalidate the iterator of\n+\t * the outer calls.\n+\t */\n+\tif (!--data_->messages_.recursion_) {\n+\t\tfor (auto iter = messages.begin(); iter != messages.end(); ) {\n+\t\t\tif (!*iter)\n+\t\t\t\titer = messages.erase(iter);\n+\t\t\telse\n+\t\t\t\t++iter;\n+\t\t}\n+\t}\n }\n \n /**\n", "prefixes": [ "libcamera-devel", "3/3" ] }