[{"id":20873,"web_url":"https://patchwork.libcamera.org/comment/20873/","msgid":"<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>","date":"2021-11-11T16:38:35","subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Hiro,\n\nOn 10/29/21 9:44 AM, Hirokazu Honda wrote:\n> Mutex2 and MutexLocker2 are annotated by clang thread safety\n\n\nNot sure I like the naming here. but I don't have any better suggestion \nas of now, still contemplating on that.. maybe AMutex and AMutexLocker \nto denote they are annotated?\n\n> annotations. So we can add annotation to code where the classes\n> are used.\n> In the future, they will replace Mutex and MutexLocker.\n\n\nOk so I see a patch following, that does this replacement. I went into \nthe territory of thinking if this annotations can be used full libcamera \ncodebase. Can it be done (not as part of the series) but as a separate \nseries. I am not sure how much useful it would be since annotation is \nclang-only. Let's see.\n\nSo, the series really:\n\n         In the future, they will replace Mutex and MutexLocker in the \nandroid HAL layer.\n\n>\n> Signed-off-by: Hirokazu Honda \n> ---\n> include/libcamera/base/meson.build | 1 +\n> include/libcamera/base/mutex.h | 66 ++++++++++++++++++++++++++++++\n\n\nPlease refer to my comment on 1/6 about putting this in \ninclude/libcamera/internal, maybe that's a better place?\n\n> include/libcamera/base/thread.h | 5 +--\n> 3 files changed, 68 insertions(+), 4 deletions(-)\n> create mode 100644 include/libcamera/base/mutex.h\n>\n> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> index 1a71ce5a..37c4435a 100644\n> --- a/include/libcamera/base/meson.build\n> +++ b/include/libcamera/base/meson.build\n> @@ -13,6 +13,7 @@ libcamera_base_headers = files([\n> 'flags.h',\n> 'log.h',\n> 'message.h',\n> + 'mutex.h',\n> 'object.h',\n> 'private.h',\n> 'semaphore.h',\n> diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h\n> new file mode 100644\n> index 00000000..d130988e\n> --- /dev/null\n> +++ b/include/libcamera/base/mutex.h\n> @@ -0,0 +1,66 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * thread.h - Thread support\n\n\noops it should be mutex.h\n\n\nPatch itself looks good to me.\n\n> + */\n> +#ifndef __LIBCAMERA_BASE_MUTEX_H__\n> +#define __LIBCAMERA_BASE_MUTEX_H__\n> +\n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +using Mutex = std::mutex;\n> +using MutexLocker = std::unique_lock;\n> +\n> +class CAPABILITY(\"mutex\") Mutex2 final\n> +{\n> +public:\n> +\tvoid lock() ACQUIRE()\n> +\t{\n> +\t\tmutex_.lock();\n> +\t}\n> +\n> +\tvoid unlock() RELEASE()\n> +\t{\n> +\t\tmutex_.unlock();\n> +\t}\n> +\n> +\tstd::mutex &get() { return mutex_; }\n> +private:\n> +\tstd::mutex mutex_;\n> +};\n> +\n> +class SCOPED_CAPABILITY MutexLocker2 final\n> +{\n> +public:\n> +\tMutexLocker2(Mutex2 &mutex) ACQUIRE(mutex)\n> +\t\t: lock_(mutex.get())\n> +\t{\n> +\t}\n> +\n> +\t~MutexLocker2() RELEASE()\n> +\t{\n> +\t}\n> +\n> +\tvoid lock() ACQUIRE()\n> +\t{\n> +\t\tlock_.lock();\n> +\t}\n> +\n> +\tvoid unlock() RELEASE()\n> +\t{\n> +\t\tlock_.unlock();\n> +\t}\n> +\n> +\tstd::unique_lock &get() { return lock_; }\n> +private:\n> +\tstd::unique_lock lock_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_BASE_MUTEX_H__ */\n> diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h\n> index e0ca0aea..ae630563 100644\n> --- a/include/libcamera/base/thread.h\n> +++ b/include/libcamera/base/thread.h\n> @@ -8,13 +8,13 @@\n> #define __LIBCAMERA_BASE_THREAD_H__\n> \n> #include \n> -#include \n> #include \n> #include \n> \n> #include \n> \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -26,9 +26,6 @@ class Object;\n> class ThreadData;\n> class ThreadMain;\n> \n> -using Mutex = std::mutex;\n> -using MutexLocker = std::unique_lock;\n> -\n> class Thread\n> {\n> public:","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 EEF03BDB1C\n\tfor ;\n\tThu, 11 Nov 2021 16:38:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4CCE36035A;\n\tThu, 11 Nov 2021 17:38:42 +0100 (CET)","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 12B13600B5\n\tfor ;\n\tThu, 11 Nov 2021 17:38:41 +0100 (CET)","from [192.168.1.106] (unknown [103.251.226.254])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 00FAC556;\n\tThu, 11 Nov 2021 17:38:39 +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=\"FQ6p0/9j\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636648720;\n\tbh=yLKJCgEcS3/cZEifUrEsSEziJBYTqU7AS9iAPRcMA+w=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=FQ6p0/9j/4nl65OYPVxoYBnEaZX9btx+XS1FMmL5qEJX0vRqz1lwK2c5SxqLo5jtJ\n\t79TI6LTOHKynDqaSzn4jow/RUKudK2liHB3Bw+ZVzXRtADngAE6nZ9WHQI47Sr3KSd\n\t9AfTOFaOIEXhTl87gI8zMMoEyAt18sl9KNdPMkIQ=","To":"Hirokazu Honda , libcamera-devel@lists.libcamera.org","References":"<20211029041424.1430886-1-hiroh@chromium.org>\n\t<20211029041424.1430886-3-hiroh@chromium.org>","From":"Umang Jain ","Message-ID":"<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>","Date":"Thu, 11 Nov 2021 22:08:35 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20211029041424.1430886-3-hiroh@chromium.org>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20883,"web_url":"https://patchwork.libcamera.org/comment/20883/","msgid":"","date":"2021-11-11T22:52:46","subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Thu, Nov 11, 2021 at 10:08:35PM +0530, Umang Jain wrote:\n> On 10/29/21 9:44 AM, Hirokazu Honda wrote:\n> > Mutex2 and MutexLocker2 are annotated by clang thread safety\n> \n> Not sure I like the naming here. but I don't have any better suggestion \n> as of now, still contemplating on that.. maybe AMutex and AMutexLocker \n> to denote they are annotated?\n\nI'm also concerned by this. Can't we replace the current Mutex and\nMutexLocker with annotated versions, without having two implementations\n?\n\n> > annotations. So we can add annotation to code where the classes are used.\n> > In the future, they will replace Mutex and MutexLocker.\n> \n> Ok so I see a patch following, that does this replacement. I went into \n> the territory of thinking if this annotations can be used full libcamera \n> codebase. Can it be done (not as part of the series) but as a separate \n> series. I am not sure how much useful it would be since annotation is \n> clang-only. Let's see.\n> \n> So, the series really:\n> \n>         In the future, they will replace Mutex and MutexLocker in the \n> android HAL layer.\n> \n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > include/libcamera/base/meson.build | 1 +\n> > include/libcamera/base/mutex.h | 66 ++++++++++++++++++++++++++++++\n> \n> Please refer to my comment on 1/6 about putting this in \n> include/libcamera/internal, maybe that's a better place?\n\nI think base makes sense, as we already define Mutex and MutexLocker\nthere.\n\n> > include/libcamera/base/thread.h | 5 +--\n> > 3 files changed, 68 insertions(+), 4 deletions(-)\n> > create mode 100644 include/libcamera/base/mutex.h\n> >\n> > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> > index 1a71ce5a..37c4435a 100644\n> > --- a/include/libcamera/base/meson.build\n> > +++ b/include/libcamera/base/meson.build\n> > @@ -13,6 +13,7 @@ libcamera_base_headers = files([\n> > 'flags.h',\n> > 'log.h',\n> > 'message.h',\n> > + 'mutex.h',\n> > 'object.h',\n> > 'private.h',\n> > 'semaphore.h',\n> > diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h\n> > new file mode 100644\n> > index 00000000..d130988e\n> > --- /dev/null\n> > +++ b/include/libcamera/base/mutex.h\n> > @@ -0,0 +1,66 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Google Inc.\n> > + *\n> > + * thread.h - Thread support\n> \n> \n> oops it should be mutex.h\n> \n> \n> Patch itself looks good to me.\n> \n> > + */\n> > +#ifndef __LIBCAMERA_BASE_MUTEX_H__\n> > +#define __LIBCAMERA_BASE_MUTEX_H__\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +using Mutex = std::mutex;\n> > +using MutexLocker = std::unique_lock;\n> > +\n> > +class CAPABILITY(\"mutex\") Mutex2 final\n\nDocumentation is missing.\n\nThinking further about this class, given that libc++ annotates\nstd::mutex, do we a custom annotated class ? It would only be used when\ncompiling with clang and libstd++, which seems a bit of a cornercase.\n\n> > +{\n> > +public:\n\nThe std::mutex constructor is constexpr, I think we should do the same\nhere to replicate the same API.\n\n> > +\tvoid lock() ACQUIRE()\n> > +\t{\n> > +\t\tmutex_.lock();\n> > +\t}\n> > +\n> > +\tvoid unlock() RELEASE()\n> > +\t{\n> > +\t\tmutex_.unlock();\n> > +\t}\n\nWe also need try_lock().\n\n> > +\n> > +\tstd::mutex &get() { return mutex_; }\n\nMissing blank line.\n\nThis function is only used by MutexLocker2. I think a friend declaration\nwould be better here, to avoid exposing get().\n\n> > +private:\n> > +\tstd::mutex mutex_;\n> > +};\n> > +\n> > +class SCOPED_CAPABILITY MutexLocker2 final\n> > +{\n> > +public:\n> > +\tMutexLocker2(Mutex2 &mutex) ACQUIRE(mutex)\n> > +\t\t: lock_(mutex.get())\n> > +\t{\n> > +\t}\n> > +\n> > +\t~MutexLocker2() RELEASE()\n> > +\t{\n> > +\t}\n> > +\n> > +\tvoid lock() ACQUIRE()\n> > +\t{\n> > +\t\tlock_.lock();\n> > +\t}\n> > +\n> > +\tvoid unlock() RELEASE()\n> > +\t{\n> > +\t\tlock_.unlock();\n> > +\t}\n\nThere are more functions in the std::unique_lock API (including more\nconstructors), should we implement them too ?\n\n> > +\n> > +\tstd::unique_lock &get() { return lock_; }\n\nMissing blank line.\n\nThis function is only used to support std::condition_variable::wait(),\nwouldn't it be better to implement a ConditionVariable class that would\ntake a MutexLocker argument in wait() ?\n\n> > +private:\n> > +\tstd::unique_lock lock_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_BASE_MUTEX_H__ */\n> > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h\n> > index e0ca0aea..ae630563 100644\n> > --- a/include/libcamera/base/thread.h\n> > +++ b/include/libcamera/base/thread.h\n> > @@ -8,13 +8,13 @@\n> > #define __LIBCAMERA_BASE_THREAD_H__\n> > \n> > #include \n> > -#include \n> > #include \n> > #include \n> > \n> > #include \n> > \n> > #include \n> > +#include \n> > #include \n> > #include \n> > \n> > @@ -26,9 +26,6 @@ class Object;\n> > class ThreadData;\n> > class ThreadMain;\n> > \n> > -using Mutex = std::mutex;\n> > -using MutexLocker = std::unique_lock;\n> > -\n> > class Thread\n> > {\n> > public:","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 99E9EBF415\n\tfor ;\n\tThu, 11 Nov 2021 22:53:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E68F36035F;\n\tThu, 11 Nov 2021 23:53:09 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B2279600B5\n\tfor ;\n\tThu, 11 Nov 2021 23:53:08 +0100 (CET)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 33341556;\n\tThu, 11 Nov 2021 23:53:08 +0100 (CET)"],"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=\"b7MvKkYp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636671188;\n\tbh=/TcYZ4SqCydAyjvG3YB+f3gl36/5upDGgCeRj2hgCiw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b7MvKkYp3oS6mLk1063CP3F/Uqj/LY323e4GjIM0LHB+yJsfoFifEs5zu7CKpCtIm\n\tc4bb1N+bEEdqqWvJz+aBCsBlHvWKLLf0XTNcmuF4RlonJOqR56KOcYRPLQ7e77lB3m\n\tmvNTqvaMXQl29kTcPISHMcnTioduqFat1GAf7iP8=","Date":"Fri, 12 Nov 2021 00:52:46 +0200","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20211029041424.1430886-1-hiroh@chromium.org>\n\t<20211029041424.1430886-3-hiroh@chromium.org>\n\t<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","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":20895,"web_url":"https://patchwork.libcamera.org/comment/20895/","msgid":"","date":"2021-11-12T07:20:01","subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent and Umang,\n\n\nOn Fri, Nov 12, 2021 at 7:53 AM Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Thu, Nov 11, 2021 at 10:08:35PM +0530, Umang Jain wrote:\n> > On 10/29/21 9:44 AM, Hirokazu Honda wrote:\n> > > Mutex2 and MutexLocker2 are annotated by clang thread safety\n> >\n> > Not sure I like the naming here. but I don't have any better suggestion\n> > as of now, still contemplating on that.. maybe AMutex and AMutexLocker\n> > to denote they are annotated?\n>\n> I'm also concerned by this. Can't we replace the current Mutex and\n> MutexLocker with annotated versions, without having two implementations\n> ?\n>\n\nThe problem is condition_variable. It takes MutexLocker\n(std::unique_lock) on wait.\nHowever, the new MutexLocker is not std::unique_lock. I avoid this\nproblem by MutexLocker2::get().\nI found [1] MutexLocker derives std::unique_lock, but not sure if it is ok.\n[1] https://stackoverflow.com/questions/40468897/clang-thread-safety-with-stdcondition-variable\n\n\n> > > annotations. So we can add annotation to code where the classes are used.\n> > > In the future, they will replace Mutex and MutexLocker.\n> >\n> > Ok so I see a patch following, that does this replacement. I went into\n> > the territory of thinking if this annotations can be used full libcamera\n> > codebase. Can it be done (not as part of the series) but as a separate\n> > series. I am not sure how much useful it would be since annotation is\n> > clang-only. Let's see.\n> >\n> > So, the series really:\n> >\n> > In the future, they will replace Mutex and MutexLocker in the\n> > android HAL layer.\n> >\n> > > Signed-off-by: Hirokazu Honda \n> > > ---\n> > > include/libcamera/base/meson.build | 1 +\n> > > include/libcamera/base/mutex.h | 66 ++++++++++++++++++++++++++++++\n> >\n> > Please refer to my comment on 1/6 about putting this in\n> > include/libcamera/internal, maybe that's a better place?\n>\n> I think base makes sense, as we already define Mutex and MutexLocker\n> there.\n>\n> > > include/libcamera/base/thread.h | 5 +--\n> > > 3 files changed, 68 insertions(+), 4 deletions(-)\n> > > create mode 100644 include/libcamera/base/mutex.h\n> > >\n> > > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> > > index 1a71ce5a..37c4435a 100644\n> > > --- a/include/libcamera/base/meson.build\n> > > +++ b/include/libcamera/base/meson.build\n> > > @@ -13,6 +13,7 @@ libcamera_base_headers = files([\n> > > 'flags.h',\n> > > 'log.h',\n> > > 'message.h',\n> > > + 'mutex.h',\n> > > 'object.h',\n> > > 'private.h',\n> > > 'semaphore.h',\n> > > diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h\n> > > new file mode 100644\n> > > index 00000000..d130988e\n> > > --- /dev/null\n> > > +++ b/include/libcamera/base/mutex.h\n> > > @@ -0,0 +1,66 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2021, Google Inc.\n> > > + *\n> > > + * thread.h - Thread support\n> >\n> >\n> > oops it should be mutex.h\n> >\n> >\n> > Patch itself looks good to me.\n> >\n> > > + */\n> > > +#ifndef __LIBCAMERA_BASE_MUTEX_H__\n> > > +#define __LIBCAMERA_BASE_MUTEX_H__\n> > > +\n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +using Mutex = std::mutex;\n> > > +using MutexLocker = std::unique_lock;\n> > > +\n> > > +class CAPABILITY(\"mutex\") Mutex2 final\n>\n> Documentation is missing.\n>\n> Thinking further about this class, given that libc++ annotates\n> std::mutex, do we a custom annotated class ? It would only be used when\n> compiling with clang and libstd++, which seems a bit of a cornercase.\n>\n\nYeah, once libc++ annotats std::unique_lock, I would let Mutex be\nstd::unique_lock if libc++ and otherwise this custom class.\n\n> > > +{\n> > > +public:\n>\n> The std::mutex constructor is constexpr, I think we should do the same\n> here to replicate the same API.\n>\n> > > + void lock() ACQUIRE()\n> > > + {\n> > > + mutex_.lock();\n> > > + }\n> > > +\n> > > + void unlock() RELEASE()\n> > > + {\n> > > + mutex_.unlock();\n> > > + }\n>\n> We also need try_lock().\n>\n> > > +\n> > > + std::mutex &get() { return mutex_; }\n>\n> Missing blank line.\n>\n> This function is only used by MutexLocker2. I think a friend declaration\n> would be better here, to avoid exposing get().\n>\n> > > +private:\n> > > + std::mutex mutex_;\n> > > +};\n> > > +\n> > > +class SCOPED_CAPABILITY MutexLocker2 final\n> > > +{\n> > > +public:\n> > > + MutexLocker2(Mutex2 &mutex) ACQUIRE(mutex)\n> > > + : lock_(mutex.get())\n> > > + {\n> > > + }\n> > > +\n> > > + ~MutexLocker2() RELEASE()\n> > > + {\n> > > + }\n> > > +\n> > > + void lock() ACQUIRE()\n> > > + {\n> > > + lock_.lock();\n> > > + }\n> > > +\n> > > + void unlock() RELEASE()\n> > > + {\n> > > + lock_.unlock();\n> > > + }\n>\n> There are more functions in the std::unique_lock API (including more\n> constructors), should we implement them too ?\n>\n> > > +\n> > > + std::unique_lock &get() { return lock_; }\n>\n> Missing blank line.\n>\n> This function is only used to support std::condition_variable::wait(),\n> wouldn't it be better to implement a ConditionVariable class that would\n> take a MutexLocker argument in wait() ?\n>\n\nI wondered about it. If we don't like MutexLocker derives\nConditionVariable, then I would try it.\n\n-Hiro\n\n> > > +private:\n> > > + std::unique_lock lock_;\n> > > +};\n> > > +\n> > > +} /* namespace libcamera */\n> > > +\n> > > +#endif /* __LIBCAMERA_BASE_MUTEX_H__ */\n> > > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h\n> > > index e0ca0aea..ae630563 100644\n> > > --- a/include/libcamera/base/thread.h\n> > > +++ b/include/libcamera/base/thread.h\n> > > @@ -8,13 +8,13 @@\n> > > #define __LIBCAMERA_BASE_THREAD_H__\n> > >\n> > > #include \n> > > -#include \n> > > #include \n> > > #include \n> > >\n> > > #include \n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > >\n> > > @@ -26,9 +26,6 @@ class Object;\n> > > class ThreadData;\n> > > class ThreadMain;\n> > >\n> > > -using Mutex = std::mutex;\n> > > -using MutexLocker = std::unique_lock;\n> > > -\n> > > class Thread\n> > > {\n> > > public:\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 2AC13BDB1C\n\tfor ;\n\tFri, 12 Nov 2021 07:20:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7F5146034D;\n\tFri, 12 Nov 2021 08:20:16 +0100 (CET)","from mail-ed1-x532.google.com (mail-ed1-x532.google.com\n\t[IPv6:2a00:1450:4864:20::532])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0650960232\n\tfor ;\n\tFri, 12 Nov 2021 08:20:15 +0100 (CET)","by mail-ed1-x532.google.com with SMTP id f4so33967706edx.12\n\tfor ;\n\tThu, 11 Nov 2021 23:20:14 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"P7wlrP1n\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ai/4dZpd/ABf5do2vC90apa4utuRG3puH4apvKonGSs=;\n\tb=P7wlrP1n5MT+4UR2kV+61U0RfA7ox5yiWnbz3d/ZqYh9gkTyQoLUqYtQgC/S5y1BlP\n\t+WopwcTg1IPPLICFMNMXwnL34h+VFaUOP+yudNnp2n06btsb1HKtFOQl9ItgYbxjgN9H\n\t7KNHZmT9aNCz9lw/siBBTRtt0hQAlWivvCW6E=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ai/4dZpd/ABf5do2vC90apa4utuRG3puH4apvKonGSs=;\n\tb=3QlGV/Lrkgh/t5qGw5ramQWXCtH4c0d9NINEGQJ3/Dfh4MEHpNWNdUj5LS4dTa0STN\n\tx9SbRVNF9cFLDhkVd9FG8RyyUxCJzEL2xNfTBjpofGs3tD9OLG88efnjmYVYad1xgER8\n\t/CyVjORXvgBEOLrryDv6HcPV71WJL/f7aesz6uCf+6fDZXtaBvqAtWZwHgh9lvOaZQHF\n\tiji+sXue6ZvRwFfthsq5msKeYQsuFBFEp6OFsucm8lrKOBf7nRQ23lFiekqbQlZqLA2w\n\t/fHbUoFcJTzk6Hof4JrJ/GfC0gpZe59JsSSfjj3NBuwbPRTEX3a0Xcd2t+6qSoyibk0S\n\tggXQ==","X-Gm-Message-State":"AOAM532IzmsJYRrYwH++Di/dIi1X0QojA5QEixktPyBeUc2gcx5/YQbD\n\t9G4kLisuMhXQIT5FVJADEnL0DH0K5IZ+S/sW/vi3FA==","X-Google-Smtp-Source":"ABdhPJyq0wy3bYD2ZSspWhylGROYi8GDGqoC/Z/hYJX94tzUxk32B16YpDWV/jqVbezWa9bX0MnUMlWdH2EyZ4LuDTE=","X-Received":"by 2002:a05:6402:42:: with SMTP id\n\tf2mr18193547edu.204.1636701614534; \n\tThu, 11 Nov 2021 23:20:14 -0800 (PST)","MIME-Version":"1.0","References":"<20211029041424.1430886-1-hiroh@chromium.org>\n\t<20211029041424.1430886-3-hiroh@chromium.org>\n\t<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Fri, 12 Nov 2021 16:20:01 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","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":21091,"web_url":"https://patchwork.libcamera.org/comment/21091/","msgid":"","date":"2021-11-22T02:23:08","subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nOn Fri, Nov 12, 2021 at 04:20:01PM +0900, Hirokazu Honda wrote:\n> On Fri, Nov 12, 2021 at 7:53 AM Laurent Pinchart wrote:\n> > On Thu, Nov 11, 2021 at 10:08:35PM +0530, Umang Jain wrote:\n> > > On 10/29/21 9:44 AM, Hirokazu Honda wrote:\n> > > > Mutex2 and MutexLocker2 are annotated by clang thread safety\n> > >\n> > > Not sure I like the naming here. but I don't have any better suggestion\n> > > as of now, still contemplating on that.. maybe AMutex and AMutexLocker\n> > > to denote they are annotated?\n> >\n> > I'm also concerned by this. Can't we replace the current Mutex and\n> > MutexLocker with annotated versions, without having two implementations\n> > ?\n> \n> The problem is condition_variable. It takes MutexLocker\n> (std::unique_lock) on wait.\n> However, the new MutexLocker is not std::unique_lock. I avoid this\n> problem by MutexLocker2::get().\n> I found [1] MutexLocker derives std::unique_lock, but not sure if it is ok.\n> [1] https://stackoverflow.com/questions/40468897/clang-thread-safety-with-stdcondition-variable\n\nI understand that, but can't we then drop the\n\nusing Mutex = std::mutex;\nusing MutexLocker = std::unique_lock;\n\nand rename Mutex2 and MutexLocker2 to Mutex and MutexLocker ? Having two\noptions is really awful.\n\n> > > > annotations. So we can add annotation to code where the classes are used.\n> > > > In the future, they will replace Mutex and MutexLocker.\n> > >\n> > > Ok so I see a patch following, that does this replacement. I went into\n> > > the territory of thinking if this annotations can be used full libcamera\n> > > codebase. Can it be done (not as part of the series) but as a separate\n> > > series. I am not sure how much useful it would be since annotation is\n> > > clang-only. Let's see.\n> > >\n> > > So, the series really:\n> > >\n> > > In the future, they will replace Mutex and MutexLocker in the\n> > > android HAL layer.\n> > >\n> > > > Signed-off-by: Hirokazu Honda \n> > > > ---\n> > > > include/libcamera/base/meson.build | 1 +\n> > > > include/libcamera/base/mutex.h | 66 ++++++++++++++++++++++++++++++\n> > >\n> > > Please refer to my comment on 1/6 about putting this in\n> > > include/libcamera/internal, maybe that's a better place?\n> >\n> > I think base makes sense, as we already define Mutex and MutexLocker\n> > there.\n> >\n> > > > include/libcamera/base/thread.h | 5 +--\n> > > > 3 files changed, 68 insertions(+), 4 deletions(-)\n> > > > create mode 100644 include/libcamera/base/mutex.h\n> > > >\n> > > > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> > > > index 1a71ce5a..37c4435a 100644\n> > > > --- a/include/libcamera/base/meson.build\n> > > > +++ b/include/libcamera/base/meson.build\n> > > > @@ -13,6 +13,7 @@ libcamera_base_headers = files([\n> > > > 'flags.h',\n> > > > 'log.h',\n> > > > 'message.h',\n> > > > + 'mutex.h',\n> > > > 'object.h',\n> > > > 'private.h',\n> > > > 'semaphore.h',\n> > > > diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h\n> > > > new file mode 100644\n> > > > index 00000000..d130988e\n> > > > --- /dev/null\n> > > > +++ b/include/libcamera/base/mutex.h\n> > > > @@ -0,0 +1,66 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2021, Google Inc.\n> > > > + *\n> > > > + * thread.h - Thread support\n> > >\n> > >\n> > > oops it should be mutex.h\n> > >\n> > >\n> > > Patch itself looks good to me.\n> > >\n> > > > + */\n> > > > +#ifndef __LIBCAMERA_BASE_MUTEX_H__\n> > > > +#define __LIBCAMERA_BASE_MUTEX_H__\n> > > > +\n> > > > +#include \n> > > > +\n> > > > +#include \n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +using Mutex = std::mutex;\n> > > > +using MutexLocker = std::unique_lock;\n> > > > +\n> > > > +class CAPABILITY(\"mutex\") Mutex2 final\n> >\n> > Documentation is missing.\n> >\n> > Thinking further about this class, given that libc++ annotates\n> > std::mutex, do we a custom annotated class ? It would only be used when\n> > compiling with clang and libstd++, which seems a bit of a cornercase.\n> \n> Yeah, once libc++ annotats std::unique_lock, I would let Mutex be\n> std::unique_lock if libc++ and otherwise this custom class.\n\nI know we need to annotate std::unique_lock, so we need to wrap it in\nMutexLocker, but with libc++ do we need a custom-annotated Mutex class,\nor could we simply use\n\nusing Mutex = std::mutex;\n\nthen ?\n\n> > > > +{\n> > > > +public:\n> >\n> > The std::mutex constructor is constexpr, I think we should do the same\n> > here to replicate the same API.\n> >\n> > > > + void lock() ACQUIRE()\n> > > > + {\n> > > > + mutex_.lock();\n> > > > + }\n> > > > +\n> > > > + void unlock() RELEASE()\n> > > > + {\n> > > > + mutex_.unlock();\n> > > > + }\n> >\n> > We also need try_lock().\n> >\n> > > > +\n> > > > + std::mutex &get() { return mutex_; }\n> >\n> > Missing blank line.\n> >\n> > This function is only used by MutexLocker2. I think a friend declaration\n> > would be better here, to avoid exposing get().\n> >\n> > > > +private:\n> > > > + std::mutex mutex_;\n> > > > +};\n> > > > +\n> > > > +class SCOPED_CAPABILITY MutexLocker2 final\n> > > > +{\n> > > > +public:\n> > > > + MutexLocker2(Mutex2 &mutex) ACQUIRE(mutex)\n> > > > + : lock_(mutex.get())\n> > > > + {\n> > > > + }\n> > > > +\n> > > > + ~MutexLocker2() RELEASE()\n> > > > + {\n> > > > + }\n> > > > +\n> > > > + void lock() ACQUIRE()\n> > > > + {\n> > > > + lock_.lock();\n> > > > + }\n> > > > +\n> > > > + void unlock() RELEASE()\n> > > > + {\n> > > > + lock_.unlock();\n> > > > + }\n> >\n> > There are more functions in the std::unique_lock API (including more\n> > constructors), should we implement them too ?\n> >\n> > > > +\n> > > > + std::unique_lock &get() { return lock_; }\n> >\n> > Missing blank line.\n> >\n> > This function is only used to support std::condition_variable::wait(),\n> > wouldn't it be better to implement a ConditionVariable class that would\n> > take a MutexLocker argument in wait() ?\n> \n> I wondered about it. If we don't like MutexLocker derives\n> ConditionVariable, then I would try it.\n\nIt's the get() that bothers me a bit.\n\n> > > > +private:\n> > > > + std::unique_lock lock_;\n> > > > +};\n> > > > +\n> > > > +} /* namespace libcamera */\n> > > > +\n> > > > +#endif /* __LIBCAMERA_BASE_MUTEX_H__ */\n> > > > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h\n> > > > index e0ca0aea..ae630563 100644\n> > > > --- a/include/libcamera/base/thread.h\n> > > > +++ b/include/libcamera/base/thread.h\n> > > > @@ -8,13 +8,13 @@\n> > > > #define __LIBCAMERA_BASE_THREAD_H__\n> > > >\n> > > > #include \n> > > > -#include \n> > > > #include \n> > > > #include \n> > > >\n> > > > #include \n> > > >\n> > > > #include \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > >\n> > > > @@ -26,9 +26,6 @@ class Object;\n> > > > class ThreadData;\n> > > > class ThreadMain;\n> > > >\n> > > > -using Mutex = std::mutex;\n> > > > -using MutexLocker = std::unique_lock;\n> > > > -\n> > > > class Thread\n> > > > {\n> > > > public:","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 6DD67BDB13\n\tfor ;\n\tMon, 22 Nov 2021 02:23:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC55260376;\n\tMon, 22 Nov 2021 03:23:33 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22C4A60228\n\tfor ;\n\tMon, 22 Nov 2021 03:23:32 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 75A18A1B;\n\tMon, 22 Nov 2021 03:23:31 +0100 (CET)"],"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=\"ugpcSm2N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637547811;\n\tbh=DqD8Rf2PBbfekSjQ4HTBsTBESQfIDjdhALkLiTUS/Lg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ugpcSm2N/M9+MuJ5xuqXyBmEyw4KO9VWL7DwWuLAv4edD/dsBvdnMRwnHEe420C7F\n\tL9N8O5oA+t33mu3xRfrXjhlAUzkjpGk7RFST5CzWyEVg5baUrF7RaerIsnSC4xhi/k\n\tYl3kLHkbtAFA2ruUT+DSMQuSxIA5ZY7LpTsMlMm0=","Date":"Mon, 22 Nov 2021 04:23:08 +0200","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20211029041424.1430886-1-hiroh@chromium.org>\n\t<20211029041424.1430886-3-hiroh@chromium.org>\n\t<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","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":21165,"web_url":"https://patchwork.libcamera.org/comment/21165/","msgid":"","date":"2021-11-23T18:25:29","subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent,\n\nOn Mon, Nov 22, 2021 at 11:23 AM Laurent Pinchart\n wrote:\n>\n> Hi Hiro,\n>\n> On Fri, Nov 12, 2021 at 04:20:01PM +0900, Hirokazu Honda wrote:\n> > On Fri, Nov 12, 2021 at 7:53 AM Laurent Pinchart wrote:\n> > > On Thu, Nov 11, 2021 at 10:08:35PM +0530, Umang Jain wrote:\n> > > > On 10/29/21 9:44 AM, Hirokazu Honda wrote:\n> > > > > Mutex2 and MutexLocker2 are annotated by clang thread safety\n> > > >\n> > > > Not sure I like the naming here. but I don't have any better suggestion\n> > > > as of now, still contemplating on that.. maybe AMutex and AMutexLocker\n> > > > to denote they are annotated?\n> > >\n> > > I'm also concerned by this. Can't we replace the current Mutex and\n> > > MutexLocker with annotated versions, without having two implementations\n> > > ?\n> >\n> > The problem is condition_variable. It takes MutexLocker\n> > (std::unique_lock) on wait.\n> > However, the new MutexLocker is not std::unique_lock. I avoid this\n> > problem by MutexLocker2::get().\n> > I found [1] MutexLocker derives std::unique_lock, but not sure if it is ok.\n> > [1] https://stackoverflow.com/questions/40468897/clang-thread-safety-with-stdcondition-variable\n>\n> I understand that, but can't we then drop the\n>\n> using Mutex = std::mutex;\n> using MutexLocker = std::unique_lock;\n>\n> and rename Mutex2 and MutexLocker2 to Mutex and MutexLocker ? Having two\n> options is really awful.\n>\n\nDo you mean to replace Mutex and MutexLocker in the code with\nstd::mutex and std::unique_lcok before this patch?\n\n> > > > > annotations. So we can add annotation to code where the classes are used.\n> > > > > In the future, they will replace Mutex and MutexLocker.\n> > > >\n> > > > Ok so I see a patch following, that does this replacement. I went into\n> > > > the territory of thinking if this annotations can be used full libcamera\n> > > > codebase. Can it be done (not as part of the series) but as a separate\n> > > > series. I am not sure how much useful it would be since annotation is\n> > > > clang-only. Let's see.\n> > > >\n> > > > So, the series really:\n> > > >\n> > > > In the future, they will replace Mutex and MutexLocker in the\n> > > > android HAL layer.\n> > > >\n> > > > > Signed-off-by: Hirokazu Honda \n> > > > > ---\n> > > > > include/libcamera/base/meson.build | 1 +\n> > > > > include/libcamera/base/mutex.h | 66 ++++++++++++++++++++++++++++++\n> > > >\n> > > > Please refer to my comment on 1/6 about putting this in\n> > > > include/libcamera/internal, maybe that's a better place?\n> > >\n> > > I think base makes sense, as we already define Mutex and MutexLocker\n> > > there.\n> > >\n> > > > > include/libcamera/base/thread.h | 5 +--\n> > > > > 3 files changed, 68 insertions(+), 4 deletions(-)\n> > > > > create mode 100644 include/libcamera/base/mutex.h\n> > > > >\n> > > > > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> > > > > index 1a71ce5a..37c4435a 100644\n> > > > > --- a/include/libcamera/base/meson.build\n> > > > > +++ b/include/libcamera/base/meson.build\n> > > > > @@ -13,6 +13,7 @@ libcamera_base_headers = files([\n> > > > > 'flags.h',\n> > > > > 'log.h',\n> > > > > 'message.h',\n> > > > > + 'mutex.h',\n> > > > > 'object.h',\n> > > > > 'private.h',\n> > > > > 'semaphore.h',\n> > > > > diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h\n> > > > > new file mode 100644\n> > > > > index 00000000..d130988e\n> > > > > --- /dev/null\n> > > > > +++ b/include/libcamera/base/mutex.h\n> > > > > @@ -0,0 +1,66 @@\n> > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > > +/*\n> > > > > + * Copyright (C) 2021, Google Inc.\n> > > > > + *\n> > > > > + * thread.h - Thread support\n> > > >\n> > > >\n> > > > oops it should be mutex.h\n> > > >\n> > > >\n> > > > Patch itself looks good to me.\n> > > >\n> > > > > + */\n> > > > > +#ifndef __LIBCAMERA_BASE_MUTEX_H__\n> > > > > +#define __LIBCAMERA_BASE_MUTEX_H__\n> > > > > +\n> > > > > +#include \n> > > > > +\n> > > > > +#include \n> > > > > +\n> > > > > +namespace libcamera {\n> > > > > +\n> > > > > +using Mutex = std::mutex;\n> > > > > +using MutexLocker = std::unique_lock;\n> > > > > +\n> > > > > +class CAPABILITY(\"mutex\") Mutex2 final\n> > >\n> > > Documentation is missing.\n> > >\n> > > Thinking further about this class, given that libc++ annotates\n> > > std::mutex, do we a custom annotated class ? It would only be used when\n> > > compiling with clang and libstd++, which seems a bit of a cornercase.\n> >\n> > Yeah, once libc++ annotats std::unique_lock, I would let Mutex be\n> > std::unique_lock if libc++ and otherwise this custom class.\n>\n> I know we need to annotate std::unique_lock, so we need to wrap it in\n> MutexLocker, but with libc++ do we need a custom-annotated Mutex class,\n> or could we simply use\n>\n> using Mutex = std::mutex;\n>\n> then ?\n\nI think you're right.\nSo we should have like\n#if defined(USE_LIBCPP)\nusing Mutex = std::mutex;\n#else\nclass Mutex {\n ...\n};\n#endif\n\nIs this what you demand?\n\n>\n> > > > > +{\n> > > > > +public:\n> > >\n> > > The std::mutex constructor is constexpr, I think we should do the same\n> > > here to replicate the same API.\n> > >\n> > > > > + void lock() ACQUIRE()\n> > > > > + {\n> > > > > + mutex_.lock();\n> > > > > + }\n> > > > > +\n> > > > > + void unlock() RELEASE()\n> > > > > + {\n> > > > > + mutex_.unlock();\n> > > > > + }\n> > >\n> > > We also need try_lock().\n> > >\n> > > > > +\n> > > > > + std::mutex &get() { return mutex_; }\n> > >\n> > > Missing blank line.\n> > >\n> > > This function is only used by MutexLocker2. I think a friend declaration\n> > > would be better here, to avoid exposing get().\n> > >\n> > > > > +private:\n> > > > > + std::mutex mutex_;\n> > > > > +};\n> > > > > +\n> > > > > +class SCOPED_CAPABILITY MutexLocker2 final\n> > > > > +{\n> > > > > +public:\n> > > > > + MutexLocker2(Mutex2 &mutex) ACQUIRE(mutex)\n> > > > > + : lock_(mutex.get())\n> > > > > + {\n> > > > > + }\n> > > > > +\n> > > > > + ~MutexLocker2() RELEASE()\n> > > > > + {\n> > > > > + }\n> > > > > +\n> > > > > + void lock() ACQUIRE()\n> > > > > + {\n> > > > > + lock_.lock();\n> > > > > + }\n> > > > > +\n> > > > > + void unlock() RELEASE()\n> > > > > + {\n> > > > > + lock_.unlock();\n> > > > > + }\n> > >\n> > > There are more functions in the std::unique_lock API (including more\n> > > constructors), should we implement them too ?\n> > >\n> > > > > +\n> > > > > + std::unique_lock &get() { return lock_; }\n> > >\n> > > Missing blank line.\n> > >\n> > > This function is only used to support std::condition_variable::wait(),\n> > > wouldn't it be better to implement a ConditionVariable class that would\n> > > take a MutexLocker argument in wait() ?\n> >\n> > I wondered about it. If we don't like MutexLocker derives\n> > ConditionVariable, then I would try it.\n>\n> It's the get() that bothers me a bit.\n\nSorry, what do you mean?\nShall we have our own ConditionVariable or let MutexLocker derive\nstd::unique_lock?\n\nBest,\n-Hiro\n>\n> > > > > +private:\n> > > > > + std::unique_lock lock_;\n> > > > > +};\n> > > > > +\n> > > > > +} /* namespace libcamera */\n> > > > > +\n> > > > > +#endif /* __LIBCAMERA_BASE_MUTEX_H__ */\n> > > > > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h\n> > > > > index e0ca0aea..ae630563 100644\n> > > > > --- a/include/libcamera/base/thread.h\n> > > > > +++ b/include/libcamera/base/thread.h\n> > > > > @@ -8,13 +8,13 @@\n> > > > > #define __LIBCAMERA_BASE_THREAD_H__\n> > > > >\n> > > > > #include \n> > > > > -#include \n> > > > > #include \n> > > > > #include \n> > > > >\n> > > > > #include \n> > > > >\n> > > > > #include \n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > >\n> > > > > @@ -26,9 +26,6 @@ class Object;\n> > > > > class ThreadData;\n> > > > > class ThreadMain;\n> > > > >\n> > > > > -using Mutex = std::mutex;\n> > > > > -using MutexLocker = std::unique_lock;\n> > > > > -\n> > > > > class Thread\n> > > > > {\n> > > > > public:\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 2A2ECBF415\n\tfor ;\n\tTue, 23 Nov 2021 18:25:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FB6A60230;\n\tTue, 23 Nov 2021 19:25:43 +0100 (CET)","from mail-ed1-x535.google.com (mail-ed1-x535.google.com\n\t[IPv6:2a00:1450:4864:20::535])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F35D260121\n\tfor ;\n\tTue, 23 Nov 2021 19:25:41 +0100 (CET)","by mail-ed1-x535.google.com with SMTP id v1so62173036edx.2\n\tfor ;\n\tTue, 23 Nov 2021 10:25:41 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"j2yeOjrl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=gbOa+aKWoxOw0+TQ7FpUhvYYbWfKpX/7C1MMCoI3T7c=;\n\tb=j2yeOjrl9aV9H0O7Ejta3EXp//hmlFOIQRq7YZYN4a2fELsg4yzTuMS68nRMOrT3Wd\n\t7dwRZ1i9YTRfqvZaszE8XmDVdCHTlrqQvVzOKog4XBvW9yumx9VzIJ8rmGfy8UyC16KY\n\tpg97IG2O2/COcyod5m5n3x3lZRVlUuac+WwSg=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=gbOa+aKWoxOw0+TQ7FpUhvYYbWfKpX/7C1MMCoI3T7c=;\n\tb=BCzKKhJgCtp9U7l4NsiCCbUV7ODoIdpwMqsX0G5/XOaiZCEURD+PZtddwVXEO0yggx\n\tXIHIaqrh6Q+5i9gkihRRlxpUlYPXHjQbiAyTXExnKHUAOdnkBPHPmphEgL3ZvDNzDlrI\n\txBn8SbQf36xY8G0WZzGMjAFHRaErsik94T6FTpo7Wq7Qw0peQLl2LbLCH465BX90ccnO\n\t3pR2CMongvH2Kyf73bAeDUoYC1JHpITNeLe6E/KEDC8lvF5FIU8qrkXYgqYbGUOFwh3N\n\t1fJcDn+ansy+i8M4BRqSYRZDk4a5glDsssAw/Hel7qlhr0u/TU5CqSAK9gx/MnZtw2/a\n\tjwvQ==","X-Gm-Message-State":"AOAM533UMXIAtBg9LyAHrHU8AOXQQ6I2A/cjQ8YlxwyFczQcAVSjmrVR\n\tWHmkEKT9tI5R8idrHoz95ZwfcNp0DiUcHhaizGiowg==","X-Google-Smtp-Source":"ABdhPJwq3XJgiFVBKLyHQr1JRrfwIfk8L6QiDLloPuo3x1G9wsm942tlSXmDLDJdRhSOVTN7mHrXPTGrEgjOXcC6NwU=","X-Received":"by 2002:a05:6402:2026:: with SMTP id\n\tay6mr12787367edb.202.1637691941415; \n\tTue, 23 Nov 2021 10:25:41 -0800 (PST)","MIME-Version":"1.0","References":"<20211029041424.1430886-1-hiroh@chromium.org>\n\t<20211029041424.1430886-3-hiroh@chromium.org>\n\t<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Wed, 24 Nov 2021 03:25:29 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","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":21170,"web_url":"https://patchwork.libcamera.org/comment/21170/","msgid":"","date":"2021-11-24T03:19:44","subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nOn Wed, Nov 24, 2021 at 03:25:29AM +0900, Hirokazu Honda wrote:\n> On Mon, Nov 22, 2021 at 11:23 AM Laurent Pinchart wrote:\n> > On Fri, Nov 12, 2021 at 04:20:01PM +0900, Hirokazu Honda wrote:\n> > > On Fri, Nov 12, 2021 at 7:53 AM Laurent Pinchart wrote:\n> > > > On Thu, Nov 11, 2021 at 10:08:35PM +0530, Umang Jain wrote:\n> > > > > On 10/29/21 9:44 AM, Hirokazu Honda wrote:\n> > > > > > Mutex2 and MutexLocker2 are annotated by clang thread safety\n> > > > >\n> > > > > Not sure I like the naming here. but I don't have any better suggestion\n> > > > > as of now, still contemplating on that.. maybe AMutex and AMutexLocker\n> > > > > to denote they are annotated?\n> > > >\n> > > > I'm also concerned by this. Can't we replace the current Mutex and\n> > > > MutexLocker with annotated versions, without having two implementations\n> > > > ?\n> > >\n> > > The problem is condition_variable. It takes MutexLocker\n> > > (std::unique_lock) on wait.\n> > > However, the new MutexLocker is not std::unique_lock. I avoid this\n> > > problem by MutexLocker2::get().\n> > > I found [1] MutexLocker derives std::unique_lock, but not sure if it is ok.\n> > > [1] https://stackoverflow.com/questions/40468897/clang-thread-safety-with-stdcondition-variable\n> >\n> > I understand that, but can't we then drop the\n> >\n> > using Mutex = std::mutex;\n> > using MutexLocker = std::unique_lock;\n> >\n> > and rename Mutex2 and MutexLocker2 to Mutex and MutexLocker ? Having two\n> > options is really awful.\n> \n> Do you mean to replace Mutex and MutexLocker in the code with\n> std::mutex and std::unique_lcok before this patch?\n\nNo, otherwise we wouldn't be able to add thread safety annotation, would\nwe ?\n\nI don't want to have two different Mutex and MutexLocker classes, one\nwith manual annotations, and one without. We need a single class that\nall the libcamera codebase can use, without having to care about\nimplementation details.\n\n> > > > > > annotations. So we can add annotation to code where the classes are used.\n> > > > > > In the future, they will replace Mutex and MutexLocker.\n> > > > >\n> > > > > Ok so I see a patch following, that does this replacement. I went into\n> > > > > the territory of thinking if this annotations can be used full libcamera\n> > > > > codebase. Can it be done (not as part of the series) but as a separate\n> > > > > series. I am not sure how much useful it would be since annotation is\n> > > > > clang-only. Let's see.\n> > > > >\n> > > > > So, the series really:\n> > > > >\n> > > > > In the future, they will replace Mutex and MutexLocker in the\n> > > > > android HAL layer.\n> > > > >\n> > > > > > Signed-off-by: Hirokazu Honda \n> > > > > > ---\n> > > > > > include/libcamera/base/meson.build | 1 +\n> > > > > > include/libcamera/base/mutex.h | 66 ++++++++++++++++++++++++++++++\n> > > > >\n> > > > > Please refer to my comment on 1/6 about putting this in\n> > > > > include/libcamera/internal, maybe that's a better place?\n> > > >\n> > > > I think base makes sense, as we already define Mutex and MutexLocker\n> > > > there.\n> > > >\n> > > > > > include/libcamera/base/thread.h | 5 +--\n> > > > > > 3 files changed, 68 insertions(+), 4 deletions(-)\n> > > > > > create mode 100644 include/libcamera/base/mutex.h\n> > > > > >\n> > > > > > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> > > > > > index 1a71ce5a..37c4435a 100644\n> > > > > > --- a/include/libcamera/base/meson.build\n> > > > > > +++ b/include/libcamera/base/meson.build\n> > > > > > @@ -13,6 +13,7 @@ libcamera_base_headers = files([\n> > > > > > 'flags.h',\n> > > > > > 'log.h',\n> > > > > > 'message.h',\n> > > > > > + 'mutex.h',\n> > > > > > 'object.h',\n> > > > > > 'private.h',\n> > > > > > 'semaphore.h',\n> > > > > > diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h\n> > > > > > new file mode 100644\n> > > > > > index 00000000..d130988e\n> > > > > > --- /dev/null\n> > > > > > +++ b/include/libcamera/base/mutex.h\n> > > > > > @@ -0,0 +1,66 @@\n> > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > > > +/*\n> > > > > > + * Copyright (C) 2021, Google Inc.\n> > > > > > + *\n> > > > > > + * thread.h - Thread support\n> > > > >\n> > > > >\n> > > > > oops it should be mutex.h\n> > > > >\n> > > > >\n> > > > > Patch itself looks good to me.\n> > > > >\n> > > > > > + */\n> > > > > > +#ifndef __LIBCAMERA_BASE_MUTEX_H__\n> > > > > > +#define __LIBCAMERA_BASE_MUTEX_H__\n> > > > > > +\n> > > > > > +#include \n> > > > > > +\n> > > > > > +#include \n> > > > > > +\n> > > > > > +namespace libcamera {\n> > > > > > +\n> > > > > > +using Mutex = std::mutex;\n> > > > > > +using MutexLocker = std::unique_lock;\n> > > > > > +\n> > > > > > +class CAPABILITY(\"mutex\") Mutex2 final\n> > > >\n> > > > Documentation is missing.\n> > > >\n> > > > Thinking further about this class, given that libc++ annotates\n> > > > std::mutex, do we a custom annotated class ? It would only be used when\n> > > > compiling with clang and libstd++, which seems a bit of a cornercase.\n> > >\n> > > Yeah, once libc++ annotats std::unique_lock, I would let Mutex be\n> > > std::unique_lock if libc++ and otherwise this custom class.\n> >\n> > I know we need to annotate std::unique_lock, so we need to wrap it in\n> > MutexLocker, but with libc++ do we need a custom-annotated Mutex class,\n> > or could we simply use\n> >\n> > using Mutex = std::mutex;\n> >\n> > then ?\n> \n> I think you're right.\n> So we should have like\n> #if defined(USE_LIBCPP)\n> using Mutex = std::mutex;\n> #else\n> class Mutex {\n> ...\n> };\n> #endif\n> \n> Is this what you demand?\n\nThat's what I had in mind, yes.\n\n> > > > > > +{\n> > > > > > +public:\n> > > >\n> > > > The std::mutex constructor is constexpr, I think we should do the same\n> > > > here to replicate the same API.\n> > > >\n> > > > > > + void lock() ACQUIRE()\n> > > > > > + {\n> > > > > > + mutex_.lock();\n> > > > > > + }\n> > > > > > +\n> > > > > > + void unlock() RELEASE()\n> > > > > > + {\n> > > > > > + mutex_.unlock();\n> > > > > > + }\n> > > >\n> > > > We also need try_lock().\n> > > >\n> > > > > > +\n> > > > > > + std::mutex &get() { return mutex_; }\n> > > >\n> > > > Missing blank line.\n> > > >\n> > > > This function is only used by MutexLocker2. I think a friend declaration\n> > > > would be better here, to avoid exposing get().\n> > > >\n> > > > > > +private:\n> > > > > > + std::mutex mutex_;\n> > > > > > +};\n> > > > > > +\n> > > > > > +class SCOPED_CAPABILITY MutexLocker2 final\n> > > > > > +{\n> > > > > > +public:\n> > > > > > + MutexLocker2(Mutex2 &mutex) ACQUIRE(mutex)\n> > > > > > + : lock_(mutex.get())\n> > > > > > + {\n> > > > > > + }\n> > > > > > +\n> > > > > > + ~MutexLocker2() RELEASE()\n> > > > > > + {\n> > > > > > + }\n> > > > > > +\n> > > > > > + void lock() ACQUIRE()\n> > > > > > + {\n> > > > > > + lock_.lock();\n> > > > > > + }\n> > > > > > +\n> > > > > > + void unlock() RELEASE()\n> > > > > > + {\n> > > > > > + lock_.unlock();\n> > > > > > + }\n> > > >\n> > > > There are more functions in the std::unique_lock API (including more\n> > > > constructors), should we implement them too ?\n> > > >\n> > > > > > +\n> > > > > > + std::unique_lock &get() { return lock_; }\n> > > >\n> > > > Missing blank line.\n> > > >\n> > > > This function is only used to support std::condition_variable::wait(),\n> > > > wouldn't it be better to implement a ConditionVariable class that would\n> > > > take a MutexLocker argument in wait() ?\n> > >\n> > > I wondered about it. If we don't like MutexLocker derives\n> > > ConditionVariable, then I would try it.\n> >\n> > It's the get() that bothers me a bit.\n> \n> Sorry, what do you mean?\n> Shall we have our own ConditionVariable or let MutexLocker derive\n> std::unique_lock?\n\nOne of those two :-) As we define Mutex and MutexLocker types, it could\nmake sense to add a ConditionVariable type too. However, if that would\nwork for the libcamera internals, it would limit interoperability with\nother APIs that use the C++ standard classes. Having MutexLocker inherit\nfrom std::unique_lock would solve that problem, but I don't know if that\ncould cause issues. std:: classes are usually not meant to be derived\nfrom, and someone who would cast MutexLocker to std::unique_lock would\nbe able to call the functions the class offers without going through\nannotations, which I assume could lead to warnings.\n\nI'm beginning to wonder if the last of thread safety annotations in\nlibc++ for std::unique_lock could be caused by the fact that thread\nsafety analysis doesn't offer the required features to be really\nusable. There have been attempts to fix this before (see\nhttps://lists.llvm.org/pipermail/cfe-dev/2016-November/051453.html for\ninstance), but nothing got merged, which isn't a good sign.\n\n> > > > > > +private:\n> > > > > > + std::unique_lock lock_;\n> > > > > > +};\n> > > > > > +\n> > > > > > +} /* namespace libcamera */\n> > > > > > +\n> > > > > > +#endif /* __LIBCAMERA_BASE_MUTEX_H__ */\n> > > > > > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h\n> > > > > > index e0ca0aea..ae630563 100644\n> > > > > > --- a/include/libcamera/base/thread.h\n> > > > > > +++ b/include/libcamera/base/thread.h\n> > > > > > @@ -8,13 +8,13 @@\n> > > > > > #define __LIBCAMERA_BASE_THREAD_H__\n> > > > > >\n> > > > > > #include \n> > > > > > -#include \n> > > > > > #include \n> > > > > > #include \n> > > > > >\n> > > > > > #include \n> > > > > >\n> > > > > > #include \n> > > > > > +#include \n> > > > > > #include \n> > > > > > #include \n> > > > > >\n> > > > > > @@ -26,9 +26,6 @@ class Object;\n> > > > > > class ThreadData;\n> > > > > > class ThreadMain;\n> > > > > >\n> > > > > > -using Mutex = std::mutex;\n> > > > > > -using MutexLocker = std::unique_lock;\n> > > > > > -\n> > > > > > class Thread\n> > > > > > {\n> > > > > > public:","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 7A513BF415\n\tfor ;\n\tWed, 24 Nov 2021 03:20:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A2B656022F;\n\tWed, 24 Nov 2021 04:20:08 +0100 (CET)","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 2CBB660121\n\tfor ;\n\tWed, 24 Nov 2021 04:20:07 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 869A0D78;\n\tWed, 24 Nov 2021 04:20:06 +0100 (CET)"],"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=\"Kv12KAPd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637724006;\n\tbh=TYS7yRLQhTmLElPitQMRbs8udtVDgy5RvDwjmUEJH6Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kv12KAPd4c8+nyxiUs8yIvBZ7p+/cxyjIobPY1U4T9PjKb5Lj8J0Ky6EFhA/0fSWB\n\t0XB4Uxq+gNWKs9llvXdsi9PqxUobW6R+i4dhNVv4elXKtxxvFH4jDjKe7h3bE35yZB\n\tY5nlkxa8tMNqAANYlHYvGO1Oae17zUed5TQ/ApsU=","Date":"Wed, 24 Nov 2021 05:19:44 +0200","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20211029041424.1430886-1-hiroh@chromium.org>\n\t<20211029041424.1430886-3-hiroh@chromium.org>\n\t<21a1d82e-fe7f-532b-94b0-4a4ca10a4e19@ideasonboard.com>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 2/6] libcamera: base: Add mutex\n\tclasses with thread safety annotations","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\" "}}]