[libcamera-devel] [PATCH 2/2] libcamera: camera: Ensure deletion via deleteLater()

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Tue, Jul 28, 2020 at 10:57:25AM +0000, Umang Jain wrote:
> Object::deleteLater() ensures that the deletion of the Object
> takes place in a thread it is bound to. Deleting the Object
> in a different thread is a violation according to the threading
> model.
> 
> On hot-unplug of a currently streaming camera, the last reference
> of Camera when dropped from the application thread (for e.g. QCam's
> thread), the destructor is then called from this thread. This is not
> allowed by the threading model. Camera is meant to be deleted in the
> thread it is bound to - in this case the CameraManager's thread.
> 
> Signed-off-by: Umang Jain <email at uajain.com>

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

> ---
>  include/libcamera/camera.h | 2 +-
>  src/libcamera/camera.cpp   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 4d1a4a9..beb87e6 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -66,7 +66,7 @@ protected:
>  	std::vector<StreamConfiguration> config_;
>  };
>  
> -class Camera final : public std::enable_shared_from_this<Camera>
> +class Camera final : public Object, public std::enable_shared_from_this<Camera>

You should include object.h at the top of the file.

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

>  {
>  public:
>  	static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 69a1b44..034f341 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -464,7 +464,7 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
>  	struct Deleter : std::default_delete<Camera> {
>  		void operator()(Camera *camera)
>  		{
> -			delete camera;
> +			camera->deleteLater();
>  		}
>  	};
>  

-- 
Regards,

Laurent Pinchart