[libcamera-devel] [PATCH v4 05/12] libcamera: base: Add mutex classes with thread safety annotations
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Dec 1 08:31:55 CET 2021
Hi Hiro,
Thank you for the patch.
On Wed, Dec 01, 2021 at 04:07:21PM +0900, Hirokazu Honda wrote:
> This replaces Mutex and MutexLocker with our own defined classes.
> The classes are annotated by clang thread safety annotations.
> So we can add annotation to code where the classes are used.
>
> v4l2 code needs to be annotated, which violates Mutex capability.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> include/libcamera/base/meson.build | 1 +
> include/libcamera/base/mutex.h | 132 +++++++++++++++++++++++++++++
> include/libcamera/base/thread.h | 7 +-
> src/libcamera/base/meson.build | 1 +
> src/libcamera/base/mutex.cpp | 65 ++++++++++++++
> src/libcamera/base/thread.cpp | 15 ----
> src/v4l2/v4l2_camera_proxy.h | 4 +-
> 7 files changed, 202 insertions(+), 23 deletions(-)
> create mode 100644 include/libcamera/base/mutex.h
> create mode 100644 src/libcamera/base/mutex.cpp
>
> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
> index 1a71ce5a..37c4435a 100644
> --- a/include/libcamera/base/meson.build
> +++ b/include/libcamera/base/meson.build
> @@ -13,6 +13,7 @@ libcamera_base_headers = files([
> 'flags.h',
> 'log.h',
> 'message.h',
> + 'mutex.h',
> 'object.h',
> 'private.h',
> 'semaphore.h',
> diff --git a/include/libcamera/base/mutex.h b/include/libcamera/base/mutex.h
> new file mode 100644
> index 00000000..8b70508e
> --- /dev/null
> +++ b/include/libcamera/base/mutex.h
> @@ -0,0 +1,132 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * mutex.h - Mutex classes with clang thread safety annotation
> + */
> +
> +#pragma once
> +
> +#include <condition_variable>
> +#include <mutex>
> +
> +#include <libcamera/base/thread_annotations.h>
> +
> +namespace libcamera {
> +
> +/* \todo using Mutex = std::mutex if lib++ is used. */
> +
> +#ifndef __DOXYGEN__
> +
> +class LIBCAMERA_TSA_CAPABILITY("mutex") Mutex final
> +{
> +public:
> + constexpr Mutex()
> + {
> + }
> +
> + void lock() LIBCAMERA_TSA_ACQUIRE()
> + {
> + mutex_.lock();
> + }
> +
> + void unlock() LIBCAMERA_TSA_RELEASE()
> + {
> + mutex_.unlock();
> + }
> +
> +private:
> + friend class MutexLocker;
> +
> + std::mutex mutex_;
> +};
> +
> +class LIBCAMERA_TSA_SCOPED_CAPABILITY MutexLocker final
> +{
> +public:
> + explicit MutexLocker(Mutex &mutex) LIBCAMERA_TSA_ACQUIRE(mutex)
> + : lock_(mutex.mutex_)
> + {
> + }
> +
> + MutexLocker(Mutex &mutex, std::defer_lock_t t) noexcept LIBCAMERA_TSA_EXCLUDES(mutex)
> + : lock_(mutex.mutex_, t)
> + {
> + }
> +
> + ~MutexLocker() LIBCAMERA_TSA_RELEASE()
> + {
> + }
> +
> + void lock() LIBCAMERA_TSA_ACQUIRE()
> + {
> + lock_.lock();
> + }
> +
> + bool try_lock() LIBCAMERA_TSA_TRY_ACQUIRE(true)
> + {
> + return lock_.try_lock();
> + }
> +
> + void unlock() LIBCAMERA_TSA_RELEASE()
> + {
> + lock_.unlock();
> + }
> +
> +private:
> + friend class ConditionVariable;
> +
> + std::unique_lock<std::mutex> lock_;
> +};
> +
> +class ConditionVariable final
> +{
> +public:
> + ConditionVariable()
> + {
> + }
> +
> + void notify_one() noexcept
> + {
> + cv_.notify_one();
> + }
> +
> + void notify_all() noexcept
> + {
> + cv_.notify_all();
> + }
> +
> + template<class Predicate>
> + void wait(MutexLocker &locker, Predicate stopWaiting)
> + {
> + cv_.wait(locker.lock_, stopWaiting);
> + }
> +
> + template<class Rep, class Period, class Predicate>
> + bool wait_for(MutexLocker &locker,
> + const std::chrono::duration<Rep, Period> &relTime,
> + Predicate stopWaiting)
> + {
> + return cv_.wait_for(locker.lock_, relTime, stopWaiting);
> + }
> +
> +private:
> + std::condition_variable cv_;
> +};
> +
> +#else /* __DOXYGEN__ */
> +
> +class Mutex final
> +{
> +};
> +
> +class MutexLocker final
> +{
> +};
> +
> +class ConditionVariable final
> +{
> +};
> +
> +#endif /* __DOXYGEN__ */
> +} /* namespace libcamera */
> diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h
> index 1ebf8363..44678c34 100644
> --- a/include/libcamera/base/thread.h
> +++ b/include/libcamera/base/thread.h
> @@ -7,15 +7,14 @@
>
> #pragma once
>
> -#include <condition_variable>
> #include <memory>
> -#include <mutex>
> #include <sys/types.h>
> #include <thread>
>
> #include <libcamera/base/private.h>
>
> #include <libcamera/base/message.h>
> +#include <libcamera/base/mutex.h>
> #include <libcamera/base/signal.h>
> #include <libcamera/base/utils.h>
>
> @@ -27,10 +26,6 @@ class Object;
> class ThreadData;
> class ThreadMain;
>
> -using ConditionVariable = std::condition_variable;
> -using Mutex = std::mutex;
> -using MutexLocker = std::unique_lock<std::mutex>;
> -
> class Thread
> {
> public:
> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> index 05fed7ac..b93b8505 100644
> --- a/src/libcamera/base/meson.build
> +++ b/src/libcamera/base/meson.build
> @@ -11,6 +11,7 @@ libcamera_base_sources = files([
> 'flags.cpp',
> 'log.cpp',
> 'message.cpp',
> + 'mutex.cpp',
> 'object.cpp',
> 'semaphore.cpp',
> 'signal.cpp',
> diff --git a/src/libcamera/base/mutex.cpp b/src/libcamera/base/mutex.cpp
> new file mode 100644
> index 00000000..313bcb20
> --- /dev/null
> +++ b/src/libcamera/base/mutex.cpp
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * mutex.cpp - Mutex classes with clang thread safety annotation
> + */
> +
> +#include <libcamera/base/mutex.h>
> +
> +/**
> + * \file base/mutex.h
> + * \brief Mutex classes with clang thread safety annotation
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class Mutex
> + * \brief std::mutex wrapper with clang thread safety annotation
> + *
> + * The Mutex class wraps a std::mutex instance to add clang thread safety
> + * annotation support. The class exposes the same interface as std::mutex and
> + * can be used as a transparent replacement. It integrates with the
> + * MutexLocker and ConditionVariable classes.
> + *
> + * See https://en.cppreference.com/w/cpp/thread/mutex for the complete API
> + * documentation.
> + */
> +
> +/**
> + * \class MutexLocker
> + * \brief std::unique_lock wrapper with clang thread safety annotation
> + *
> + * The MutexLocker class wraps a std::unique_lock instance to add clang thread
> + * safety annotation support. The class exposes the same interface as
> + * std::unique_lock and can be used as a transparent replacement. It integrates
> + * with the Mutex and ConditionVariable classes.
Extra space before "with".
> + *
> + * See https://en.cppreference.com/w/cpp/thread/mutex for the complete API
This should point to the unique_lock documentation.
> + * documentation.
> + */
> +
> +/**
> + * \class MutexLocker
> + * \brief std::unique_lock wrapper with clang thread safety annotation
> + *
> + * The MutexLocker class wraps a std::unique_lock instance to add clang thread
> + * safety annotation support. The class exposes the same interface as
> + * std::unique_lock and can be used as a transparent replacement. It integrates
> + * with the Mutex and ConditionVariable classes.
> + *
> + * See https://en.cppreference.com/w/cpp/thread/mutex for the complete API
> + * documentation.
> + */
Duplicate documentation for MutexLocker ?
> +
> +/**
> + * \class ConditionVariable
> + * \brief std::condition_variable wrapper integrating with MutexLocker
> + *
> + * The ConditionVariable class wraps a std::condition_variable instance to
> + * integrate MutexLocker. The class exposes the same interface as
s/integrate MutexLocker/integrate with the MutexLocker class/
> + * std::condition_variable and can be used as a transparent replacement.
A pointer to the condition_variable documentation would be useful here
too, like for mutex and unique_lock.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> index b893135f..b2043b7e 100644
> --- a/src/libcamera/base/thread.cpp
> +++ b/src/libcamera/base/thread.cpp
> @@ -204,21 +204,6 @@ ThreadData *ThreadData::current()
> return data;
> }
>
> -/**
> - * \typedef ConditionVariable
> - * \brief An alias for std::condition_variable
> - */
> -
> -/**
> - * \typedef Mutex
> - * \brief An alias for std::mutex
> - */
> -
> -/**
> - * \typedef MutexLocker
> - * \brief An alias for std::unique_lock<std::mutex>
> - */
> -
> /**
> * \class Thread
> * \brief A thread of execution
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 040954dd..fa0a49e0 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -14,7 +14,7 @@
> #include <sys/types.h>
> #include <vector>
>
> -#include <libcamera/base/thread.h>
> +#include <libcamera/base/mutex.h>
>
> #include <libcamera/camera.h>
>
> @@ -59,7 +59,7 @@ private:
> int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg);
> int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);
> int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,
> - libcamera::Mutex *lock);
> + libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);
> int vidioc_streamon(V4L2CameraFile *file, int *arg);
> int vidioc_streamoff(V4L2CameraFile *file, int *arg);
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list