{"id":19433,"url":"https://patchwork.libcamera.org/api/patches/19433/?format=json","web_url":"https://patchwork.libcamera.org/patch/19433/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240121035948.4226-13-laurent.pinchart@ideasonboard.com>","date":"2024-01-21T03:59:48","name":"[libcamera-devel,12/12] libcamera: object: Add and use thread-bound assertion","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"6dfb60f9d8a124e7677a90aa7c0ed2b9b1f4a6bf","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19433/mbox/","series":[{"id":4149,"url":"https://patchwork.libcamera.org/api/series/4149/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4149","date":"2024-01-21T03:59:36","name":"libcamera: Hardening against thread race conditions","version":1,"mbox":"https://patchwork.libcamera.org/series/4149/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19433/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19433/checks/","tags":{},"headers":{"Return-Path":"","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 704CCC32C1\n\tfor ;\n\tSun, 21 Jan 2024 04:00:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C253062945;\n\tSun, 21 Jan 2024 05:00:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AC56B62951\n\tfor ;\n\tSun, 21 Jan 2024 05:00:01 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A5A9C138B;\n\tSun, 21 Jan 2024 04:58:49 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705809607;\n\tbh=uUewvC+Z7f7DorNRtypkfDnV1u4i5LCMuN15JkZJ6Oc=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=2qSGZTCHPpMCGzgte66cengSSh2/Ent5+BBDo4jnvFbrBS/TDGfkxlnOUMkshbe0k\n\trxicxRBhy8GLrvB0rdgyK6tNacCAMWN2nbbTn6c4nULAw2cj3OQ45I4XdutQ4YHxGc\n\tKUhL3EJI7o7ZH7sADvlAbk2plN1T+qzLuaOxNzq+PHAuu/QFNC8waUKltIRJDEjMKF\n\t2yn3rqg7qNGEFRI2082BkesDWjpgjLjRFs1/PWyfvhHB7uhjrP39KBV4YRYbu5svVb\n\tekGr4JmYtJjcDQtfFl2k/XMqVGoNBFIu0sNuRrdqWJR03LqMHZzEgwwZqali6qZTRa\n\torpAkip2LlFZw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705809529;\n\tbh=uUewvC+Z7f7DorNRtypkfDnV1u4i5LCMuN15JkZJ6Oc=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=UoqwGTGDtKMQVQFa6tl9WGP8A7Pm3cXulOQWFwMcTEfANeXMKQCHdX5ychd8N4Q0s\n\t7PElyuj0p9rFCaFzIbeYN7Ih9h2n3ofnqyi1EmFk8isaDlRXvf0Eti0kpAjK3ogBTG\n\t2NESBW1feYC3LvOHuH1isE82MZKlLd6aLMLg9wqU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UoqwGTGD\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Sun, 21 Jan 2024 05:59:48 +0200","Message-ID":"<20240121035948.4226-13-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.43.0","In-Reply-To":"<20240121035948.4226-1-laurent.pinchart@ideasonboard.com>","References":"<20240121035948.4226-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 12/12] libcamera: object: Add and use\n\tthread-bound assertion","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Several functions in libcamera classes are marked as thread-bound,\nrestricting the contexts in which those functions can be called. There\nis no infrastructure to enfore these restrictions, causing difficult to\ndebug race conditions when they are not met by callers.\n\nAs a first step to solve this, add an assertThreadBound() protected\nfunction to the Object class to test if the calling thread context is\nvalid, and use it in member functions of Object subclasses marked as\nthread-bound. This replaces manual tests in a few locations.\n\nThe thread-bound member functions of classes that do not inherit from\nObject are not checked, and neither are the functions of classes marked\nas thread-bound at the class level. These issue should be addressed in\nthe future.\n\nSigned-off-by: Laurent Pinchart \n---\n include/libcamera/base/object.h | 2 ++\n src/libcamera/base/event_notifier.cpp | 6 +++++\n src/libcamera/base/object.cpp | 32 ++++++++++++++++++++++++++-\n src/libcamera/base/timer.cpp | 10 +++------\n 4 files changed, 42 insertions(+), 8 deletions(-)","diff":"diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\nindex 933336361155..cb7e0a132be2 100644\n--- a/include/libcamera/base/object.h\n+++ b/include/libcamera/base/object.h\n@@ -49,6 +49,8 @@ public:\n protected:\n \tvirtual void message(Message *msg);\n \n+\tbool assertThreadBound(const char *message);\n+\n private:\n \tfriend class SignalBase;\n \tfriend class Thread;\ndiff --git a/src/libcamera/base/event_notifier.cpp b/src/libcamera/base/event_notifier.cpp\nindex fd93c0878c6f..a519aec38efb 100644\n--- a/src/libcamera/base/event_notifier.cpp\n+++ b/src/libcamera/base/event_notifier.cpp\n@@ -8,6 +8,7 @@\n #include \n \n #include \n+#include \n #include \n #include \n \n@@ -20,6 +21,8 @@\n \n namespace libcamera {\n \n+LOG_DECLARE_CATEGORY(Event)\n+\n /**\n * \\class EventNotifier\n * \\brief Notify of activity on a file descriptor\n@@ -104,6 +107,9 @@ EventNotifier::~EventNotifier()\n */\n void EventNotifier::setEnabled(bool enable)\n {\n+\tif (!assertThreadBound(\"EventNotifier can't be enabled from another thread\"))\n+\t\treturn;\n+\n \tif (enabled_ == enable)\n \t\treturn;\n \ndiff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\nindex 8af0337f5448..d98a9157773e 100644\n--- a/src/libcamera/base/object.cpp\n+++ b/src/libcamera/base/object.cpp\n@@ -224,6 +224,35 @@ void Object::message(Message *msg)\n \t}\n }\n \n+/**\n+ * \\fn Object::assertThreadBound()\n+ * \\brief Check if the caller complies with thread-bound constraints\n+ * \\param[in] message The message to be printed on error\n+ *\n+ * This function verifies the calling constraints required by the \\threadbound\n+ * definition. It shall be called at the beginning of member functions of an\n+ * Object subclass that are explicitly marked as thread-bound in their\n+ * documentation.\n+ *\n+ * If the thread-bound constraints are not met, the function prints \\a message\n+ * as an error message. For debug builds, it additionally causes an assertion\n+ * error.\n+ *\n+ * \\todo Verify the thread-bound requirements for functions marked as\n+ * thread-bound at the class level.\n+ *\n+ * \\return True if the call is thread-bound compliant, false otherwise\n+ */\n+bool Object::assertThreadBound(const char *message)\n+{\n+\tif (Thread::current() == thread_)\n+\t\treturn true;\n+\n+\tLOG(Object, Error) << message;\n+\tASSERT(false);\n+\treturn false;\n+}\n+\n /**\n * \\fn R Object::invokeMethod()\n * \\brief Invoke a method asynchronously on an Object instance\n@@ -275,7 +304,8 @@ void Object::message(Message *msg)\n */\n void Object::moveToThread(Thread *thread)\n {\n-\tASSERT(Thread::current() == thread_);\n+\tif (!assertThreadBound(\"Object can't be moved from another thread\"))\n+\t\treturn;\n \n \tif (thread_ == thread)\n \t\treturn;\ndiff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp\nindex 74b060af32ff..24dbf1e892c3 100644\n--- a/src/libcamera/base/timer.cpp\n+++ b/src/libcamera/base/timer.cpp\n@@ -85,10 +85,8 @@ void Timer::start(std::chrono::milliseconds duration)\n */\n void Timer::start(std::chrono::steady_clock::time_point deadline)\n {\n-\tif (Thread::current() != thread()) {\n-\t\tLOG(Timer, Error) << \"Timer \" << this << \" << can't be started from another thread\";\n+\tif (!assertThreadBound(\"Timer can't be started from another thread\"))\n \t\treturn;\n-\t}\n \n \tdeadline_ = deadline;\n \n@@ -114,13 +112,11 @@ void Timer::start(std::chrono::steady_clock::time_point deadline)\n */\n void Timer::stop()\n {\n-\tif (!isRunning())\n+\tif (!assertThreadBound(\"Timer can't be stopped from another thread\"))\n \t\treturn;\n \n-\tif (Thread::current() != thread()) {\n-\t\tLOG(Timer, Error) << \"Timer \" << this << \" can't be stopped from another thread\";\n+\tif (!isRunning())\n \t\treturn;\n-\t}\n \n \tunregisterTimer();\n }\n","prefixes":["libcamera-devel","12/12"]}