[{"id":19082,"web_url":"https://patchwork.libcamera.org/comment/19082/","msgid":"","date":"2021-08-25T23:02:32","subject":"Re: [libcamera-devel] [PATCH v1 3/3] libcamerasrc: Fix deadlock on\n\tEOS","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the patch.\n\nOn Wed, Aug 25, 2021 at 05:18:52PM -0400, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne \n> \n> It's not allowed in GStreamer to push events while holding the object\n> lock. This reduce the scope into which we hold the object lock. In\n> fact we don't need to protect against gst_task_resume() concurrency\n> when we stop the task as resume only do something if the task is paused.\n\nI've tried to check this, but overall I'll trust you on that as my\nknowledge is limited here.\n\n> This fixes a deadlock when running multiple instances of libcamerasrc\n> and closing one of the streaming window.\n> \n> Signed-off-by: Nicolas Dufresne \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/gstreamer/gstlibcamerasrc.cpp | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index bdd9f88c..2be44edd 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -312,12 +312,6 @@ gst_libcamera_src_task_run(gpointer user_data)\n> \t}\n> \n> \t{\n> -\t\t/*\n> -\t\t * Here we need to decide if we want to pause or stop the task. This\n> -\t\t * needs to happen in lock step with the callback thread which may want\n> -\t\t * to resume the task.\n> -\t\t */\n> -\t\tGLibLocker lock(GST_OBJECT(self));\n> \t\tif (ret != GST_FLOW_OK) {\n> \t\t\tif (ret == GST_FLOW_EOS) {\n> \t\t\t\tg_autoptr(GstEvent) eos = gst_event_new_eos();\n> @@ -332,6 +326,12 @@ gst_libcamera_src_task_run(gpointer user_data)\n> \t\t\treturn;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * Here we need to decide if we want to pause. This needs to\n> +\t\t * happen in lock step with the callback thread which may want\n> +\t\t * to resume the task and might push pending buffers.\n> +\t\t */\n> +\t\tGLibLocker lock(GST_OBJECT(self));\n> \t\tbool do_pause = true;\n> \t\tfor (GstPad *srcpad : state->srcpads_) {\n> \t\t\tif (gst_libcamera_pad_has_pending(srcpad)) {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1B10FBD87D\n\tfor ;\n\tWed, 25 Aug 2021 23:02:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 883D960505;\n\tThu, 26 Aug 2021 01:02:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F5E360504\n\tfor ;\n\tThu, 26 Aug 2021 01:02:45 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7CE8024F;\n\tThu, 26 Aug 2021 01:02:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"K7/K6QA6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629932564;\n\tbh=DDWx361gth50iG/H+9+HIU57yr2SpKHZb6JBgRAxD9c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K7/K6QA6spLGQIfjmdUSs+3ccHo7iU/IHBiiWdBnxeUMvN43Blx/0MEYPdDoUNtiy\n\tMeOX2z5TmQ0ihvMznrse5bu98ZyOrAWpDg/XP06xG8F9oklmwOBykfSiG4D25dJnXT\n\tcmLt9rUYmBtvsbZh8NRCeCKcA3NF41JFI9WWkToY=","Date":"Thu, 26 Aug 2021 02:02:32 +0300","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Message-ID":"","References":"<20210825211852.1207168-1-nicolas@ndufresne.ca>\n\t<20210825211852.1207168-4-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210825211852.1207168-4-nicolas@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] libcamerasrc: Fix deadlock on\n\tEOS","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]