[{"id":34710,"web_url":"https://patchwork.libcamera.org/comment/34710/","msgid":"<20250627184312.GB28633@pendragon.ideasonboard.com>","date":"2025-06-27T18:43:12","subject":"Re: [PATCH] libcamera: pipeline_handler: cancel waiting requests\n\tfirst","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jun 23, 2025 at 05:34:14PM +0100, Kieran Bingham wrote:\n> When stopping a camera, we may now find that there are requests\n> queued to the camera but not yet available in the pipeline handler.\n> \n> These are \"waitingReqeusts\" which are not yet queued to the device.\n> \n> While stopping a camera, we ask the pipeline to stop with stopDevice()\n> and then cancel any waiting requests after.\n> \n> This however can lead to a race where having stopped the camera and\n> completed the requests, the pipeline may try to further consume requests\n> from the waiting lists.\n> \n> When we call PipelineHandler::stop() we know that we can not process any\n> of the requests which are sat in the waiting queue, therefore cancel\n> those first before asking the pipeline handler to complete or cancel and\n> requests that it has ownership of.\n\nDoesn't the lead to requests completing out of order ? We should fix\nraces, but ensuring strict completion order is important.\n\n> Signed-off-by: Kieran Bingham \n> ---\n> \n> Hi Stefan,\n> \n> Testing your series I discovered this shutdown race.\n> \n> Without this I get:\n> \n> \n> \n> [1:48:10.770431125] [2675] FATAL default 6490.777390] audit: type=1701 audit(1741278393.952:115): auid=4294967295 uid=0 gid=0 ses=4294967295 pid=2674 comm=\"cam\" exe=\"/usr/bin/cam\" sig=6 res=1\n> ;34mrkisp1_ipa_proxy.cpp:467 assertion \"state_ == ProxyRunning\" failed in queueRequestThread()\n> Backtrace:\n> /lib/libcamera.so.0.5(_ZN9libcamera3ipa6rkisp114IPAProxyRkISP118queueRequestThreadEjRKNS_11ControlListE+0x94) [0xffffb32d[ 6490.810893] audit: type=1334 audit(1741278393.984:116): prog-id=51 op=LOAD\n> 7f8c]\n> /lib/libcamera.so.0.5(_ZN9[ 6490.819683] audit: type=1334 audit(1741278393.996:117): prog-id=52 op=LOAD\n> libcamera3ipa6rkisp114IPAProxyRkI[ 6490.820111] audit: type=1334 audit(1741278393.996:118): prog-id=53 op=LOAD\n> SP112queueRequestEjRKNS_11ControlListE+0x28) [0xffffb32d8048]\n> /lib/libcamera.so.0.5(_ZN9libcamera21PipelineHandlerRkISP118queueRequestDeviceEPNS_6CameraEPNS_7RequestE+0x50) [0xffffb3360b14]\n> \n> \n> This patch ensures that requests which are in the waitingRequests queue\n> are cancelled first to ensure they don't get processed by the pipeline\n> after it has completed it's stop().\n> \n> \n> src/libcamera/pipeline_handler.cpp | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 0389f34486ab..9cd3504dc9db 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -360,9 +360,6 @@ void PipelineHandler::unlockMediaDevices()\n> */\n> void PipelineHandler::stop(Camera *camera)\n> {\n> -\t/* Stop the pipeline handler and let the queued requests complete. */\n> -\tstopDevice(camera);\n> -\n> \tCamera::Private *data = camera->_d();\n> \n> \t/* Cancel and signal as complete all waiting requests. */\n> @@ -372,8 +369,12 @@ void PipelineHandler::stop(Camera *camera)\n> \t\tcancelRequest(request);\n> \t}\n> \n> +\t/* Stop the pipeline handler and let the queued requests complete. */\n> +\tstopDevice(camera);\n> +\n> \t/* Make sure no requests are pending. */\n> \tASSERT(data->queuedRequests_.empty());\n> +\tASSERT(data->waitingRequests_.empty());\n> \n> \tdata->requestSequence_ = 0;\n> }","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 0C358BDCBF\n\tfor ;\n\tFri, 27 Jun 2025 18:43:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BCC2F68E04;\n\tFri, 27 Jun 2025 20:43:36 +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 7D5B968DE5\n\tfor ;\n\tFri, 27 Jun 2025 20:43:35 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 83BAA16A;\n\tFri, 27 Jun 2025 20:43:15 +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=\"m9bb6KL0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751049795;\n\tbh=kq+KKZMMP00wuCtRLNfoGvQZI03aptf07Ydl42U09y0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=m9bb6KL0UxeQubuS2LklKbA1TrDjdwkKpIqC31z3gz7BY6nk+yMa1MH8vnpyW+8WQ\n\toNY/MdoUqKK/AvZjZ9axDL/UxFwY5yJk+ZsqKHC2z0K5zo4MuA3XldCRY74lRK4OHa\n\t+quEj5eNIiMSr0xzqZut3QoSF0YKM4wtuimY/1qE=","Date":"Fri, 27 Jun 2025 21:43:12 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera devel ,\n\tstefan.klug@ideasonboard.com","Subject":"Re: [PATCH] libcamera: pipeline_handler: cancel waiting requests\n\tfirst","Message-ID":"<20250627184312.GB28633@pendragon.ideasonboard.com>","References":"<20250526214224.13631-1-stefan.klug@ideasonboard.com>\n\t<20250623163414.3936727-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250623163414.3936727-1-kieran.bingham@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]