From patchwork Sun Jan 21 03:59:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19433 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 704CCC32C1 for ; Sun, 21 Jan 2024 04:00:08 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C253062945; Sun, 21 Jan 2024 05:00:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809607; bh=uUewvC+Z7f7DorNRtypkfDnV1u4i5LCMuN15JkZJ6Oc=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=2qSGZTCHPpMCGzgte66cengSSh2/Ent5+BBDo4jnvFbrBS/TDGfkxlnOUMkshbe0k rxicxRBhy8GLrvB0rdgyK6tNacCAMWN2nbbTn6c4nULAw2cj3OQ45I4XdutQ4YHxGc KUhL3EJI7o7ZH7sADvlAbk2plN1T+qzLuaOxNzq+PHAuu/QFNC8waUKltIRJDEjMKF 2yn3rqg7qNGEFRI2082BkesDWjpgjLjRFs1/PWyfvhHB7uhjrP39KBV4YRYbu5svVb ekGr4JmYtJjcDQtfFl2k/XMqVGoNBFIu0sNuRrdqWJR03LqMHZzEgwwZqali6qZTRa orpAkip2LlFZw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AC56B62951 for ; Sun, 21 Jan 2024 05:00:01 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="UoqwGTGD"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A5A9C138B; Sun, 21 Jan 2024 04:58:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809529; bh=uUewvC+Z7f7DorNRtypkfDnV1u4i5LCMuN15JkZJ6Oc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UoqwGTGDtKMQVQFa6tl9WGP8A7Pm3cXulOQWFwMcTEfANeXMKQCHdX5ychd8N4Q0s 7PElyuj0p9rFCaFzIbeYN7Ih9h2n3ofnqyi1EmFk8isaDlRXvf0Eti0kpAjK3ogBTG 2NESBW1feYC3LvOHuH1isE82MZKlLd6aLMLg9wqU= 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 Subject: [libcamera-devel] [PATCH 12/12] libcamera: object: Add and use thread-bound assertion X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Several functions in libcamera classes are marked as thread-bound, restricting the contexts in which those functions can be called. There is no infrastructure to enfore these restrictions, causing difficult to debug race conditions when they are not met by callers. As a first step to solve this, add an assertThreadBound() protected function to the Object class to test if the calling thread context is valid, and use it in member functions of Object subclasses marked as thread-bound. This replaces manual tests in a few locations. The thread-bound member functions of classes that do not inherit from Object are not checked, and neither are the functions of classes marked as thread-bound at the class level. These issue should be addressed in the future. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- include/libcamera/base/object.h | 2 ++ src/libcamera/base/event_notifier.cpp | 6 +++++ src/libcamera/base/object.cpp | 32 ++++++++++++++++++++++++++- src/libcamera/base/timer.cpp | 10 +++------ 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h index 933336361155..cb7e0a132be2 100644 --- a/include/libcamera/base/object.h +++ b/include/libcamera/base/object.h @@ -49,6 +49,8 @@ public: protected: virtual void message(Message *msg); + bool assertThreadBound(const char *message); + private: friend class SignalBase; friend class Thread; diff --git a/src/libcamera/base/event_notifier.cpp b/src/libcamera/base/event_notifier.cpp index fd93c0878c6f..a519aec38efb 100644 --- a/src/libcamera/base/event_notifier.cpp +++ b/src/libcamera/base/event_notifier.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -20,6 +21,8 @@ namespace libcamera { +LOG_DECLARE_CATEGORY(Event) + /** * \class EventNotifier * \brief Notify of activity on a file descriptor @@ -104,6 +107,9 @@ EventNotifier::~EventNotifier() */ void EventNotifier::setEnabled(bool enable) { + if (!assertThreadBound("EventNotifier can't be enabled from another thread")) + return; + if (enabled_ == enable) return; diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp index 8af0337f5448..d98a9157773e 100644 --- a/src/libcamera/base/object.cpp +++ b/src/libcamera/base/object.cpp @@ -224,6 +224,35 @@ void Object::message(Message *msg) } } +/** + * \fn Object::assertThreadBound() + * \brief Check if the caller complies with thread-bound constraints + * \param[in] message The message to be printed on error + * + * This function verifies the calling constraints required by the \threadbound + * definition. It shall be called at the beginning of member functions of an + * Object subclass that are explicitly marked as thread-bound in their + * documentation. + * + * If the thread-bound constraints are not met, the function prints \a message + * as an error message. For debug builds, it additionally causes an assertion + * error. + * + * \todo Verify the thread-bound requirements for functions marked as + * thread-bound at the class level. + * + * \return True if the call is thread-bound compliant, false otherwise + */ +bool Object::assertThreadBound(const char *message) +{ + if (Thread::current() == thread_) + return true; + + LOG(Object, Error) << message; + ASSERT(false); + return false; +} + /** * \fn R Object::invokeMethod() * \brief Invoke a method asynchronously on an Object instance @@ -275,7 +304,8 @@ void Object::message(Message *msg) */ void Object::moveToThread(Thread *thread) { - ASSERT(Thread::current() == thread_); + if (!assertThreadBound("Object can't be moved from another thread")) + return; if (thread_ == thread) return; diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp index 74b060af32ff..24dbf1e892c3 100644 --- a/src/libcamera/base/timer.cpp +++ b/src/libcamera/base/timer.cpp @@ -85,10 +85,8 @@ void Timer::start(std::chrono::milliseconds duration) */ void Timer::start(std::chrono::steady_clock::time_point deadline) { - if (Thread::current() != thread()) { - LOG(Timer, Error) << "Timer " << this << " << can't be started from another thread"; + if (!assertThreadBound("Timer can't be started from another thread")) return; - } deadline_ = deadline; @@ -114,13 +112,11 @@ void Timer::start(std::chrono::steady_clock::time_point deadline) */ void Timer::stop() { - if (!isRunning()) + if (!assertThreadBound("Timer can't be stopped from another thread")) return; - if (Thread::current() != thread()) { - LOG(Timer, Error) << "Timer " << this << " can't be stopped from another thread"; + if (!isRunning()) return; - } unregisterTimer(); }