Patch Detail
Show a patch.
GET /api/patches/19457/?format=api
{ "id": 19457, "url": "https://patchwork.libcamera.org/api/patches/19457/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19457/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20240123011249.22716-10-laurent.pinchart@ideasonboard.com>", "date": "2024-01-23T01:12:46", "name": "[v2,09/12] test: timer-thread: Move timer start from wrong thread to separate test", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "bc1a13297442450d499e4ca36df49a0e8df96726", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/19457/mbox/", "series": [ { "id": 4157, "url": "https://patchwork.libcamera.org/api/series/4157/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4157", "date": "2024-01-23T01:12:37", "name": "libcamera: Hardening against thread race conditions", "version": 2, "mbox": "https://patchwork.libcamera.org/series/4157/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/19457/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/19457/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 B7802C32C2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Jan 2024 01:13:01 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5AAA962916;\n\tTue, 23 Jan 2024 02:13:01 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FBE662956\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jan 2024 02:12:58 +0100 (CET)", "from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C96DE1593;\n\tTue, 23 Jan 2024 02:11:44 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WC7T0zma\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705972305;\n\tbh=V7i+CEzlTUd2+3RHPPykckvdjZ2Kf7qJ5QFDTsyn/lI=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=WC7T0zmazMaLGMX0Jq04hi0REZhmdxn2wNWU14euer17IFrlHOrfe4sGW2hGCEUS5\n\taeFvNo6Y7dka3uo7bptxg9joRHlyUucZs4BOzUTMoa/dUjpjrUaY1g7oIoZ54zNAM+\n\taTyQLGBZWOA4q0vaHkr8EIWwePSBPFMdv693hkgw=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[PATCH v2 09/12] test: timer-thread: Move timer start from wrong\n\tthread to separate test", "Date": "Tue, 23 Jan 2024 03:12:46 +0200", "Message-ID": "<20240123011249.22716-10-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20240123011249.22716-1-laurent.pinchart@ideasonboard.com>", "References": "<20240123011249.22716-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Starting a timer from the wrong thread is expected to fail, and we test\nthis in the timer-thread unit test. This is however not something that a\ncaller is allowed to do, and libcamera will get assertion failures to\ncatch this invalid usage. The unit test will then fail.\n\nTo prepare for this, split the unit test in two, with a test that is\nexpected by meson to succeed, and one that is expected to fail. The\nassertion will then cause an expected failure, making the test suite\nsucceed.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nChanges since v1:\n\n- Improve comment explaining why the test is expected to fail\n---\n test/meson.build | 9 ++--\n test/timer-fail.cpp | 103 ++++++++++++++++++++++++++++++++++++++++++\n test/timer-thread.cpp | 22 ---------\n 3 files changed, 109 insertions(+), 25 deletions(-)\n create mode 100644 test/timer-fail.cpp", "diff": "diff --git a/test/meson.build b/test/meson.build\nindex 189e1428485a..8b6057d4e800 100644\n--- a/test/meson.build\n+++ b/test/meson.build\n@@ -69,6 +69,7 @@ internal_tests = [\n {'name': 'signal-threads', 'sources': ['signal-threads.cpp']},\n {'name': 'threads', 'sources': 'threads.cpp', 'dependencies': [libthreads]},\n {'name': 'timer', 'sources': ['timer.cpp']},\n+ {'name': 'timer-fail', 'sources': ['timer-fail.cpp'], 'should_fail': true},\n {'name': 'timer-thread', 'sources': ['timer-thread.cpp']},\n {'name': 'unique-fd', 'sources': ['unique-fd.cpp']},\n {'name': 'utils', 'sources': ['utils.cpp']},\n@@ -91,7 +92,7 @@ foreach test : public_tests\n link_with : test_libraries,\n include_directories : test_includes_public)\n \n- test(test['name'], exe)\n+ test(test['name'], exe, should_fail : test.get('should_fail', false))\n endforeach\n \n foreach test : internal_tests\n@@ -105,7 +106,7 @@ foreach test : internal_tests\n link_with : test_libraries,\n include_directories : test_includes_internal)\n \n- test(test['name'], exe)\n+ test(test['name'], exe, should_fail : test.get('should_fail', false))\n endforeach\n \n foreach test : internal_non_parallel_tests\n@@ -119,5 +120,7 @@ foreach test : internal_non_parallel_tests\n link_with : test_libraries,\n include_directories : test_includes_internal)\n \n- test(test['name'], exe, is_parallel : false)\n+ test(test['name'], exe,\n+ is_parallel : false,\n+ should_fail : test.get('should_fail', false))\n endforeach\ndiff --git a/test/timer-fail.cpp b/test/timer-fail.cpp\nnew file mode 100644\nindex 000000000000..2c622ca3410d\n--- /dev/null\n+++ b/test/timer-fail.cpp\n@@ -0,0 +1,103 @@\n+/* SPDX-License-Identifier: GPL-2.0-or-later */\n+/*\n+ * Copyright (C) 2024, Ideas on Board Oy\n+ *\n+ * timer-fail.cpp - Threaded timer failure test\n+ */\n+\n+#include <chrono>\n+#include <iostream>\n+\n+#include <libcamera/base/event_dispatcher.h>\n+#include <libcamera/base/object.h>\n+#include <libcamera/base/thread.h>\n+#include <libcamera/base/timer.h>\n+\n+#include \"test.h\"\n+\n+using namespace libcamera;\n+using namespace std;\n+using namespace std::chrono_literals;\n+\n+class TimeoutHandler : public Object\n+{\n+public:\n+\tTimeoutHandler()\n+\t\t: timer_(this), timeout_(false)\n+\t{\n+\t\ttimer_.timeout.connect(this, &TimeoutHandler::timeoutHandler);\n+\t}\n+\n+\tvoid start()\n+\t{\n+\t\ttimer_.start(100ms);\n+\t}\n+\n+\tbool timeout() const\n+\t{\n+\t\treturn timeout_;\n+\t}\n+\n+private:\n+\tvoid timeoutHandler()\n+\t{\n+\t\ttimeout_ = true;\n+\t}\n+\n+\tTimer timer_;\n+\tbool timeout_;\n+};\n+\n+class TimerFailTest : public Test\n+{\n+protected:\n+\tint init()\n+\t{\n+\t\tthread_.start();\n+\t\ttimeout_.moveToThread(&thread_);\n+\n+\t\treturn TestPass;\n+\t}\n+\n+\tint run()\n+\t{\n+\t\t/*\n+\t\t * Test that the forbidden operation of starting the timer from\n+\t\t * another thread results in a failure. We need to interrupt the\n+\t\t * event dispatcher to make sure we don't succeed simply because\n+\t\t * the event dispatcher hasn't noticed the timer restart.\n+\t\t */\n+\t\ttimeout_.start();\n+\t\tthread_.eventDispatcher()->interrupt();\n+\n+\t\tthis_thread::sleep_for(chrono::milliseconds(200));\n+\n+\t\t/*\n+\t\t * The wrong start() call should result in an assertion in debug\n+\t\t * builds, and a timeout in release builds. The test is\n+\t\t * therefore marked in meson.build as expected to fail. We need\n+\t\t * to return TestPass in the unexpected (usually known as\n+\t\t * \"fail\") case, and TestFail otherwise.\n+\t\t */\n+\t\tif (timeout_.timeout()) {\n+\t\t\tcout << \"Timer start from wrong thread succeeded unexpectedly\"\n+\t\t\t << endl;\n+\t\t\treturn TestPass;\n+\t\t}\n+\n+\t\treturn TestFail;\n+\t}\n+\n+\tvoid cleanup()\n+\t{\n+\t\t/* Must stop thread before destroying timeout. */\n+\t\tthread_.exit(0);\n+\t\tthread_.wait();\n+\t}\n+\n+private:\n+\tTimeoutHandler timeout_;\n+\tThread thread_;\n+};\n+\n+TEST_REGISTER(TimerFailTest)\ndiff --git a/test/timer-thread.cpp b/test/timer-thread.cpp\nindex 0bcd0d8ce194..4caf4e33b2ba 100644\n--- a/test/timer-thread.cpp\n+++ b/test/timer-thread.cpp\n@@ -29,12 +29,6 @@ public:\n \t\ttimer_.start(100ms);\n \t}\n \n-\tvoid restart()\n-\t{\n-\t\ttimeout_ = false;\n-\t\ttimer_.start(100ms);\n-\t}\n-\n \tbool timeout() const\n \t{\n \t\treturn timeout_;\n@@ -74,22 +68,6 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\t/*\n-\t\t * Test that starting the timer from another thread fails. We\n-\t\t * need to interrupt the event dispatcher to make sure we don't\n-\t\t * succeed simply because the event dispatcher hasn't noticed\n-\t\t * the timer restart.\n-\t\t */\n-\t\ttimeout_.restart();\n-\t\tthread_.eventDispatcher()->interrupt();\n-\n-\t\tthis_thread::sleep_for(chrono::milliseconds(200));\n-\n-\t\tif (timeout_.timeout()) {\n-\t\t\tcout << \"Timer restart test failed\" << endl;\n-\t\t\treturn TestFail;\n-\t\t}\n-\n \t\treturn TestPass;\n \t}\n \n", "prefixes": [ "v2", "09/12" ] }