{"id":19430,"url":"https://patchwork.libcamera.org/api/patches/19430/?format=json","web_url":"https://patchwork.libcamera.org/patch/19430/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240121035948.4226-10-laurent.pinchart@ideasonboard.com>","date":"2024-01-21T03:59:45","name":"[libcamera-devel,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":"ad26a85a2a5691385393fd3cded2bacc088828a0","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19430/mbox/","series":[{"id":4149,"url":"https://patchwork.libcamera.org/api/series/4149/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4149","date":"2024-01-21T03:59:36","name":"libcamera: Hardening against thread race conditions","version":1,"mbox":"https://patchwork.libcamera.org/series/4149/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19430/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19430/checks/","tags":{},"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 73EAEC32BE\n\tfor ;\n\tSun, 21 Jan 2024 04:00:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C63562945;\n\tSun, 21 Jan 2024 05:00:05 +0100 (CET)","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 D1CA66294C\n\tfor ;\n\tSun, 21 Jan 2024 04:59:57 +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 CF4C513C5;\n\tSun, 21 Jan 2024 04:58:45 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705809605;\n\tbh=v9GuiS/h4iMeflUS4bTI8gyEEBPHzrxo/e9UcfRjy78=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=plR/8+egEhb/VIFPArgVM0YAib0/1LKTeFyuK5m3oHY/SPcCxLmtGPxjyOGdznB4J\n\tPxbZM27DvpOnpE58EFvZEk9lNo+nlSmwFf5lNbPbf3ouLTKce8GvX4hNySS0NQh+F5\n\tKAwKRFGv2fM8m0dOpgWncBVNenE0p3cteK8+4UwAuI2mGDMGRWOhl4kHWqXG9H9xhx\n\t3VcbUYWudTinfiDwBxvXRpqC5IXpjz1+bnLCFt6x0Vw+3Djdn8WGDA1SVIJhHlcGrk\n\tawOVfVJovHhSAVQ0FSFLfKXffrC3QXCQadG+yoWrTdGGyCTqOGiHAPloCJF/xIRVwD\n\tIC8HzTp86TdhQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705809526;\n\tbh=v9GuiS/h4iMeflUS4bTI8gyEEBPHzrxo/e9UcfRjy78=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=LWQHChuLjZq1rsNpUBrO3E7V9qnx5XKDyEJhSrGGEwcw8zZDWkvyyQjY9qF1XKHAN\n\tongqn6BUmo6s5/4+F5mXBhH7fAkDn+yWTS4jsg5qQremBCnMPfezz0itF6PnV2BCas\n\tFO39RYiJ7z8K6QUbUqJW4bsrmAF/frX6W7IKV0RA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LWQHChuL\"; dkim-atps=neutral","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","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 09/12] test: timer-thread: Move timer\n\tstart 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":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"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 \n---\n test/meson.build | 9 ++--\n test/timer-fail.cpp | 101 ++++++++++++++++++++++++++++++++++++++++++\n test/timer-thread.cpp | 22 ---------\n 3 files changed, 107 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..e9a3b1b61bcb\n--- /dev/null\n+++ b/test/timer-fail.cpp\n@@ -0,0 +1,101 @@\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 \n+#include \n+\n+#include \n+#include \n+#include \n+#include \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 * Meson expects this test to fail, so we need to return\n+\t\t * TestPass in the unexpected (usually known as \"fail\"), case,\n+\t\t * 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":["libcamera-devel","09/12"]}