From patchwork Fri Jul 31 18:14:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 9122 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 8F5B7BD879 for ; Fri, 31 Jul 2020 18:14:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4BDF161F1B; Fri, 31 Jul 2020 20:14:23 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="hdIwWGIg"; dkim-atps=neutral Received: from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 991A7611A2 for ; Fri, 31 Jul 2020 20:14:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com; h=from:subject:in-reply-to:references:mime-version:to:cc: content-transfer-encoding:content-type; s=s1; bh=G8aUrpC88hKULTyVG1vQulK2b6B6VEjsFwMbdsHK/YY=; b=hdIwWGIgkAaT1ISjyCWRbRI9LGb8pNxkXUanp/HHiYMi8sHeEpA7OyreDITTJjX0NArP ZvMNzmdLFUjD/QDzmapZ+OkAkkxIJJG6giCd+6NcitkkFe7QUZQNMJjG2VBjwBLxSUWNw2 Vufj8WW8O6x1jcHN5Kf7tmbnsFaC/u0WY= Received: by filterdrecv-p3mdw1-7ff865655c-svvjh with SMTP id filterdrecv-p3mdw1-7ff865655c-svvjh-19-5F245F7C-4D 2020-07-31 18:14:20.612016378 +0000 UTC m=+172687.713613617 Received: from mail.uajain.com (unknown) by ismtpd0005p1hnd1.sendgrid.net (SG) with ESMTP id grWcS78XREGRNAHZ4crFTQ Fri, 31 Jul 2020 18:14:20.177 +0000 (UTC) From: Umang Jain Date: Fri, 31 Jul 2020 18:14:20 +0000 (UTC) Message-Id: <20200731181410.99892-5-email@uajain.com> In-Reply-To: <20200731181410.99892-1-email@uajain.com> References: <20200731181410.99892-1-email@uajain.com> Mime-Version: 1.0 X-SG-EID: 1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPcj9TjcWbOI2k4lJ0fKJaeeyGQUzl0aZlU4nehjRj03/+6xzpI2FEkb70FYM1DwSIZgISXCR0j8IJXDzCROJ0Ix5AxU5khER5WZoxJ47OpOcEyJg7MS3cLo469M1zAho7xfkhluQzEytX8/lB8Wt/RD+LmASzQiSVt7T7+L3ujfLNdw3vxHDVUohfXyJ4BfFV1yMoxFhV0ZesepVGbmmlueA== To: libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v4 4/4] libcamera: camera: Ensure deletion via deleteLater() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Reviewed-by: Laurent Pinchart --- include/libcamera/camera.h | 3 ++- src/libcamera/camera.cpp | 2 +- src/libcamera/camera_manager.cpp | 6 +++++- 3 files changed, 8 insertions(+), 3 deletions(-) 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 #include +#include #include #include #include @@ -66,7 +67,7 @@ protected: std::vector config_; }; -class Camera final : public std::enable_shared_from_this +class Camera final : public Object, public std::enable_shared_from_this { public: static std::shared_ptr 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::create(PipelineHandler *pipe, struct Deleter : std::default_delete { 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); }