From patchwork Mon Jun 23 16:34:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 23627 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 493A8BDE6B for ; Mon, 23 Jun 2025 16:34:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5D5F168DE3; Mon, 23 Jun 2025 18:34:20 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="bq9SEaIl"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F0E4968DC9 for ; Mon, 23 Jun 2025 18:34:18 +0200 (CEST) Received: from Monstersaurus.hippo-penny.ts.net (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EAFC410D4; Mon, 23 Jun 2025 18:34:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1750696442; bh=J3ay8Bis3+vdhIozP2Ro1shCrOX1tSkT8hcJMeO1NLc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bq9SEaIlPyUgdV2naVRm3aEyH44Fi2mIsPvO9SjrRmyM0o+nUYzKvdhUgd4Y+StuR TV6bjWqThYRFMsNW1qhUqfuBGEzQbNmdIrJWBE68veRp6PpR1sxILh8A8j1gz8mAsi OnLb3iwSJArP/VUVefHng6xM7FOFRGwx+6X4J33w= From: Kieran Bingham To: libcamera devel , stefan.klug@ideasonboard.com Cc: Kieran Bingham Subject: [PATCH] libcamera: pipeline_handler: cancel waiting requests first Date: Mon, 23 Jun 2025 17:34:14 +0100 Message-ID: <20250623163414.3936727-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250526214224.13631-1-stefan.klug@ideasonboard.com> References: <20250526214224.13631-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When stopping a camera, we may now find that there are requests queued to the camera but not yet available in the pipeline handler. These are "waitingReqeusts" which are not yet queued to the device. While stopping a camera, we ask the pipeline to stop with stopDevice() and then cancel any waiting requests after. This however can lead to a race where having stopped the camera and completed the requests, the pipeline may try to further consume requests from the waiting lists. When we call PipelineHandler::stop() we know that we can not process any of the requests which are sat in the waiting queue, therefore cancel those first before asking the pipeline handler to complete or cancel and requests that it has ownership of. Signed-off-by: Kieran Bingham --- Hi Stefan, Testing your series I discovered this shutdown race. Without this I get: [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 ;34mrkisp1_ipa_proxy.cpp:467 assertion "state_ == ProxyRunning" failed in queueRequestThread() Backtrace: /lib/libcamera.so.0.5(_ZN9libcamera3ipa6rkisp114IPAProxyRkISP118queueRequestThreadEjRKNS_11ControlListE+0x94) [0xffffb32d[ 6490.810893] audit: type=1334 audit(1741278393.984:116): prog-id=51 op=LOAD 7f8c] /lib/libcamera.so.0.5(_ZN9[ 6490.819683] audit: type=1334 audit(1741278393.996:117): prog-id=52 op=LOAD libcamera3ipa6rkisp114IPAProxyRkI[ 6490.820111] audit: type=1334 audit(1741278393.996:118): prog-id=53 op=LOAD SP112queueRequestEjRKNS_11ControlListE+0x28) [0xffffb32d8048] /lib/libcamera.so.0.5(_ZN9libcamera21PipelineHandlerRkISP118queueRequestDeviceEPNS_6CameraEPNS_7RequestE+0x50) [0xffffb3360b14] This patch ensures that requests which are in the waitingRequests queue are cancelled first to ensure they don't get processed by the pipeline after it has completed it's stop(). src/libcamera/pipeline_handler.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 0389f34486ab..9cd3504dc9db 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -360,9 +360,6 @@ void PipelineHandler::unlockMediaDevices() */ void PipelineHandler::stop(Camera *camera) { - /* Stop the pipeline handler and let the queued requests complete. */ - stopDevice(camera); - Camera::Private *data = camera->_d(); /* Cancel and signal as complete all waiting requests. */ @@ -372,8 +369,12 @@ void PipelineHandler::stop(Camera *camera) cancelRequest(request); } + /* Stop the pipeline handler and let the queued requests complete. */ + stopDevice(camera); + /* Make sure no requests are pending. */ ASSERT(data->queuedRequests_.empty()); + ASSERT(data->waitingRequests_.empty()); data->requestSequence_ = 0; }