From patchwork Thu Aug 26 13:23:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Dufresne X-Patchwork-Id: 13511 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 48FDBBD87C for ; Thu, 26 Aug 2021 13:24:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0675468925; Thu, 26 Aug 2021 15:24:02 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6771A68915 for ; Thu, 26 Aug 2021 15:24:00 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nicolas) with ESMTPSA id 7E3251F44332 From: Nicolas Dufresne To: libcamera-devel@lists.libcamera.org Date: Thu, 26 Aug 2021 09:23:44 -0400 Message-Id: <20210826132346.1238420-2-nicolas@ndufresne.ca> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210826132346.1238420-1-nicolas@ndufresne.ca> References: <20210826132346.1238420-1-nicolas@ndufresne.ca> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/3] gstreamer: Fix deadlock when last allocator ref is held by buffer 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: , Cc: Nicolas Dufresne Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nicolas Dufresne This deadlock occurs when a buffer is holding the last reference on the allocator. In gst_libcamera_allocator_release() we must drop the object lock before dropping the last ref of that object since the destructor will lock it again causing deadlock. This was notice while switching camera or resolution in Cheese software. Signed-off-by: Nicolas Dufresne Reviewed-by: Laurent Pinchart --- src/gstreamer/gstlibcameraallocator.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp index 7bd8ba2d..a8fa4f86 100644 --- a/src/gstreamer/gstlibcameraallocator.cpp +++ b/src/gstreamer/gstlibcameraallocator.cpp @@ -108,15 +108,18 @@ gst_libcamera_allocator_release(GstMiniObject *mini_object) { GstMemory *mem = GST_MEMORY_CAST(mini_object); GstLibcameraAllocator *self = GST_LIBCAMERA_ALLOCATOR(mem->allocator); - GLibLocker lock(GST_OBJECT(self)); - auto *frame = reinterpret_cast(gst_mini_object_get_qdata(mini_object, FrameWrap::getQuark())); - gst_memory_ref(mem); + { + GLibLocker lock(GST_OBJECT(self)); + auto *frame = reinterpret_cast(gst_mini_object_get_qdata(mini_object, FrameWrap::getQuark())); + + gst_memory_ref(mem); - if (frame->releasePlane()) { - auto *pool = reinterpret_cast(g_hash_table_lookup(self->pools, frame->stream_)); - g_return_val_if_fail(pool, TRUE); - g_queue_push_tail(pool, frame); + if (frame->releasePlane()) { + auto *pool = reinterpret_cast(g_hash_table_lookup(self->pools, frame->stream_)); + g_return_val_if_fail(pool, TRUE); + g_queue_push_tail(pool, frame); + } } /* Keep last in case we are holding on the last allocator ref. */