[libcamera-devel] [PATCH v2 2/5] libcamera: base: thread: Apply clang thread safety annotation

[Laurent Pinchart]

Hi Umang and Hiro,

Thank you for the patch.

On Mon, Jun 20, 2022 at 10:20:24PM +0530, Umang Jain via libcamera-devel wrote:
> From: Hirokazu Honda <hiroh at chromium.org>
> 
> This annotates member variables of ThreadData by clang thread
> safety annotations.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/libcamera/base/thread.cpp | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> index 6bda9d14..2e26b83c 100644
> --- a/src/libcamera/base/thread.cpp
> +++ b/src/libcamera/base/thread.cpp
> @@ -151,7 +151,7 @@ private:
>  	friend class ThreadMain;
>  
>  	Thread *thread_;
> -	bool running_;
> +	bool running_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  	pid_t tid_;
>  
>  	Mutex mutex_;
> @@ -160,7 +160,7 @@ private:
>  
>  	ConditionVariable cv_;
>  	std::atomic<bool> exit_;
> -	int exitCode_;
> +	int exitCode_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  
>  	MessageQueue messages_;
>  };
> @@ -422,11 +422,19 @@ bool Thread::wait(utils::duration duration)
>  	{
>  		MutexLocker locker(data_->mutex_);
>  
> -		if (duration == utils::duration::max())
> -			data_->cv_.wait(locker, [&]() { return !data_->running_; });
> -		else
> -			hasFinished = data_->cv_.wait_for(locker, duration,
> -							  [&]() { return !data_->running_; });
> +		if (duration == utils::duration::max()) {
> +			data_->cv_.wait(
> +				locker,
> +				[&]() LIBCAMERA_TSA_REQUIRES(data_->mutex_) {
> +					return !data_->running_;
> +				});
> +		} else {
> +			hasFinished = data_->cv_.wait_for(
> +				locker, duration,
> +				[&]() LIBCAMERA_TSA_REQUIRES(data_->mutex_) {
> +					return !data_->running_;
> +				});
> +		}

Let's improve readability a bit:

		auto isRunning = [&]() LIBCAMERA_TSA_REQUIRES(data_->mutex_) {
			return !data_->running_;
		});

		if (duration == utils::duration::max())
			data_->cv_.wait(locker, isRunning);
		else
			hasFinished = data_->cv_.wait_for(locker, duration,
							  isRunning);

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  	}
>  
>  	if (thread_.joinable())

-- 
Regards,

Laurent Pinchart