[libcamera-devel,v4,4/4] libcamera: camera: Ensure deletion via deleteLater()

Message ID 20200731181410.99892-5-email@uajain.com
State Accepted
Commit f2df5abe5db9da36d4f07192a037acbcda2dee02
Headers show
Series
  • libcamera: object: Add deleteLater() support
Related show

Commit Message

Umang Jain July 31, 2020, 6:14 p.m. UTC
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 libcamera
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 libcamera 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       | 3 ++-
 src/libcamera/camera.cpp         | 2 +-
 src/libcamera/camera_manager.cpp | 6 +++++-
 3 files changed, 8 insertions(+), 3 deletions(-)

Patch

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index 4d1a4a9..7dd23d7 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -13,6 +13,7 @@ 
 #include <string>
 
 #include <libcamera/controls.h>
+#include <libcamera/object.h>
 #include <libcamera/request.h>
 #include <libcamera/signal.h>
 #include <libcamera/stream.h>
@@ -66,7 +67,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();
 		}
 	};
 
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index f60491d..c45bf33 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -164,9 +164,13 @@  void CameraManager::Private::cleanup()
 
 	/*
 	 * Release all references to cameras to ensure they all get destroyed
-	 * before the device enumerator deletes the media devices.
+	 * before the device enumerator deletes the media devices. Cameras are
+	 * destroyed via Object::deleteLater() API, hence we need to explicitly
+	 * process deletion requests from the thread's message queue as the event
+	 * loop is not in action here.
 	 */
 	cameras_.clear();
+	dispatchMessages(Message::Type::DeferredDelete);
 
 	enumerator_.reset(nullptr);
 }