From patchwork Fri Nov 8 23:34:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 21873 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 5E0B7C323E for ; Fri, 8 Nov 2024 23:34:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 982BD657B4; Sat, 9 Nov 2024 00:34:21 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Iqv1dYJr"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DF38265474 for ; Sat, 9 Nov 2024 00:34:19 +0100 (CET) Received: from charm.tail69b4.ts.net (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D11646DF; Sat, 9 Nov 2024 00:34:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1731108849; bh=pqeOiyy2CmdLXV7MLW6R02YIsSMudnY2HNR4FJY75gU=; h=From:To:Cc:Subject:Date:From; b=Iqv1dYJr7Ckkk2+MSlRq6M0PtZqZ517dZbO++UYDPBLaE5f+A5wx97kWp9cyDx3iy WrxYK2bEbWpt561/iX7FJoBl848Rd6D5kjlrpUZLd8J5Xnf2po28x9N+nlSga5sWF4 O1qvwiupuODWp2s8HMGtp0CrDkjN9zvEHzo91A88= From: Kieran Bingham To: libcamera devel Cc: Kieran Bingham Subject: [PATCH] CameraManager: Ensure we cleanup on failure Date: Fri, 8 Nov 2024 23:34:15 +0000 Message-ID: <20241108233415.25782-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.47.0 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" If the CameraManager fails to initialise at startup during CameraManager::Private::init(), then the run() function will prepare the thread for operations, but the thread will immediately close without executing any cleanup. This can leave instantiated objects such as the EventNotifier registered by the udev enumerator constructed in a thread which no longer exists. The destructor of those objects can then fire an assertion that they are being executed from an incorrect thread while performing their cleanup. Ensure that the failure path does not result in reporting thread ownership assertions by performing cleanup correctly on the CameraManager thread before it closes. Signed-off-by: Kieran Bingham Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- This is the result of debugging an awkward error on an unrelated failure path that caused the CameraManager to fail to construct: """ camera_sensor_legacy.cpp:850 'ar0144 2-0010': The analogue crop rectangle has been defaulted to the active area size [1:49:26.079788375] [922] WARN RkISP1Agc agc.cpp:46 'AeMeteringMode' parameter not found in tuning file [1:49:26.079829125] [922] WARN RkISP1Agc agc.cpp:69 No metering modes read from tuning file; defaulting to matrix [1:49:26.080311125] [921] ERROR Camera camera_manager.cpp:350 Failed to start camera manager: No such file or directory Failed to start camera manager: No such file or directory [1:49:26.080547875] [921] ERROR Object object.cpp:252 EventNotifier can't be enabled from another thread [1:49:26.080582375] [921] FATAL default object.cpp:253 assertion "false" failed in assertThreadBound() Backtrace: /usr/lib/libcamera-base.so.0.3(_ZN9libcamera6Object17assertThreadBoundEPKc+0xe4) [0xffffa35fd884] /usr/lib/libcamera-base.so.0.3(_ZN9libcamera13EventNotifier10setEnabledEb+0x24) [0xffffa3601b34] /usr/lib/libcamera-base.so.0.3(_ZN9libcamera13EventNotifierD2Ev+0x50) [0xffffa3601bf0] /usr/lib/libcamera-base.so.0.3(_ZN9libcamera13EventNotifierD0Ev+0x18) [0xffffa3601cb8] /usr/lib/libcamera.so.0.3(_ZN9libcamera20DeviceEnumeratorUdevD2Ev+0x38) [0xffffa3798698] /usr/lib/libcamera.so.0.3(_ZN9libcamera20DeviceEnumeratorUdevD0Ev+0x18) [0xffffa3798738] /usr/lib/libcamera.so.0.3(+0x997cc) [0xffffa36d97cc] /usr/lib/libcamera.so.0.3(_ZN9libcamera13CameraManagerD1Ev+0x134) [0xffffa36d6f04] /usr/lib/libcamera.so.0.3(_ZN9libcamera13CameraManagerD0Ev+0x18) [0xffffa36d6f78] cam(+0x1ba60) [0xaaaad8d9ba60] cam(+0x8b64) [0xaaaad8d88b64] /usr/lib/libc.so.6(+0x284b4) [0xffffa30984b4] /usr/lib/libc.so.6(__libc_start_main+0x9c) [0xffffa309858c] cam(+0x8fb0) [0xaaaad8d88fb0] Aborted (core dumped) """ Whilst the error "No such file or directory" reported above was out of tree code causing the camera manager to fail - libcamera should not then fail with an assertion after that while shutting down and calling destructors. Signed-off-by: Kieran Bingham --- src/libcamera/camera_manager.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 6280b2b3e485..a8b36bcea2d4 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -83,8 +83,10 @@ void CameraManager::Private::run() mutex_.unlock(); cv_.notify_one(); - if (ret < 0) + if (ret < 0) { + cleanup(); return; + } /* Now start processing events and messages. */ exec();