From patchwork Wed Aug 25 21:18:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Dufresne X-Patchwork-Id: 13494 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 0C4F2BD87D for ; Wed, 25 Aug 2021 21:19:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D537968893; Wed, 25 Aug 2021 23:19:14 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 03F9760288 for ; Wed, 25 Aug 2021 23:19:13 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nicolas) with ESMTPSA id 49C0E1F435D8 From: Nicolas Dufresne To: libcamera-devel@lists.libcamera.org Date: Wed, 25 Aug 2021 17:18:50 -0400 Message-Id: <20210825211852.1207168-2-nicolas@ndufresne.ca> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210825211852.1207168-1-nicolas@ndufresne.ca> References: <20210825211852.1207168-1-nicolas@ndufresne.ca> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v1 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. */