[libcamera-devel] [RFC PATCH] android: CameraWorker: Cancel pending messages in queue in stop()

[Jacopo Mondi]

On Fri, Apr 23, 2021 at 01:04:27PM +0900, Hirokazu Honda wrote:
> CameraWorker::stop() executes Thread::exit() and Thread::wait().
> However, intuitively, the two functions doesn't clean up pending
> messages in the message queue of the thread. They are rather
> cleaned in the destruction of CameraWorker, but CameraWorker's
> life time is the same as CameraDevice and thus CameraManager,
> which is never destroyed. On the other hand,
> Camera3RequestDescriptors are destroyed in CameraDevice::stop().
>
> This causes CameraWorker::Worker::queueRequest() starts to be
> executed after CameraWorker::start(), which is after

I'm not sure I'm following right the sequence of Worker::queueRequest(),
CameraWorker::start() and CameraDevice::stop()

Should this be CameraWorker::stop() ?

Did you ever triggered this failure ?

> CameraDevice::stop(). This obviously causes a segmentation fault,
> because CameraWorker::Worker::queueRequest() touches
> CaptureRequest which is owned by Camera3RequestDescriptor.
>
> This fixes the issue by cancel pending message in thread queue in
> CameraWorker::stop() with Thread::dispatchMessages().

For my limited understanding of the Object and Thread classes, I think
it's ok to dispatch all pending method calls.

However what concerns me is also Request that are pending in
void CameraWorker::Worker::processRequest() when the Worker gets
stopped. Should we wait for the queue of pending requests to have
finised waiting their fences (or instrument an interruption mechanism)
before returning from stop() ?

>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/android/camera_worker.cpp | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp
> index 300ddde0..b68ce9d5 100644
> --- a/src/android/camera_worker.cpp
> +++ b/src/android/camera_worker.cpp
> @@ -62,6 +62,7 @@ void CameraWorker::start()
>
>  void CameraWorker::stop()
>  {
> +	thread_.dispatchMessages(Message::Type::InvokeMessage);
>  	thread_.exit();
>  	thread_.wait();
>  }
> --
> 2.31.1.498.g6c1eba8ee3d-goog
>