From patchwork Sun Jan 21 03:59:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19422 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 6F41CC3243 for ; Sun, 21 Jan 2024 03:59:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 227F362916; Sun, 21 Jan 2024 04:59:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809589; bh=YKMrmR+f1h9TsYaiD3Ie7x5llPfWyBSX3GKqx+lg7W8=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ZOLwqUKScIT7a5CtJSgXQV/C6vzfGc9EirBuaqcjtbZLUXcsxoEknXEr9FEzvnodw OQHjZG0F2Ua3BBxzuCcBJT1w0sZpurvem7eDChpsVUDaCb7Dk4EaS3/oaxJrX+Vwxp KKi7MruO8fYQXm22vsRrqxB6MuxbSj+pRZ15H/quyMtg1Yr9ubB4qakbuj2To7cZve jDGW/STzGD6pHFY09UmDZbLVL1sVA85EOGnMZ4Op+O5qsWVc+MYldyQU6bflfCmXGw +2gRdpPWytffago8I5woQIpo6WtXB0kquKlmVALbiklO9CTT2uifLTF5hAmBLcCFM3 nGhm7W9e8oNvg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E1810628B6 for ; Sun, 21 Jan 2024 04:59:46 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="JJE4J2nz"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D8335138B; Sun, 21 Jan 2024 04:58:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809515; bh=YKMrmR+f1h9TsYaiD3Ie7x5llPfWyBSX3GKqx+lg7W8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JJE4J2nzKrJGH45Kv1A64l6hHM5Y8KSdT+RmjWvawP5Rz+0W/UTcg3MY15gGo498h 0Y8Ke5LLBlrxJDgq/p+EXsnxMkVDKrzrewx0F/9bA1oumw8CLEZDT/n9wfMkalhhXo 6vbuk5w37+hSfmxyM1TZU5nBnR6NC1zQLp1zK8pI= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:37 +0200 Message-ID: <20240121035948.4226-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 01/12] libcamera: object: Fix thread-bound reference in documentation 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The Object::message() function is documented as thread-bound without using the correct \threadbound reference. Fix it to ensure it gets included in the thread safety context list. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- src/libcamera/base/object.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp index 92cecd22fbe9..1fce5a2af9af 100644 --- a/src/libcamera/base/object.cpp +++ b/src/libcamera/base/object.cpp @@ -259,7 +259,7 @@ void Object::message(Message *msg) * Moving an object that has a parent is not allowed, and causes undefined * behaviour. * - * \context This function is thread-bound. + * \context This function is \threadbound. */ void Object::moveToThread(Thread *thread) { From patchwork Sun Jan 21 03:59:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19423 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 CF76FC32BC for ; Sun, 21 Jan 2024 03:59:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A273B6293E; Sun, 21 Jan 2024 04:59:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809589; bh=EEM3IQqZiHYZ5usYYAwswhlLDeBS4Vs4LmZfGpSEkXo=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ajzwazlMk6ShFFGBFZnvmgw0oN4cjm6aDb5O54lpIvl3dgyDB6IzXg4McNaGKyB4s efx9e4mo5aNY7bC0HMG2e9HEql6OOtkdk6Q40ZE6QFNAEAneaf7cAfLMloYiTloDpz VQWy808y4yLdq+B+V+60NSNcrqrGB+j0djaVag0cOIPt/Th7KS5cCM6ZFWvCxt2IRi pQOc1PhV5liW1FWQb4XUdnfHD/kR0fv97LmRiMSqnzuxZ7o0HOxJXLhb7HamtsdUc7 wAkHeTVNCVFlcng8Aa4mo+BFw/no2F5X579YwolXuy0SC8Qq5i9cw4kufEHWdKYpcW wteblVcHPlR4w== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9267661D30 for ; Sun, 21 Jan 2024 04:59:48 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="gm/+XGs8"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 839288D4; Sun, 21 Jan 2024 04:58:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809516; bh=EEM3IQqZiHYZ5usYYAwswhlLDeBS4Vs4LmZfGpSEkXo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gm/+XGs85T27UA1Xe5kSzu4zJyKI3lpXUDl0f/tOVEcwH1PeqvzSislT05AW5wzRO dzZPT44BLVysl/AapDIoYAzaEnf9tmWjLjWckfC4aPNCdrS7C0nahWMWjIATu/gwok YneAwsE3lhgQ+9ikKAQ/MqvL6AHg6lVDjLFeRvuI= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:38 +0200 Message-ID: <20240121035948.4226-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 02/12] libcamera: signal: Replace object.h inclusion with forward declatation 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The signal.h header doesn't need to include object.h. Replace it with a forward declaration, and instead include object.h in source files that require it. To can speed up compilation a little bit, but more importantly avoids unintended dependencies from the Signal class to the Object class to be added later as the compiler will catch them. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- include/libcamera/base/signal.h | 3 ++- src/libcamera/base/bound_method.cpp | 1 + src/libcamera/base/signal.cpp | 1 + src/libcamera/base/thread.cpp | 1 + test/event-thread.cpp | 1 + test/ipa/ipa_interface_test.cpp | 1 + test/message.cpp | 1 + test/signal-threads.cpp | 1 + test/timer-thread.cpp | 1 + .../ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl | 1 + 10 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/libcamera/base/signal.h b/include/libcamera/base/signal.h index 841e4b4ca15c..444997b4525d 100644 --- a/include/libcamera/base/signal.h +++ b/include/libcamera/base/signal.h @@ -13,10 +13,11 @@ #include #include -#include namespace libcamera { +class Object; + class SignalBase { public: diff --git a/src/libcamera/base/bound_method.cpp b/src/libcamera/base/bound_method.cpp index 3ecec51c4b75..c83d623f107d 100644 --- a/src/libcamera/base/bound_method.cpp +++ b/src/libcamera/base/bound_method.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include diff --git a/src/libcamera/base/signal.cpp b/src/libcamera/base/signal.cpp index a46386a05abf..de51d060c21a 100644 --- a/src/libcamera/base/signal.cpp +++ b/src/libcamera/base/signal.cpp @@ -8,6 +8,7 @@ #include #include +#include /** * \file base/signal.h diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp index b96951ac19ba..75693c92a0b1 100644 --- a/src/libcamera/base/thread.cpp +++ b/src/libcamera/base/thread.cpp @@ -18,6 +18,7 @@ #include #include #include +#include /** * \page thread Thread Support diff --git a/test/event-thread.cpp b/test/event-thread.cpp index ef8a52c3de55..88a8c07ef9f0 100644 --- a/test/event-thread.cpp +++ b/test/event-thread.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 051ef96e7ed2..56f3cd6d57ba 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include diff --git a/test/message.cpp b/test/message.cpp index d148a13d6c7a..0e76f323e3b9 100644 --- a/test/message.cpp +++ b/test/message.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include "test.h" diff --git a/test/signal-threads.cpp b/test/signal-threads.cpp index d5e2eb662df2..8c550eb014d8 100644 --- a/test/signal-threads.cpp +++ b/test/signal-threads.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include diff --git a/test/timer-thread.cpp b/test/timer-thread.cpp index 618217538779..0bcd0d8ce194 100644 --- a/test/timer-thread.cpp +++ b/test/timer-thread.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index ed270f5cd49c..6e823598930e 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -18,6 +18,7 @@ #include #include +#include #include #include "libcamera/internal/control_serializer.h" From patchwork Sun Jan 21 03:59:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19424 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 0FD44C323E for ; Sun, 21 Jan 2024 03:59:54 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B76C76294B; Sun, 21 Jan 2024 04:59:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809593; bh=4/6ICExn9z3UFsYl6U2WMNHXAzdh7EjXq0WmhmVa7xc=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=K5Z2hzgT84sAqHn+44SK9jkoja48Jp7WFCUjwoc9dDEKsiTlmH/A658tryS7ZWnwA FERUopbGIQzh8aeSASJaGaLv+UxpHkBNXf/js+RGtldeWgQ445IP9WKBb5UrtXwG30 QBkot7qKSAUTzmfbQR0hPkIDXcnTzwW/jq3w/ms/8rtMqrbKS4A8iSLVC9nRDI86RK Tojk1OA2kxraQylzr34hvF1i0KOzPHOskGD+kIgSc5om9e+f2aGttwtvfKaGh1ezF/ 3r/JkQVEsFBdGVlMvuNATBTYbLM4I1bsWXe985ATxu9sJu2k+/uDSjCrHOWmCyaHRM 0lATzuu0ozeow== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A173628E9 for ; Sun, 21 Jan 2024 04:59:50 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="FIE6cEmd"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D26D81574; Sun, 21 Jan 2024 04:58:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809518; bh=4/6ICExn9z3UFsYl6U2WMNHXAzdh7EjXq0WmhmVa7xc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FIE6cEmdXLpQpjLMDF7ShALP9DJGwvr7J0Tj4xlPDS2L7r6uEBSsQ1Zs8g7o9MKNV rtDmIMritcRRXLIp6yoPy3kY18zj26Fqz/aMjeGYyL0Z+w8cQfqLD1pmfNQDSxZAUE hLhgc9EaGZdN1FIo5sFQIOfnvp07QJUF61wMz0j0= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:39 +0200 Message-ID: <20240121035948.4226-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 03/12] test: object-delete: Test deferred delete just before thread stops 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The Object::deleterLater() function is expected to not race with stopping the thread the object is bound to. Add a test for this. The test currently fails, demonstrating a bug in libcamera. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- test/object-delete.cpp | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/test/object-delete.cpp b/test/object-delete.cpp index eabefe935974..80b7dc41cd37 100644 --- a/test/object-delete.cpp +++ b/test/object-delete.cpp @@ -33,10 +33,10 @@ public: unsigned int *deleteCount_; }; -class NewThread : public Thread +class DeleterThread : public Thread { public: - NewThread(Object *obj) + DeleterThread(Object *obj) : object_(obj) { } @@ -63,9 +63,9 @@ protected: unsigned int count = 0; TestObject *obj = new TestObject(&count); - NewThread thread(obj); - thread.start(); - thread.wait(); + DeleterThread delThread(obj); + delThread.start(); + delThread.wait(); Thread::current()->dispatchMessages(Message::Type::DeferredDelete); @@ -89,6 +89,26 @@ protected: return TestFail; } + /* + * Test that deleteLater() works properly when called just + * before the object's thread exits. + */ + Thread boundThread; + boundThread.start(); + + count = 0; + obj = new TestObject(&count); + obj->moveToThread(&boundThread); + + obj->deleteLater(); + boundThread.exit(); + boundThread.wait(); + + if (count != 1) { + cout << "Object deletion right before thread exit failed (" << count << ")" << endl; + return TestFail; + } + return TestPass; } }; From patchwork Sun Jan 21 03:59:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19425 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 C9BC9C3243 for ; Sun, 21 Jan 2024 03:59:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6F2D16293E; Sun, 21 Jan 2024 04:59:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809596; bh=gqUGYcK7brqQTRQ3ldW7HRkE0TmtK5kwwdbwRLBWFxc=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=NPNjqHYTlcueuaDWYvrgfgo7aotbXTa85qrhV1hTjIrE6/gtT45SSnUQw36jjB4z8 5241tGLfPZHD7Qy9YKPgb03zsblCFnGHt4AnNYse08A0peIaXlAXXTvHQ06cjseCbT 8rJLObPTLzghaDQbVxl7c7ROtZu56YkvoAshpliaV2odMoR03aHD+DkexMMvzMeOwF N6WeXAR6RmKLJ32wKfs+7cann4ThuAiycPfDeYogaX7OPGZAcyY2CjN0mdNF0WlXvD 5kshya9n3YHl7+hQ9qVBAo4tDF+ECynqoF28In0mTk2v94vC8pNX+GerJB6rHMALvx 8YN4qFAlLcN1Q== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 52C5D62947 for ; Sun, 21 Jan 2024 04:59:51 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LDgqOCaa"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4066B138B; Sun, 21 Jan 2024 04:58:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809519; bh=gqUGYcK7brqQTRQ3ldW7HRkE0TmtK5kwwdbwRLBWFxc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LDgqOCaaJ0bKmk4XhjWG0/VHdcZm6diG/5v/DNK7GMKBS56aMA1/zc9+jDvZMiwy3 ISzcBqMDCgO/T45TGAU8WygrHm393zkMP0KCvRJKcsA1L7A1cdHAu5kDCJxvbYtE9H IWvD4FyxD73rLWSiKrPePgh5UBlKBKC2vUKdBYaw= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:40 +0200 Message-ID: <20240121035948.4226-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 04/12] libcamera: thread: Ensure deferred deletion of all objects before stopping 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Objects can be scheduled for deletion with Object::deleteLater(), which queues a deferred deletion to the thread's event loop. As the deleteLater() function is meant to be called from a different thread, this may race with thread termination, and deferred deletions queued just before calling Thread::exit() may not be processed by the event loop. Make sure they get processed when finishing the thread, before stopping. This fixes a failure in the object-delete unit test. Signed-off-by: Laurent Pinchart --- src/libcamera/base/thread.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp index 75693c92a0b1..4ac72036aa69 100644 --- a/src/libcamera/base/thread.cpp +++ b/src/libcamera/base/thread.cpp @@ -371,6 +371,12 @@ void Thread::run() void Thread::finishThread() { + /* + * Objects may have been scheduled for deletion right before the thread + * exited. Ensure they get deleted now, before the thread stops. + */ + dispatchMessages(Message::Type::DeferredDelete); + data_->mutex_.lock(); data_->running_ = false; data_->mutex_.unlock(); From patchwork Sun Jan 21 03:59:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19426 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 84310C32BC for ; Sun, 21 Jan 2024 03:59:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 253F562945; Sun, 21 Jan 2024 04:59:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809597; bh=ezRIQuCZBnWVllkKVhQA2PQQBoAV/G8PQmQ/Z/WIfGs=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=vUZuDwqpo7g+zhmfSf05JOpv3OIqN4nl/8oBh27lP45AoNXs7vvnxW7WHIzVqLvCM 82RlIHrvHxWJukUuc9Ch9dvuV5dY1sYW09pECkA+9dAtl23nh5bbiJGU/Pg73BMVfe HnLXaSUgXFVwUpS/rRfLbRIrFJNxTr724uH7B7irJDaWtLC1USAKUNeffrriGRnvdp IhAlMg8wREEjLgo8WvAK2amWIzKBjQQn2fSO5Wl2g8pKcOQqgqzB5cPy57BPtQubBd DjnOmTVoK7x7btdi24yAWKNXbD3x+21pHJZZtJG2xOsSkF5IvzQQQqynnAOxKK5ELS 7EG71x/658CGw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A00946293E for ; Sun, 21 Jan 2024 04:59:52 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="c9s0JG50"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8DD798D4; Sun, 21 Jan 2024 04:58:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809520; bh=ezRIQuCZBnWVllkKVhQA2PQQBoAV/G8PQmQ/Z/WIfGs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=c9s0JG50zOgsJ9VKKgzV83VYFtU1rRNDiMxqO5XYugRO6+4MXFNfQJZMHHxNCBgd4 9I2nT7X4jakbXYBXGfPtxqMfOZ6fiAzRykwhvD1lit2qTqAwS+WTpK2xW5+37oCeZS gV2+xWSg//DznIv0AuIT/nOGRUpSp9XF1o9z4qto= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:41 +0200 Message-ID: <20240121035948.4226-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 05/12] test: event-thread: Destroy Object from correct thread context 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The EventHandler used in the test is destroyed from the main thread, which is invalid for a thread-bound object bound to a different thread. Fix it by destroying it with deleteLater(). Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- test/event-thread.cpp | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/test/event-thread.cpp b/test/event-thread.cpp index 88a8c07ef9f0..d6e5d27af185 100644 --- a/test/event-thread.cpp +++ b/test/event-thread.cpp @@ -85,10 +85,17 @@ private: class EventThreadTest : public Test { protected: + int init() + { + thread_.start(); + + handler_ = new EventHandler(); + + return TestPass; + } + int run() { - Thread thread; - thread.start(); /* * Fire the event notifier and then move the notifier to a @@ -98,23 +105,33 @@ protected: * different thread will correctly process already pending * events in the new thread. */ - EventHandler handler; - handler.notify(); - handler.moveToThread(&thread); + handler_->notify(); + handler_->moveToThread(&thread_); this_thread::sleep_for(chrono::milliseconds(100)); - /* Must stop thread before destroying the handler. */ - thread.exit(0); - thread.wait(); - - if (!handler.notified()) { + if (!handler_->notified()) { cout << "Thread event handling test failed" << endl; return TestFail; } return TestPass; } + + void cleanup() + { + /* + * Object class instances must be destroyed from the thread + * they live in. + */ + handler_->deleteLater(); + thread_.exit(0); + thread_.wait(); + } + +private: + EventHandler *handler_; + Thread thread_; }; TEST_REGISTER(EventThreadTest) From patchwork Sun Jan 21 03:59:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19427 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 4E8EBC32BD for ; Sun, 21 Jan 2024 03:59:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D206562951; Sun, 21 Jan 2024 04:59:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809597; bh=ypHTeS71nz6sHKStFmfA7pQqOYKn6VXfE0idipTqYPA=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=PgGu32Ef7vs8DQH93G4hpCHUUPYa3voFG5EaNBeQ4bclNCQHwD0MSbWs5ciLlaI9k p+3IDQ5WV9RosG58ha/dUwOl66If2sbOny40eETke3H3cZyqApGiIQtkWk6hcAwz6X 2TfMBQfrgs4FaXVvmCAlKN2sfi8R3SVVaPWGAO1Q7q29wpBaENC3nmcRsfSbJ7vqd9 JTetL4LiVVQyR3z0awcN5CAIg5R50tIxKQwpKNuijiErqnhLXkHDFMJU6a+KyPkzXH pdWfzZMX963S+PttSIbx8Dvr6W1y45CCszvYD+5X6zr8TGa0yvnTP6Gz1hysTPLqT/ awFdPMkgkexJA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DB46F6294C for ; Sun, 21 Jan 2024 04:59:53 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="NYRdiHf2"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CCD44138B; Sun, 21 Jan 2024 04:58:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809522; bh=ypHTeS71nz6sHKStFmfA7pQqOYKn6VXfE0idipTqYPA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NYRdiHf23eRIhYhHgD1Mf2ZAyZ5dOv+pP8cqpoDTMFdypesU3zb/tAMIDC9QDsqje ACawEhofWqxA8I0MdR7v8KdHEIMSm3YQgmvNAw+ldL60yUBkO+qoIteRUanvOQkYQE kljpeq1DhM7U1YkgdFxRffkelcW9GE7BxzqIMNTE= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:42 +0200 Message-ID: <20240121035948.4226-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 06/12] test: message: Remove incorrect slow receiver test 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The slow receiver test verifies there's no race condition between concurrent message delivery and object deletion. This is no a valid use case in the first place, as objects are not allowed to be deleted from a different thread than the one they are bound to. Remove the incorrect test. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- test/message.cpp | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/test/message.cpp b/test/message.cpp index 0e76f323e3b9..a34e0f0b5e10 100644 --- a/test/message.cpp +++ b/test/message.cpp @@ -93,25 +93,6 @@ private: bool success_; }; -class SlowMessageReceiver : public Object -{ -protected: - void message(Message *msg) - { - if (msg->type() != Message::None) { - Object::message(msg); - return; - } - - /* - * Don't access any member of the object here (including the - * vtable) as the object will be deleted by the main thread - * while we're sleeping. - */ - this_thread::sleep_for(chrono::milliseconds(100)); - } -}; - class MessageTest : public Test { protected: @@ -148,21 +129,6 @@ protected: break; } - /* - * Test for races between message delivery and object deletion. - * Failures result in assertion errors, there is no need for - * explicit checks. - */ - SlowMessageReceiver *slowReceiver = new SlowMessageReceiver(); - slowReceiver->moveToThread(&thread_); - slowReceiver->postMessage(std::make_unique(Message::None)); - - this_thread::sleep_for(chrono::milliseconds(10)); - - delete slowReceiver; - - this_thread::sleep_for(chrono::milliseconds(100)); - /* * Test recursive calls to Thread::dispatchMessages(). Messages * should be delivered correctly, without crashes or memory From patchwork Sun Jan 21 03:59:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19428 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 E24C3C323E for ; Sun, 21 Jan 2024 03:59:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8486C6294F; Sun, 21 Jan 2024 04:59:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809598; bh=KC/mNBwsTvo7jbNoWkM9MEDV6p6gdf3XmassPtSI1BY=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=20HNywIfbZGB/d7nREhT/oNCkqMVORoaK2SyTeczMzQ43pDbpqdGSm1nQbNfdV7F/ +YkT+jMVeLEVustz42KtkLQ8dXlNOf+uDrSOr4/2c0tBrcuaDkfQpA3XdgK/aDmmu+ FQacjdJL+HW++Bbwj+8JL0V1LGhcVQDlgPSgW4YJOAtclZSGwDwJn0D00QPcoCSUse YzMYy1srmq7aatdEmAI/1Q4X63Y8IOI2Gx1daUQRlz2j9T7zMBbd9PWNJPVDfWGmwI YqHeDr3w9KRq4PTA4ozDXa6Y+bDjLF47DT3PTkohY731WbieOnIGSN9zwY3TSneS8d AngThxzlvwGMQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FE0962931 for ; Sun, 21 Jan 2024 04:59:55 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="SKgxIdYP"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3F4521574; Sun, 21 Jan 2024 04:58:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809523; bh=KC/mNBwsTvo7jbNoWkM9MEDV6p6gdf3XmassPtSI1BY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SKgxIdYPo4SZDENA8F2UWmFt+5tji5rFzSQTSKx7oTcg3Kh1eAW1dltFZ+wpKmaij vfIxcZ16uyy5n4gbDjsOSmoRLTumj9d9MOpUU3iTnRGbX81Otqz+6ZE4o/qpXdjB5+ rOGp0Zr2VMxR4skQrZb82hZjoL35oIfc/t+crsK8= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:43 +0200 Message-ID: <20240121035948.4226-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 07/12] test: message: Destroy Object from correct thread context 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The MessageReceiver and RecursiveMessageReceiver used in the test are destroyed from the main thread, which is invalid for a thread-bound object bound to a different thread. Fix it by destroying them with deleteLater(). Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- test/message.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/test/message.cpp b/test/message.cpp index a34e0f0b5e10..2f9f281c5101 100644 --- a/test/message.cpp +++ b/test/message.cpp @@ -109,16 +109,19 @@ protected: return TestFail; } - MessageReceiver receiver; - receiver.moveToThread(&thread_); + MessageReceiver *receiver = new MessageReceiver(); + receiver->moveToThread(&thread_); thread_.start(); - receiver.postMessage(std::make_unique(Message::None)); + receiver->postMessage(std::make_unique(Message::None)); this_thread::sleep_for(chrono::milliseconds(100)); - switch (receiver.status()) { + MessageReceiver::Status status = receiver->status(); + receiver->deleteLater(); + + switch (status) { case MessageReceiver::NoMessage: cout << "No message received" << endl; return TestFail; @@ -135,8 +138,7 @@ protected: * leaks. Two messages need to be posted to ensure we don't only * test the simple case of a queue containing a single message. */ - std::unique_ptr recursiveReceiver = - std::make_unique(); + RecursiveMessageReceiver *recursiveReceiver = new RecursiveMessageReceiver(); recursiveReceiver->moveToThread(&thread_); recursiveReceiver->postMessage(std::make_unique(Message::None)); @@ -144,7 +146,10 @@ protected: this_thread::sleep_for(chrono::milliseconds(10)); - if (!recursiveReceiver->success()) { + bool success = recursiveReceiver->success(); + recursiveReceiver->deleteLater(); + + if (!success) { cout << "Recursive message delivery failed" << endl; return TestFail; } From patchwork Sun Jan 21 03:59:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19429 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 CE412C3243 for ; Sun, 21 Jan 2024 04:00:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9BE6A62916; Sun, 21 Jan 2024 05:00:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809604; bh=1Ry5mE8qE3p3JgOCxvID4DWvwVQFWwR6qvm0EdZYFoA=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=KOB4wTB9tPZb8tkir24GoNClJOja4IiZmi4Ag+NesfLeK/ZFZCWdQkeCq2pdwzeqo pVWfdzTGj7TqoFAdyqng5zd3vSoSyB7WIzuHkK6nxiteD4C/NSNfmRFrH/zNIUbuun N9Ip6DesSKh1ts6LksjXuqL0YHFvVZt5sIm64+avwxmSHVWXI6ffGN9XDFMU4t7xA9 SEDF/DG2E21TMoK0GkxtYKqZS6EuLmdzyDvWtK66guV166T/npM96dt6r/l4Dr3SgM Bq88sjMuCqhOuPECtwCyGR8o4HHBtK43cNGvgq7+7fr/yzqU85YB3CTqHOymlMyRDt sdje7vTXN8qCQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D36A6294C for ; Sun, 21 Jan 2024 04:59:56 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="fM6IyA2E"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7ED918D4; Sun, 21 Jan 2024 04:58:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809524; bh=1Ry5mE8qE3p3JgOCxvID4DWvwVQFWwR6qvm0EdZYFoA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fM6IyA2EhO+JmsY69T9HKYc4+zi7IDCUVLaPqUS8ai0yWvWW5iRxAMykVTxItTVZs Q63IaZ4/sK5dhTDMVnnQzkNfiGX13vUUri5g5qD+tvEZa2FcM+cI0p8b1RD5J3KKYr 49RSH5eFA0CZN6FuAMR+O981sCt+MVpdQRoXEJLI= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:44 +0200 Message-ID: <20240121035948.4226-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 08/12] test: signal-threads: Destroy Object from correct thread context 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The SignalReceiver used in the test is destroyed from the main thread, which is invalid for a thread-bound object bound to a different thread. Fix it by destroying it with deleteLater(). Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- test/signal-threads.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/test/signal-threads.cpp b/test/signal-threads.cpp index 8c550eb014d8..8c212b6f9ade 100644 --- a/test/signal-threads.cpp +++ b/test/signal-threads.cpp @@ -59,15 +59,20 @@ private: class SignalThreadsTest : public Test { protected: + int init() + { + receiver_ = new SignalReceiver(); + signal_.connect(receiver_, &SignalReceiver::slot); + + return TestPass; + } + int run() { - SignalReceiver receiver; - signal_.connect(&receiver, &SignalReceiver::slot); - /* Test that a signal is received in the main thread. */ signal_.emit(0); - switch (receiver.status()) { + switch (receiver_->status()) { case SignalReceiver::NoSignal: cout << "No signal received for direct connection" << endl; return TestFail; @@ -83,8 +88,8 @@ protected: * Move the object to a thread and verify that the signal is * correctly delivered, with the correct data. */ - receiver.reset(); - receiver.moveToThread(&thread_); + receiver_->reset(); + receiver_->moveToThread(&thread_); thread_.start(); @@ -92,7 +97,7 @@ protected: this_thread::sleep_for(chrono::milliseconds(100)); - switch (receiver.status()) { + switch (receiver_->status()) { case SignalReceiver::NoSignal: cout << "No signal received for message connection" << endl; return TestFail; @@ -104,7 +109,7 @@ protected: break; } - if (receiver.value() != 42) { + if (receiver_->value() != 42) { cout << "Signal received with incorrect value" << endl; return TestFail; } @@ -114,11 +119,13 @@ protected: void cleanup() { + receiver_->deleteLater(); thread_.exit(0); thread_.wait(); } private: + SignalReceiver *receiver_; Thread thread_; Signal signal_; From patchwork Sun Jan 21 03:59:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19430 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 73EAEC32BE for ; Sun, 21 Jan 2024 04:00:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2C63562945; Sun, 21 Jan 2024 05:00:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809605; bh=v9GuiS/h4iMeflUS4bTI8gyEEBPHzrxo/e9UcfRjy78=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=plR/8+egEhb/VIFPArgVM0YAib0/1LKTeFyuK5m3oHY/SPcCxLmtGPxjyOGdznB4J PxbZM27DvpOnpE58EFvZEk9lNo+nlSmwFf5lNbPbf3ouLTKce8GvX4hNySS0NQh+F5 KAwKRFGv2fM8m0dOpgWncBVNenE0p3cteK8+4UwAuI2mGDMGRWOhl4kHWqXG9H9xhx 3VcbUYWudTinfiDwBxvXRpqC5IXpjz1+bnLCFt6x0Vw+3Djdn8WGDA1SVIJhHlcGrk awOVfVJovHhSAVQ0FSFLfKXffrC3QXCQadG+yoWrTdGGyCTqOGiHAPloCJF/xIRVwD IC8HzTp86TdhQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D1CA66294C for ; Sun, 21 Jan 2024 04:59:57 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LWQHChuL"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CF4C513C5; Sun, 21 Jan 2024 04:58:45 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809526; bh=v9GuiS/h4iMeflUS4bTI8gyEEBPHzrxo/e9UcfRjy78=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LWQHChuLjZq1rsNpUBrO3E7V9qnx5XKDyEJhSrGGEwcw8zZDWkvyyQjY9qF1XKHAN ongqn6BUmo6s5/4+F5mXBhH7fAkDn+yWTS4jsg5qQremBCnMPfezz0itF6PnV2BCas FO39RYiJ7z8K6QUbUqJW4bsrmAF/frX6W7IKV0RA= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:45 +0200 Message-ID: <20240121035948.4226-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 09/12] test: timer-thread: Move timer start from wrong thread to separate test 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Starting a timer from the wrong thread is expected to fail, and we test this in the timer-thread unit test. This is however not something that a caller is allowed to do, and libcamera will get assertion failures to catch this invalid usage. The unit test will then fail. To prepare for this, split the unit test in two, with a test that is expected by meson to succeed, and one that is expected to fail. The assertion will then cause an expected failure, making the test suite succeed. Signed-off-by: Laurent Pinchart --- test/meson.build | 9 ++-- test/timer-fail.cpp | 101 ++++++++++++++++++++++++++++++++++++++++++ test/timer-thread.cpp | 22 --------- 3 files changed, 107 insertions(+), 25 deletions(-) create mode 100644 test/timer-fail.cpp diff --git a/test/meson.build b/test/meson.build index 189e1428485a..8b6057d4e800 100644 --- a/test/meson.build +++ b/test/meson.build @@ -69,6 +69,7 @@ internal_tests = [ {'name': 'signal-threads', 'sources': ['signal-threads.cpp']}, {'name': 'threads', 'sources': 'threads.cpp', 'dependencies': [libthreads]}, {'name': 'timer', 'sources': ['timer.cpp']}, + {'name': 'timer-fail', 'sources': ['timer-fail.cpp'], 'should_fail': true}, {'name': 'timer-thread', 'sources': ['timer-thread.cpp']}, {'name': 'unique-fd', 'sources': ['unique-fd.cpp']}, {'name': 'utils', 'sources': ['utils.cpp']}, @@ -91,7 +92,7 @@ foreach test : public_tests link_with : test_libraries, include_directories : test_includes_public) - test(test['name'], exe) + test(test['name'], exe, should_fail : test.get('should_fail', false)) endforeach foreach test : internal_tests @@ -105,7 +106,7 @@ foreach test : internal_tests link_with : test_libraries, include_directories : test_includes_internal) - test(test['name'], exe) + test(test['name'], exe, should_fail : test.get('should_fail', false)) endforeach foreach test : internal_non_parallel_tests @@ -119,5 +120,7 @@ foreach test : internal_non_parallel_tests link_with : test_libraries, include_directories : test_includes_internal) - test(test['name'], exe, is_parallel : false) + test(test['name'], exe, + is_parallel : false, + should_fail : test.get('should_fail', false)) endforeach diff --git a/test/timer-fail.cpp b/test/timer-fail.cpp new file mode 100644 index 000000000000..e9a3b1b61bcb --- /dev/null +++ b/test/timer-fail.cpp @@ -0,0 +1,101 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * timer-fail.cpp - Threaded timer failure test + */ + +#include +#include + +#include +#include +#include +#include + +#include "test.h" + +using namespace libcamera; +using namespace std; +using namespace std::chrono_literals; + +class TimeoutHandler : public Object +{ +public: + TimeoutHandler() + : timer_(this), timeout_(false) + { + timer_.timeout.connect(this, &TimeoutHandler::timeoutHandler); + } + + void start() + { + timer_.start(100ms); + } + + bool timeout() const + { + return timeout_; + } + +private: + void timeoutHandler() + { + timeout_ = true; + } + + Timer timer_; + bool timeout_; +}; + +class TimerFailTest : public Test +{ +protected: + int init() + { + thread_.start(); + timeout_.moveToThread(&thread_); + + return TestPass; + } + + int run() + { + /* + * Test that the forbidden operation of starting the timer from + * another thread results in a failure. We need to interrupt the + * event dispatcher to make sure we don't succeed simply because + * the event dispatcher hasn't noticed the timer restart. + */ + timeout_.start(); + thread_.eventDispatcher()->interrupt(); + + this_thread::sleep_for(chrono::milliseconds(200)); + + /* + * Meson expects this test to fail, so we need to return + * TestPass in the unexpected (usually known as "fail"), case, + * and TestFail otherwise. + */ + if (timeout_.timeout()) { + cout << "Timer start from wrong thread succeeded unexpectedly" + << endl; + return TestPass; + } + + return TestFail; + } + + void cleanup() + { + /* Must stop thread before destroying timeout. */ + thread_.exit(0); + thread_.wait(); + } + +private: + TimeoutHandler timeout_; + Thread thread_; +}; + +TEST_REGISTER(TimerFailTest) diff --git a/test/timer-thread.cpp b/test/timer-thread.cpp index 0bcd0d8ce194..4caf4e33b2ba 100644 --- a/test/timer-thread.cpp +++ b/test/timer-thread.cpp @@ -29,12 +29,6 @@ public: timer_.start(100ms); } - void restart() - { - timeout_ = false; - timer_.start(100ms); - } - bool timeout() const { return timeout_; @@ -74,22 +68,6 @@ protected: return TestFail; } - /* - * Test that starting the timer from another thread fails. We - * need to interrupt the event dispatcher to make sure we don't - * succeed simply because the event dispatcher hasn't noticed - * the timer restart. - */ - timeout_.restart(); - thread_.eventDispatcher()->interrupt(); - - this_thread::sleep_for(chrono::milliseconds(200)); - - if (timeout_.timeout()) { - cout << "Timer restart test failed" << endl; - return TestFail; - } - return TestPass; } From patchwork Sun Jan 21 03:59:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19431 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 EA8C2C32BF for ; Sun, 21 Jan 2024 04:00:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 19E3C6294B; Sun, 21 Jan 2024 05:00:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809606; bh=+e/hHPQCKT9nxgeE3fXUKLUmAWXegOJC3OWkWcvvDI0=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=JyNy39FTP6z8hcmw3HhbSKYbPD6oC6gEBnRMdW8Lxhh2vueZjFCszr+gjpbD+OiY0 oMqCP2Ym3xy0JAWUzBZW1YnHGE4e1K51z+JXubBnOGMcJ4psJPukshN8uvt+rg8VF9 k0Ykgdlzobxggb3W55p+d1qGKng7Dy8JNbeO67PYh/m7/PHU9kEuQ7oHx8CePTRPR4 c84HfVx8HdkdKIS1J9DPdhTrcVZSKh0vtrUmbWFDeacRyp6hki0/T9e1Su3DdWh1b7 +cfbwIJmZhMmUZ2ELZHyvgD52G1TqNhxz7+TswZGaKvxMmwmMsqvkD52E0X/Znhcgo 2Pchs2XBKyQsQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2AB9B62957 for ; Sun, 21 Jan 2024 04:59:59 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="MLma6vBW"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 24D938D4; Sun, 21 Jan 2024 04:58:47 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809527; bh=+e/hHPQCKT9nxgeE3fXUKLUmAWXegOJC3OWkWcvvDI0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MLma6vBWxs0qR9co/qZ2GgswuB8OMAfk9ZdD2gRDX6cCf6lW160FhY0Y3dRt+VCGd 0ndfMPzD0AuGLzaItTz0MxUrjl4YbmEjUqf7Cmse9clE5Snqm6px82ltTQVu74Xcb4 wojltPonRUG38ZXM0oFhrj3uUIbVNJfiHytZCdWs= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:46 +0200 Message-ID: <20240121035948.4226-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 10/12] test: timer-thread: Destroy Object from correct thread context 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The TimeoutHandler used in the test is destroyed from the main thread, which is invalid for a thread-bound object bound to a different thread. Fix it by destroying it with deleteLater(). Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- test/timer-fail.cpp | 16 +++++++++++----- test/timer-thread.cpp | 14 ++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/test/timer-fail.cpp b/test/timer-fail.cpp index e9a3b1b61bcb..b89c85f2ee72 100644 --- a/test/timer-fail.cpp +++ b/test/timer-fail.cpp @@ -54,7 +54,9 @@ protected: int init() { thread_.start(); - timeout_.moveToThread(&thread_); + + timeout_ = new TimeoutHandler(); + timeout_->moveToThread(&thread_); return TestPass; } @@ -67,7 +69,7 @@ protected: * event dispatcher to make sure we don't succeed simply because * the event dispatcher hasn't noticed the timer restart. */ - timeout_.start(); + timeout_->start(); thread_.eventDispatcher()->interrupt(); this_thread::sleep_for(chrono::milliseconds(200)); @@ -77,7 +79,7 @@ protected: * TestPass in the unexpected (usually known as "fail"), case, * and TestFail otherwise. */ - if (timeout_.timeout()) { + if (timeout_->timeout()) { cout << "Timer start from wrong thread succeeded unexpectedly" << endl; return TestPass; @@ -88,13 +90,17 @@ protected: void cleanup() { - /* Must stop thread before destroying timeout. */ + /* + * Object class instances must be destroyed from the thread + * they live in. + */ + timeout_->deleteLater(); thread_.exit(0); thread_.wait(); } private: - TimeoutHandler timeout_; + TimeoutHandler *timeout_; Thread thread_; }; diff --git a/test/timer-thread.cpp b/test/timer-thread.cpp index 4caf4e33b2ba..8675e2480aa9 100644 --- a/test/timer-thread.cpp +++ b/test/timer-thread.cpp @@ -50,7 +50,9 @@ protected: int init() { thread_.start(); - timeout_.moveToThread(&thread_); + + timeout_ = new TimeoutHandler(); + timeout_->moveToThread(&thread_); return TestPass; } @@ -63,7 +65,7 @@ protected: */ this_thread::sleep_for(chrono::milliseconds(200)); - if (!timeout_.timeout()) { + if (!timeout_->timeout()) { cout << "Timer expiration test failed" << endl; return TestFail; } @@ -73,13 +75,17 @@ protected: void cleanup() { - /* Must stop thread before destroying timeout. */ + /* + * Object class instances must be destroyed from the thread + * they live in. + */ + timeout_->deleteLater(); thread_.exit(0); thread_.wait(); } private: - TimeoutHandler timeout_; + TimeoutHandler *timeout_; Thread thread_; }; From patchwork Sun Jan 21 03:59:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19432 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 7F935C32C0 for ; Sun, 21 Jan 2024 04:00:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C74DF62950; Sun, 21 Jan 2024 05:00:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809606; bh=z0BmluV3+ys9PMdaTdconxWAgym5vmaEG5yZyDHy/JY=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=mUQshlTdBbOxXt9/Smd1UutBnz6RfatQ6z3PnnUdYJxQtSFlBrg1IUBpR+SYpjJel HKLtv2Co6IQ7YNk2Zr8YSpGJRKhJd81oNX754uTiIAV4mVn/zYMysRMBer9b3kxBMV U1iRBCkpjtCdHmabARKpWiXbk5+ahw0vDn8vTOLq6PKzHSQLna8ClLtwdh8izNNgcI IjPBiZX9r5vIgC51pzWf957mHMlWzkzpG6aJk7/Ev/+e1TBguPRftrFPRgx88jp2Pn gjUGoIFlTosIvmb0e1osr7Qc5U5I9EsC++ebefgyiRk4Ii80dDhNOLjICeiKLJWg8P k08wdZMfTZEsw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7FFAE62956 for ; Sun, 21 Jan 2024 05:00:00 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="JVivAXNB"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 64B931574; Sun, 21 Jan 2024 04:58:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809528; bh=z0BmluV3+ys9PMdaTdconxWAgym5vmaEG5yZyDHy/JY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JVivAXNBa2vCimQqNH+EM2ADe9pJdqZHPqYoYJa3WFzHhWmYcnxT1emcAecFm5AqR m2KVNydz3Tcp224eWOUdniNXamYqQasLNPvUSqCzQWr9FsfV10A4mIC5P24x8MTgKI vZH7p195MjkFFcWs1rpvhfhHfdz0VzMqVohA+HqU= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:47 +0200 Message-ID: <20240121035948.4226-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 11/12] libcamera: object: Document and ensure Object deletion constraints 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Object instances are meant to be deleted from the thread they are bound to, and this requirement is documented. There are however exceptions to the rule, as threads may be stopped and restarted, with objects bound to them not being deleted and recreated for every stop/restart cycle. Bound objects may therefore need to be deleted after the thread has stopped, making it impossible to use Object::deleteLater(). Document the lifetime requirements more precisely, and enforce them with an assertion. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- src/libcamera/base/object.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp index 1fce5a2af9af..8af0337f5448 100644 --- a/src/libcamera/base/object.cpp +++ b/src/libcamera/base/object.cpp @@ -40,8 +40,9 @@ LOG_DEFINE_CATEGORY(Object) * Object class. * * Deleting an object from a thread other than the one the object is bound to is - * unsafe, unless the caller ensures that the object isn't processing any - * message concurrently. + * unsafe, unless the caller ensures that the object's thread is stopped and no + * parent or child of the object gets deleted concurrently. See + * Object::~Object() for more information. * * Object slots connected to signals will also run in the context of the * object's thread, regardless of whether the signal is emitted in the same or @@ -84,9 +85,20 @@ Object::Object(Object *parent) * Object instances shall be destroyed from the thread they are bound to, * otherwise undefined behaviour may occur. If deletion of an Object needs to * be scheduled from a different thread, deleteLater() shall be used. + * + * As an exception to this rule, Object instances may be deleted from a + * different thread if the thread the instance is bound to is stopped through + * the whole duration of the object's destruction, *and* the parent and children + * of the object do not get deleted concurrently. The caller is responsible for + * fulfilling those requirements. + * + * In all cases Object instances shall be deleted before the Thread they are + * bound to. */ Object::~Object() { + ASSERT(Thread::current() == thread_ || !thread_->isRunning()); + /* * Move signals to a private list to avoid concurrent iteration and * deletion of items from Signal::disconnect(). From patchwork Sun Jan 21 03:59:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 19433 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 704CCC32C1 for ; Sun, 21 Jan 2024 04:00:08 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C253062945; Sun, 21 Jan 2024 05:00:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1705809607; bh=uUewvC+Z7f7DorNRtypkfDnV1u4i5LCMuN15JkZJ6Oc=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=2qSGZTCHPpMCGzgte66cengSSh2/Ent5+BBDo4jnvFbrBS/TDGfkxlnOUMkshbe0k rxicxRBhy8GLrvB0rdgyK6tNacCAMWN2nbbTn6c4nULAw2cj3OQ45I4XdutQ4YHxGc KUhL3EJI7o7ZH7sADvlAbk2plN1T+qzLuaOxNzq+PHAuu/QFNC8waUKltIRJDEjMKF 2yn3rqg7qNGEFRI2082BkesDWjpgjLjRFs1/PWyfvhHB7uhjrP39KBV4YRYbu5svVb ekGr4JmYtJjcDQtfFl2k/XMqVGoNBFIu0sNuRrdqWJR03LqMHZzEgwwZqali6qZTRa orpAkip2LlFZw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AC56B62951 for ; Sun, 21 Jan 2024 05:00:01 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="UoqwGTGD"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi [89.27.53.110]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A5A9C138B; Sun, 21 Jan 2024 04:58:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1705809529; bh=uUewvC+Z7f7DorNRtypkfDnV1u4i5LCMuN15JkZJ6Oc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UoqwGTGDtKMQVQFa6tl9WGP8A7Pm3cXulOQWFwMcTEfANeXMKQCHdX5ychd8N4Q0s 7PElyuj0p9rFCaFzIbeYN7Ih9h2n3ofnqyi1EmFk8isaDlRXvf0Eti0kpAjK3ogBTG 2NESBW1feYC3LvOHuH1isE82MZKlLd6aLMLg9wqU= To: libcamera-devel@lists.libcamera.org Date: Sun, 21 Jan 2024 05:59:48 +0200 Message-ID: <20240121035948.4226-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> References: <20240121035948.4226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 12/12] libcamera: object: Add and use thread-bound assertion 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Several functions in libcamera classes are marked as thread-bound, restricting the contexts in which those functions can be called. There is no infrastructure to enfore these restrictions, causing difficult to debug race conditions when they are not met by callers. As a first step to solve this, add an assertThreadBound() protected function to the Object class to test if the calling thread context is valid, and use it in member functions of Object subclasses marked as thread-bound. This replaces manual tests in a few locations. The thread-bound member functions of classes that do not inherit from Object are not checked, and neither are the functions of classes marked as thread-bound at the class level. These issue should be addressed in the future. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal --- include/libcamera/base/object.h | 2 ++ src/libcamera/base/event_notifier.cpp | 6 +++++ src/libcamera/base/object.cpp | 32 ++++++++++++++++++++++++++- src/libcamera/base/timer.cpp | 10 +++------ 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h index 933336361155..cb7e0a132be2 100644 --- a/include/libcamera/base/object.h +++ b/include/libcamera/base/object.h @@ -49,6 +49,8 @@ public: protected: virtual void message(Message *msg); + bool assertThreadBound(const char *message); + private: friend class SignalBase; friend class Thread; diff --git a/src/libcamera/base/event_notifier.cpp b/src/libcamera/base/event_notifier.cpp index fd93c0878c6f..a519aec38efb 100644 --- a/src/libcamera/base/event_notifier.cpp +++ b/src/libcamera/base/event_notifier.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -20,6 +21,8 @@ namespace libcamera { +LOG_DECLARE_CATEGORY(Event) + /** * \class EventNotifier * \brief Notify of activity on a file descriptor @@ -104,6 +107,9 @@ EventNotifier::~EventNotifier() */ void EventNotifier::setEnabled(bool enable) { + if (!assertThreadBound("EventNotifier can't be enabled from another thread")) + return; + if (enabled_ == enable) return; diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp index 8af0337f5448..d98a9157773e 100644 --- a/src/libcamera/base/object.cpp +++ b/src/libcamera/base/object.cpp @@ -224,6 +224,35 @@ void Object::message(Message *msg) } } +/** + * \fn Object::assertThreadBound() + * \brief Check if the caller complies with thread-bound constraints + * \param[in] message The message to be printed on error + * + * This function verifies the calling constraints required by the \threadbound + * definition. It shall be called at the beginning of member functions of an + * Object subclass that are explicitly marked as thread-bound in their + * documentation. + * + * If the thread-bound constraints are not met, the function prints \a message + * as an error message. For debug builds, it additionally causes an assertion + * error. + * + * \todo Verify the thread-bound requirements for functions marked as + * thread-bound at the class level. + * + * \return True if the call is thread-bound compliant, false otherwise + */ +bool Object::assertThreadBound(const char *message) +{ + if (Thread::current() == thread_) + return true; + + LOG(Object, Error) << message; + ASSERT(false); + return false; +} + /** * \fn R Object::invokeMethod() * \brief Invoke a method asynchronously on an Object instance @@ -275,7 +304,8 @@ void Object::message(Message *msg) */ void Object::moveToThread(Thread *thread) { - ASSERT(Thread::current() == thread_); + if (!assertThreadBound("Object can't be moved from another thread")) + return; if (thread_ == thread) return; diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp index 74b060af32ff..24dbf1e892c3 100644 --- a/src/libcamera/base/timer.cpp +++ b/src/libcamera/base/timer.cpp @@ -85,10 +85,8 @@ void Timer::start(std::chrono::milliseconds duration) */ void Timer::start(std::chrono::steady_clock::time_point deadline) { - if (Thread::current() != thread()) { - LOG(Timer, Error) << "Timer " << this << " << can't be started from another thread"; + if (!assertThreadBound("Timer can't be started from another thread")) return; - } deadline_ = deadline; @@ -114,13 +112,11 @@ void Timer::start(std::chrono::steady_clock::time_point deadline) */ void Timer::stop() { - if (!isRunning()) + if (!assertThreadBound("Timer can't be stopped from another thread")) return; - if (Thread::current() != thread()) { - LOG(Timer, Error) << "Timer " << this << " can't be stopped from another thread"; + if (!isRunning()) return; - } unregisterTimer(); }