[{"id":28560,"web_url":"https://patchwork.libcamera.org/comment/28560/","msgid":"<87wms15bqw.fsf@redhat.com>","date":"2024-01-22T20:55:35","subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n> Several functions in libcamera classes are marked as thread-bound,\n> restricting the contexts in which those functions can be called. There\n> is no infrastructure to enfore these restrictions, causing difficult to\n ^^^^^^\nenforce\n\n> debug race conditions when they are not met by callers.\n>\n> As a first step to solve this, add an assertThreadBound() protected\n> function to the Object class to test if the calling thread context is\n> valid, and use it in member functions of Object subclasses marked as\n> thread-bound. This replaces manual tests in a few locations.\n>\n> The thread-bound member functions of classes that do not inherit from\n> Object are not checked, and neither are the functions of classes marked\n> as thread-bound at the class level. These issue should be addressed in\n> the future.\n>\n> Signed-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(-)\n>\n> diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n> index 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;\n> diff --git a/src/libcamera/base/event_notifier.cpp b/src/libcamera/base/event_notifier.cpp\n> index 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\nIs it OK to continue with libcamera execution when the requested action is not\nfulfilled here? Can a wrong state of EventNotifier lead to serious trouble and\ndamages worse than exiting the program?\n\nThe same question for the other checks below.\n\n> \n> \tif (enabled_ == enable) return;\n> \n> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> index 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\nOr maybe simply exiting here rather than leaving the decision on the callers?\n\n(There can be different answers for \"in this patch for now\" and \"in future once\nmore tested\".)\n\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;\n> diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp\n> index 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> }","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 38C46C323E\n\tfor ;\n\tMon, 22 Jan 2024 20:55:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4FD06291F;\n\tMon, 22 Jan 2024 21:55:43 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DED1161D30\n\tfor ;\n\tMon, 22 Jan 2024 21:55:41 +0100 (CET)","from mail-lf1-f70.google.com (mail-lf1-f70.google.com\n\t[209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-656-fs-_DIPKO_WziEUczbRoOA-1; Mon, 22 Jan 2024 15:55:39 -0500","by mail-lf1-f70.google.com with SMTP id\n\t2adb3069b0e04-50e411af4e6so2546907e87.2\n\tfor ;\n\tMon, 22 Jan 2024 12:55:39 -0800 (PST)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\th16-20020a170906399000b00a28a66028bcsm13719367eje.91.2024.01.22.12.55.36\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 22 Jan 2024 12:55:36 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Acg2TTDz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1705956940;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=wHIaEi2mKi60ip+s0lGvcVuoOKkPJITB2AFchSg/vuQ=;\n\tb=Acg2TTDzTPnxPfhQWwK+LzaYNxQswYIip27KvXiazTpspi2qQqGN9TmX5Ach07XN1XjJhy\n\tD38KwfEwBzlvzaLKKbkcDnq6ID1xtuMiVettF/PgsV1d2tveN+A3f5xHn3FSKDZxqonu4q\n\tX5sas3uxajO3OEeyfluChgndwXSJG8Y=","X-MC-Unique":"fs-_DIPKO_WziEUczbRoOA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1705956937; x=1706561737;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=wHIaEi2mKi60ip+s0lGvcVuoOKkPJITB2AFchSg/vuQ=;\n\tb=gL6iEwfQ84aT9zLoUeSJQ66Tlg0COjveBvFczBLKXF3cPpI8W0pZjCu4Dgfk5H0tVk\n\tlOxIAdU4SWHm608P+/FxCohD1RadnsT02kPPDihaK6BG7S5XAdsqPS8jwWLzecKDCTTp\n\tL1taFSFO6Gmad5N9jKBi+WfTJltjAsm0DsSYltxpCVC5tnu1KUepZSYhIghLB+WjXTnH\n\tqCWqGJnW7UOsdZd4fl8mSFvhGOO9ZyjMnLQ2jCZ7EsUu1Wijkl+qL3GuXYR5zpScMi9+\n\tsh5+6n5V2P+WSRpU264syf8b2+esy92OhCwT/ws/qHfeEO6plAPoIzM/E4HuLnFi6MKV\n\tgBJw==","X-Gm-Message-State":"AOJu0YxXs+i9ylxfap9UBu5C/9n+o+qGceCq56a059yqgXq1Xbc8KdIb\n\tQ2lHznrDALyAcub5y7mVApq7xUBMYXP2Rf056EvdCwilCPLpMfK+MzvrN3c3CnFbyWmOcjHiFVD\n\thftgEhKn+zE6VFDydyboF/1lLxuVTxUdC8/CrwmUDpvadyxzfHZHG/j9at4SDVAYTnlnrwpisbj\n\tLol9k+KkbUU4a0XIP1seuexXslLR57QwJ8Kyg4JmRD60ayTf/9X2rpYPg=","X-Received":["by 2002:a05:6512:280d:b0:50e:a08d:174 with SMTP id\n\tcf13-20020a056512280d00b0050ea08d0174mr2581537lfb.95.1705956937232; \n\tMon, 22 Jan 2024 12:55:37 -0800 (PST)","by 2002:a05:6512:280d:b0:50e:a08d:174 with SMTP id\n\tcf13-20020a056512280d00b0050ea08d0174mr2581533lfb.95.1705956936795; \n\tMon, 22 Jan 2024 12:55:36 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHEwT0kkKWcCkpwZkLdZckgLBWGvHOiJ+j9J78+b0N97+mmZNmuKJuIHBaUtJHFmiSp++LoUw==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","In-Reply-To":"<20240121035948.4226-13-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Sun, 21 Jan 2024 05:59:48 +0200\")","References":"<20240121035948.4226-1-laurent.pinchart@ideasonboard.com>\n\t<20240121035948.4226-13-laurent.pinchart@ideasonboard.com>","Date":"Mon, 22 Jan 2024 21:55:35 +0100","Message-ID":"<87wms15bqw.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28567,"web_url":"https://patchwork.libcamera.org/comment/28567/","msgid":"<20240122234717.GB10843@pendragon.ideasonboard.com>","date":"2024-01-22T23:47:17","subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jan 22, 2024 at 09:55:35PM +0100, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> \n> > Several functions in libcamera classes are marked as thread-bound,\n> > restricting the contexts in which those functions can be called. There\n> > is no infrastructure to enfore these restrictions, causing difficult to\n> ^^^^^^\n> enforce\n> \n> > debug race conditions when they are not met by callers.\n> >\n> > As a first step to solve this, add an assertThreadBound() protected\n> > function to the Object class to test if the calling thread context is\n> > valid, and use it in member functions of Object subclasses marked as\n> > thread-bound. This replaces manual tests in a few locations.\n> >\n> > The thread-bound member functions of classes that do not inherit from\n> > Object are not checked, and neither are the functions of classes marked\n> > as thread-bound at the class level. These issue should be addressed in\n> > the future.\n> >\n> > Signed-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(-)\n> >\n> > diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n> > index 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;\n> > diff --git a/src/libcamera/base/event_notifier.cpp b/src/libcamera/base/event_notifier.cpp\n> > index 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> \n> Is it OK to continue with libcamera execution when the requested action is not\n> fulfilled here? Can a wrong state of EventNotifier lead to serious trouble and\n> damages worse than exiting the program?\n> \n> The same question for the other checks below.\n\nIn debug builds we certainly want to fail very loudly. In release\nbuilds, however, I don't think exiting is a reasonable option. I expect\nlibcamera to be run as a service (underneath pipewire) in quite a lot of\nuse cases, and taking pipewire down would be Bad (TM). I think we should\ninstead propagate errors up the call chain all the way to the\napplication, but not in this patch series.\n\n> > \tif (enabled_ == enable) return;\n> > \n> > diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> > index 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> Or maybe simply exiting here rather than leaving the decision on the callers?\n> \n> (There can be different answers for \"in this patch for now\" and \"in future once\n> more tested\".)\n> \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;\n> > diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp\n> > index 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> > }","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 659E1BDB1C\n\tfor ;\n\tMon, 22 Jan 2024 23:47:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B58E62948;\n\tTue, 23 Jan 2024 00:47:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8AA27628B7\n\tfor ;\n\tTue, 23 Jan 2024 00:47:13 +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 258D7541;\n\tTue, 23 Jan 2024 00:46:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oPclEFaQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705967160;\n\tbh=fk+gp+leDpjxieR4xeC33RpmHf7K+UWNic0EraVzO7Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oPclEFaQ5zNcjcTArwa9qFY+V/oxPJez1Z6y28A0+2upMdYKrQCoA5dQsvuuOIu/4\n\tdAbep7k5Iu38X7+U7aQptTIs2cuJUrj3e70FVLGv4oYY10ZQFmDovIO5N7IowsHujf\n\teoS8Qt8THzQhUdlkkdjIngPSJetV0vh/OvobvZ18=","Date":"Tue, 23 Jan 2024 01:47:17 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","Message-ID":"<20240122234717.GB10843@pendragon.ideasonboard.com>","References":"<20240121035948.4226-1-laurent.pinchart@ideasonboard.com>\n\t<20240121035948.4226-13-laurent.pinchart@ideasonboard.com>\n\t<87wms15bqw.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87wms15bqw.fsf@redhat.com>","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28582,"web_url":"https://patchwork.libcamera.org/comment/28582/","msgid":"<87il3kb84v.fsf@redhat.com>","date":"2024-01-23T11:29:20","subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n> On Mon, Jan 22, 2024 at 09:55:35PM +0100, Milan Zamazal wrote:\n>> Laurent Pinchart writes:\n>> \n>\n>> > Several functions in libcamera classes are marked as thread-bound,\n>> > restricting the contexts in which those functions can be called. There\n>> > is no infrastructure to enfore these restrictions, causing difficult to\n>> ^^^^^^\n>> enforce\n>> \n>> > debug race conditions when they are not met by callers.\n>> >\n>> > As a first step to solve this, add an assertThreadBound() protected\n>> > function to the Object class to test if the calling thread context is\n>> > valid, and use it in member functions of Object subclasses marked as\n>> > thread-bound. This replaces manual tests in a few locations.\n>> >\n>> > The thread-bound member functions of classes that do not inherit from\n>> > Object are not checked, and neither are the functions of classes marked\n>> > as thread-bound at the class level. These issue should be addressed in\n>> > the future.\n>> >\n>> > Signed-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(-)\n>> >\n>> > diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n>> > index 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;\n>> > diff --git a/src/libcamera/base/event_notifier.cpp b/src/libcamera/base/event_notifier.cpp\n>> > index 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>> \n>> Is it OK to continue with libcamera execution when the requested action is not\n>> fulfilled here? Can a wrong state of EventNotifier lead to serious trouble and\n>> damages worse than exiting the program?\n>> \n>> The same question for the other checks below.\n>\n> In debug builds we certainly want to fail very loudly. In release\n> builds, however, I don't think exiting is a reasonable option. I expect\n> libcamera to be run as a service (underneath pipewire) in quite a lot of\n> use cases, and taking pipewire down would be Bad (TM). \n\nOTOH such bugs can lead to freezes. Will it then freeze also pipewire or not?\nIf yes then freeze (it doesn't work and nothing happens) may be worse than\ngoing down (there is a chance for autorestart). But yes, this is just one of\nthe possible scenarios and trying to keep the service running some way may make\nsense. I'm not strongly convinced to either side, so I'm OK with it as it is.\n\n> I think we should instead propagate errors up the call chain all the way to\n> the application, \n\nOK.\n\n> but not in this patch series.\n\nYes, I understand this will be a non-trivial amount of work.\n\nSo for now:\n\nReviewed-by: Milan Zamazal \n\n>> > \tif (enabled_ == enable) return;\n>> > \n>> > diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n>> > index 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>> Or maybe simply exiting here rather than leaving the decision on the callers?\n>> \n>> (There can be different answers for \"in this patch for now\" and \"in future once\n>> more tested\".)\n>> \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;\n>> > diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp\n>> > index 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>> > }","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 E9E8BBDC71\n\tfor ;\n\tTue, 23 Jan 2024 11:29:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2240B62944;\n\tTue, 23 Jan 2024 12:29:28 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D34C628E9\n\tfor ;\n\tTue, 23 Jan 2024 12:29:27 +0100 (CET)","from mail-qv1-f71.google.com (mail-qv1-f71.google.com\n\t[209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-228-KZCJNwp0OZ-Zm9aPF3JmAw-1; Tue, 23 Jan 2024 06:29:24 -0500","by mail-qv1-f71.google.com with SMTP id\n\t6a1803df08f44-68694f9e037so19377166d6.2\n\tfor ;\n\tTue, 23 Jan 2024 03:29:24 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tnh12-20020a056214390c00b0068174386320sm3380151qvb.19.2024.01.23.03.29.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Jan 2024 03:29:21 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"fYt6Rv8y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1706009366;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=I9p/THCxGgvbhf0mBIlXJkxFpDVmby6kO5hbjaZeaOI=;\n\tb=fYt6Rv8yY4kbpje0NZcMYnSKnBLV3Um2b5+BnJlIXSwBbjqtu9BXUN8rGeIfzKKMHS2aZr\n\tHkaUttkxACC0u+LoA4BDlQ59Udb2anYWlxgqETWgt748ka95oJM9z/Sypk6IRrbm1Op5q6\n\tdziMh4+ldl6kR3BaA7l9qkzHTqg66TU=","X-MC-Unique":"KZCJNwp0OZ-Zm9aPF3JmAw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706009363; x=1706614163;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=I9p/THCxGgvbhf0mBIlXJkxFpDVmby6kO5hbjaZeaOI=;\n\tb=CzJPGC/gTLcvyoj/Rt6eLiZw9U2TlOqheCyIN2b+VrXT/3cSzjgsBa63vFPwXH4fQ0\n\t8yOR4aihv2O1kvR8UPEmyfAHyO6N1KeskFgk26mdr4lULIdGq+KYOxaVwF38vlb+jfgT\n\tzLOt8flGk4mSk9Bq0rP2bcfZaDQ/LFbgX/8oyv+MBDfe5FeJYMZlVJbfYHfx7YpaYsBr\n\tphSbNoo6Qqrs+HZ7+LYPmjzdHJ8T5xbNdf6ibIvUahDgyqXykUwgBeFpY2r5HC+2LKI4\n\tEeRLfGPBFIJa23iHDVemoGJoQogaD1/CJS39nAOqdHimEwWG3OcQnT63GImf4E3w+E0z\n\tOrLg==","X-Gm-Message-State":"AOJu0Yzg8LCwUc1MmqbSZK9pigrH0ar3LOxGllnKAG6WVXY2nsplhDV9\n\teXI6YAih8IPGoWbOHq52xMHkuh5uYn+15UuqCi9o5bK/h6MXxK4FOWMXHaqU7bGz1HsV4nio4wc\n\t4gTlP6Sk2HGis6KurS+N/KQ5AaQFTmy3CbR1ZS30uDxA3HEe9tY7CBvj3Hn47+O7yMNqRCxPg3G\n\tF/UoEyofoIHCyaWMIU3sDlFtMzQZUjFO7hs5lHpRNEwytFwq15YVfBU50=","X-Received":["by 2002:a05:6214:21ed:b0:685:6792:b522 with SMTP id\n\tp13-20020a05621421ed00b006856792b522mr751789qvj.57.1706009362933; \n\tTue, 23 Jan 2024 03:29:22 -0800 (PST)","by 2002:a05:6214:21ed:b0:685:6792:b522 with SMTP id\n\tp13-20020a05621421ed00b006856792b522mr751782qvj.57.1706009362518; \n\tTue, 23 Jan 2024 03:29:22 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IGdym898G9iSg1jevN/S6GvHBkZqrc8q8dBXAj5xswv1lvwJlF9hu9Qhj+LRcW0QM1qf+25Lw==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","In-Reply-To":"<20240122234717.GB10843@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Tue, 23 Jan 2024 01:47:17 +0200\")","References":"<20240121035948.4226-1-laurent.pinchart@ideasonboard.com>\n\t<20240121035948.4226-13-laurent.pinchart@ideasonboard.com>\n\t<87wms15bqw.fsf@redhat.com>\n\t<20240122234717.GB10843@pendragon.ideasonboard.com>","Date":"Tue, 23 Jan 2024 12:29:20 +0100","Message-ID":"<87il3kb84v.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28587,"web_url":"https://patchwork.libcamera.org/comment/28587/","msgid":"<20240123122322.GJ22880@pendragon.ideasonboard.com>","date":"2024-01-23T12:23:22","subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 23, 2024 at 12:29:20PM +0100, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> > On Mon, Jan 22, 2024 at 09:55:35PM +0100, Milan Zamazal wrote:\n> >> Laurent Pinchart writes:\n> >\n> >> > Several functions in libcamera classes are marked as thread-bound,\n> >> > restricting the contexts in which those functions can be called. There\n> >> > is no infrastructure to enfore these restrictions, causing difficult to\n> >> ^^^^^^\n> >> enforce\n> >> \n> >> > debug race conditions when they are not met by callers.\n> >> >\n> >> > As a first step to solve this, add an assertThreadBound() protected\n> >> > function to the Object class to test if the calling thread context is\n> >> > valid, and use it in member functions of Object subclasses marked as\n> >> > thread-bound. This replaces manual tests in a few locations.\n> >> >\n> >> > The thread-bound member functions of classes that do not inherit from\n> >> > Object are not checked, and neither are the functions of classes marked\n> >> > as thread-bound at the class level. These issue should be addressed in\n> >> > the future.\n> >> >\n> >> > Signed-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(-)\n> >> >\n> >> > diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n> >> > index 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;\n> >> > diff --git a/src/libcamera/base/event_notifier.cpp b/src/libcamera/base/event_notifier.cpp\n> >> > index 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> >> \n> >> Is it OK to continue with libcamera execution when the requested action is not\n> >> fulfilled here? Can a wrong state of EventNotifier lead to serious trouble and\n> >> damages worse than exiting the program?\n> >> \n> >> The same question for the other checks below.\n> >\n> > In debug builds we certainly want to fail very loudly. In release\n> > builds, however, I don't think exiting is a reasonable option. I expect\n> > libcamera to be run as a service (underneath pipewire) in quite a lot of\n> > use cases, and taking pipewire down would be Bad (TM). \n> \n> OTOH such bugs can lead to freezes. Will it then freeze also pipewire or not?\n> If yes then freeze (it doesn't work and nothing happens) may be worse than\n> going down (there is a chance for autorestart). But yes, this is just one of\n> the possible scenarios and trying to keep the service running some way may make\n> sense. I'm not strongly convinced to either side, so I'm OK with it as it is.\n\nIt may freeze in the sense that it will stop producing images, yes.\nCalls to libcamera shouldn't block though, and pipewire should be able\nto cleanly shut down the camera, and possibly restart it.\n\nOn the other hand, if we crash, we'll take down everything, including\naudio.\n\n> > I think we should instead propagate errors up the call chain all the way to\n> > the application, \n> \n> OK.\n> \n> > but not in this patch series.\n> \n> Yes, I understand this will be a non-trivial amount of work.\n> \n> So for now:\n> \n> Reviewed-by: Milan Zamazal \n> \n> >> > \tif (enabled_ == enable) return;\n> >> > \n> >> > diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> >> > index 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> >> Or maybe simply exiting here rather than leaving the decision on the callers?\n> >> \n> >> (There can be different answers for \"in this patch for now\" and \"in future once\n> >> more tested\".)\n> >> \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;\n> >> > diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp\n> >> > index 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> >> > }","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 CBDB5C323E\n\tfor ;\n\tTue, 23 Jan 2024 12:23:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C32F62944;\n\tTue, 23 Jan 2024 13:23:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EEBB0628E9\n\tfor ;\n\tTue, 23 Jan 2024 13:23:18 +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 399946E0;\n\tTue, 23 Jan 2024 13:22:05 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kKtoo41z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706012525;\n\tbh=we1/VqgXEyk6l1jXOel5TSWU44rUUAKYLz+wsvXPJIU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kKtoo41zziqBX/+yCqu6vRLR6m7PFlxSFKSzOPAW7Vz/OizGbLwri2Odv8S3PZsZ0\n\t9or/am3lijzrY389MGcp1+ZpcOmTGkakhXYNdyQ+kCwk/NuEhhQcYWmMLf3HK2gxsm\n\tbUsJ1pKERDcSSvB2VB5DC6t0ssuGbhp70IEmDWlw=","Date":"Tue, 23 Jan 2024 14:23:22 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Subject":"Re: [PATCH 12/12] libcamera: object: Add and use thread-bound\n\tassertion","Message-ID":"<20240123122322.GJ22880@pendragon.ideasonboard.com>","References":"<20240121035948.4226-1-laurent.pinchart@ideasonboard.com>\n\t<20240121035948.4226-13-laurent.pinchart@ideasonboard.com>\n\t<87wms15bqw.fsf@redhat.com>\n\t<20240122234717.GB10843@pendragon.ideasonboard.com>\n\t<87il3kb84v.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87il3kb84v.fsf@redhat.com>","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]