[libcamera-devel] [PATCH v2 3/5] libcamera: camera_manager: Apply clang thread safety annotation

[Laurent Pinchart]

Hi Umang and Hiro,

Thank you for the patch.

On Mon, Jun 20, 2022 at 10:20:25PM +0530, Umang Jain via libcamera-devel wrote:
> From: Hirokazu Honda <hiroh at chromium.org>
> 
> This annotates member functions and variables of
> CameraManager::Private 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/camera_manager.cpp | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 70d73822..4f946516 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -42,8 +42,8 @@ public:
>  
>  	int start();
>  	void addCamera(std::shared_ptr<Camera> camera,
> -		       const std::vector<dev_t> &devnums);
> -	void removeCamera(Camera *camera);
> +		       const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);
> +	void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
>  
>  	/*
>  	 * This mutex protects
> @@ -52,8 +52,8 @@ public:
>  	 * - cameras_ and camerasByDevnum_ after initialization
>  	 */
>  	mutable Mutex mutex_;
> -	std::vector<std::shared_ptr<Camera>> cameras_;
> -	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
> +	std::vector<std::shared_ptr<Camera>> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
> +	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  
>  protected:
>  	void run() override;
> @@ -61,11 +61,11 @@ protected:
>  private:
>  	int init();
>  	void createPipelineHandlers();
> -	void cleanup();
> +	void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);
>  
>  	ConditionVariable cv_;
> -	bool initialized_;
> -	int status_;
> +	bool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
> +	int status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  
>  	std::unique_ptr<DeviceEnumerator> enumerator_;
>  
> @@ -87,7 +87,8 @@ int CameraManager::Private::start()
>  
>  	{
>  		MutexLocker locker(mutex_);
> -		cv_.wait(locker, [&] { return initialized_; });
> +		cv_.wait(locker,
> +			 [&]() LIBCAMERA_TSA_REQUIRES(mutex_) { return initialized_; });

		cv_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(mutex_) {
				 return initialized_;
			 });

to match the usual style.

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

>  		status = status_;
>  	}
>  

-- 
Regards,

Laurent Pinchart