| Message ID | 20211201075348.3121186-6-hiroh@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, On 12/1/21 1:23 PM, 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@chromium.org> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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 | 56 ++++++++++++ > src/libcamera/base/thread.cpp | 15 ---- > src/v4l2/v4l2_camera_proxy.h | 4 +- > 7 files changed, 193 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. */ s/lib++/libc++/ Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > + > +#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..8e63387b > --- /dev/null > +++ b/src/libcamera/base/mutex.cpp > @@ -0,0 +1,56 @@ > +/* 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. > + * > + * See https://en.cppreference.com/w/cpp/thread/unique_lock for the complete API > + * documentation. > + */ > + > +/** > + * \class ConditionVariable > + * \brief std::condition_variable wrapper integrating with MutexLocker > + * > + * The ConditionVariable class wraps a std::condition_variable instance to > + * integrate with the MutexLocker class. The class exposes the same interface as > + * std::condition_variable and can be used as a transparent replacement. > + * > + * See https://en.cppreference.com/w/cpp/thread/condition_variable for the > + * complete API documentation. > + * > + */ > + > +} /* 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); >
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..8e63387b --- /dev/null +++ b/src/libcamera/base/mutex.cpp @@ -0,0 +1,56 @@ +/* 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. + * + * See https://en.cppreference.com/w/cpp/thread/unique_lock for the complete API + * documentation. + */ + +/** + * \class ConditionVariable + * \brief std::condition_variable wrapper integrating with MutexLocker + * + * The ConditionVariable class wraps a std::condition_variable instance to + * integrate with the MutexLocker class. The class exposes the same interface as + * std::condition_variable and can be used as a transparent replacement. + * + * See https://en.cppreference.com/w/cpp/thread/condition_variable for the + * complete API documentation. + * + */ + +} /* 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);