| Message ID | 20200728105541.13326-3-email@uajain.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@uajain.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@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@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(); > } > }; >
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> { 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(); } };
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@uajain.com> --- include/libcamera/camera.h | 2 +- src/libcamera/camera.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)