[libcamera-devel] [PATCH v3 2/3] android: CameraDevice: Prevent race due to concurrent HAL calls

[Jacopo Mondi]

Hi Hiro,

On Mon, Apr 26, 2021 at 05:38:29PM +0900, Hirokazu Honda wrote:
> HAL API functions might be called by different threads in Android
> Camera HAL3 API, while process_capture_request() must be called
> by a single thread. Furthermore, close() and flush() can be
> called any time. Therefore, races to variables can occur among
> the HAL API functions.
> This prevents the races by enclosing HAL calls by mutex. It
> might not be the best in terms of the performance, but the
> simplicity is good as a first step and also further development.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
>  	/*
> -	 * \todo Report and identify the stream number or configuration to
> -	 * clarify the stream that failed.
> +	 * \todo Report and identify the stream number or configuration
> +	 * to clarify the stream that failed.
>  	 */
> -	LOG(HAL, Error) << "Error occurred on frame " << frameNumber << " ("
> -			<< toPixelFormat(stream->format).toString() << ")";
> +	LOG(HAL, Error)
> +		<< "Error occurred on frame " << descriptor.frameNumber_ << " ("
> +		<< toPixelFormat(descriptor.buffers_[0].stream->format).toString() << ")";

That's probably a left-over as the patch does not compile with this
hunk applied

Thanks
  j

>
>  	notify.type = CAMERA3_MSG_ERROR;
>  	notify.message.error.error_stream = stream;
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 95d77890..325fb967 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -125,9 +125,12 @@ private:
>
>  	std::vector<Camera3StreamConfiguration> streamConfigurations_;
>  	std::map<int, libcamera::PixelFormat> formatsMap_;
> -	std::vector<CameraStream> streams_;
>
> -	std::mutex mutex_; /* Protect descriptors_ and requests_. */
> +	/* Avoid races by concurrent Camera HAL API calls. */
> +	std::mutex halMutex_;
> +
> +	std::mutex mutex_; /* Protect streams_, descriptors_ and requests_. */
> +	std::vector<CameraStream> streams_;
>  	std::map<uint64_t, Camera3RequestDescriptor> descriptors_;
>  	std::map<uint64_t, std::unique_ptr<CaptureRequest>> requests_;
>
> --
> 2.31.1.498.g6c1eba8ee3d-goog
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel