From patchwork Thu Aug 26 13:23:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Dufresne X-Patchwork-Id: 13513 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 2BC7BC3242 for ; Thu, 26 Aug 2021 13:24:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 42F9268921; Thu, 26 Aug 2021 15:24:05 +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 59ACD6890C for ; Thu, 26 Aug 2021 15:24:02 +0200 (CEST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: nicolas) with ESMTPSA id 7BA1A1F44332 From: Nicolas Dufresne To: libcamera-devel@lists.libcamera.org Date: Thu, 26 Aug 2021 09:23:46 -0400 Message-Id: <20210826132346.1238420-4-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 3/3] libcamerasrc: Fix deadlock on EOS 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 It's not allowed in GStreamer to push events while holding the object lock. This reduce the scope into which we hold the object lock. In fact we don't need to protect against gst_task_resume() concurrency when we stop the task as resume only do something if the task is paused. This fixes a deadlock when running multiple instances of libcamerasrc and closing one of the streaming window. Signed-off-by: Nicolas Dufresne Reviewed-by: Laurent Pinchart --- src/gstreamer/gstlibcamerasrc.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index bdd9f88c..2be44edd 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -312,12 +312,6 @@ gst_libcamera_src_task_run(gpointer user_data) } { - /* - * Here we need to decide if we want to pause or stop the task. This - * needs to happen in lock step with the callback thread which may want - * to resume the task. - */ - GLibLocker lock(GST_OBJECT(self)); if (ret != GST_FLOW_OK) { if (ret == GST_FLOW_EOS) { g_autoptr(GstEvent) eos = gst_event_new_eos(); @@ -332,6 +326,12 @@ gst_libcamera_src_task_run(gpointer user_data) return; } + /* + * Here we need to decide if we want to pause. This needs to + * happen in lock step with the callback thread which may want + * to resume the task and might push pending buffers. + */ + GLibLocker lock(GST_OBJECT(self)); bool do_pause = true; for (GstPad *srcpad : state->srcpads_) { if (gst_libcamera_pad_has_pending(srcpad)) {