[libcamera-devel] [PATCH] android: camera_device: Fix race on queuing capture request

[Hirokazu Honda]

Hi Umang, thank you for the patch.

On Fri, Sep 24, 2021 at 3:44 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> The Camera3RequestDescriptor containing the capture request
> is adding to the descriptors_ map after a call to
> CameraWorker::queueRequest(). This is a race condition since
> CameraWorker::queueRequest() queues request to libcamera::Camera
> asynchronously and the addition of the descriptor to the map
> occurs with std::move(). Hence, it might happen that the async
> queueRequest() hasn't finished but the descriptor gets std::move()ed.
>
> Fix it by adding the descriptor to map first, before
> CameraWorker::queueRequest().


I don't understand the problem well.
I think the pointer of descriptors.request_.get() is not invalidated
by std::move().

>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 21844e51..c4c03d86 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1065,13 +1065,14 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>                 state_ = State::Running;
>         }
>
> -       worker_.queueRequest(descriptor.request_.get());
> -
> +       unsigned long requestCookie = descriptor.request_->cookie();
>         {
>                 MutexLocker descriptorsLock(descriptorsMutex_);
> -               descriptors_[descriptor.request_->cookie()] = std::move(descriptor);
> +               descriptors_[requestCookie] = std::move(descriptor);
>         }
>
> +       worker_.queueRequest(descriptors_[requestCookie].request_.get());

Accessing descriptors_ must be while holding descriptorsMutex_.

-Hiro
> +
>         return 0;
>  }
>
> --
> 2.31.1
>