From 2fe983d2886290dcc7ee64acc821053a41eb76b0 Mon Sep 17 00:00:00 2001 From: unai_71 Date: Tue, 10 Mar 2026 19:55:47 +0000 Subject: [PATCH 1/6] feat: added tests to check consumer resilience against corrupted data. All tests pased --- tests/test_consumer.cxx | 95 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tests/test_consumer.cxx b/tests/test_consumer.cxx index d330570..610e6c0 100644 --- a/tests/test_consumer.cxx +++ b/tests/test_consumer.cxx @@ -1,7 +1,11 @@ #include +#include +#include +#include #include #include +#include #include "Consumer.hpp" #include "UnixIpcBridge.hpp" @@ -118,3 +122,94 @@ TEST(ConsumerThreadTest, StopsCleanlyWhenNeverStarted) // stop() on a consumer that was never started must not crash consumer.stop(); } + +// --------------------------------------------------------------------------- +// Requirement 2: Consumer receiving corrupted data (non-numeric) +// --------------------------------------------------------------------------- + +/// Helper: raw-connect to a UNIX socket and send arbitrary bytes. +static void send_raw_bytes(const std::string& path, const void* data, + size_t len) +{ + int fd = socket(AF_UNIX, SOCK_STREAM, 0); + ASSERT_GE(fd, 0); + + struct sockaddr_un addr = {}; + addr.sun_family = AF_UNIX; + std::strncpy(addr.sun_path, path.c_str(), sizeof(addr.sun_path) - 1); + + ASSERT_EQ( + connect(fd, reinterpret_cast(&addr), sizeof(addr)), 0); + + if (len > 0) + { + ::send(fd, data, len, 0); + } + close(fd); +} + +TEST(ConsumerThreadTest, DropsCorruptedShortMessage) +{ + const std::string sock = "/tmp/test_ct_corrupt_short.sock"; + + ConsumerThread consumer(sock); + QSignalSpy spy(&consumer, &ConsumerThread::valueReceived); + consumer.start(); + + // Send only 2 bytes instead of sizeof(int)==4 — corrupted / partial message + uint16_t garbage = 0xBEEF; + send_raw_bytes(sock, &garbage, sizeof(garbage)); + + // Give the consumer time to process (or not) + spy.wait(500); + consumer.stop(); + + // No signal should have been emitted + EXPECT_EQ(spy.count(), 0); +} + +TEST(ConsumerThreadTest, DropsEmptyConnection) +{ + const std::string sock = "/tmp/test_ct_corrupt_empty.sock"; + + ConsumerThread consumer(sock); + QSignalSpy spy(&consumer, &ConsumerThread::valueReceived); + consumer.start(); + + // Connect and immediately close — zero bytes sent + send_raw_bytes(sock, nullptr, 0); + + spy.wait(500); + consumer.stop(); + + EXPECT_EQ(spy.count(), 0); +} + +TEST(ConsumerThreadTest, SurvivesCorruptedThenReceivesValid) +{ + const std::string sock = "/tmp/test_ct_corrupt_then_valid.sock"; + + ConsumerThread consumer(sock); + QSignalSpy spy(&consumer, &ConsumerThread::valueReceived); + consumer.start(); + + // First: send corrupted (1 byte) + uint8_t one_byte = 0xFF; + send_raw_bytes(sock, &one_byte, sizeof(one_byte)); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + // Then: send a valid int via the normal bridge + UnixIpcBridge bridge(sock); + bridge.send(777); + + // Wait for the valid signal + for (int attempt = 0; spy.count() < 1 && attempt < 20; ++attempt) + { + spy.wait(100); + } + consumer.stop(); + + // The corrupted message must have been dropped, valid one received + ASSERT_EQ(spy.count(), 1); + EXPECT_EQ(spy.at(0).at(0).toInt(), 777); +} -- 2.49.1 From 4a743f26a4b59d0df41ab3816337e7c18205fbe6 Mon Sep 17 00:00:00 2001 From: unai_71 Date: Tue, 10 Mar 2026 20:05:19 +0000 Subject: [PATCH 2/6] feat: add race condition tests for consumer and producer interactions --- tests/CMakeLists.txt | 15 ++++++ tests/test_race_conditions.cxx | 83 ++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 tests/test_race_conditions.cxx diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 30538ed..aa8f19e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -73,3 +73,18 @@ target_link_libraries(test_main_window ) add_test(NAME test_main_window COMMAND test_main_window) + +add_executable(test_race_conditions + test_race_conditions.cxx +) + +target_link_libraries(test_race_conditions + PRIVATE + core + gtest + gtest_main + Qt5::Core + Qt5::Test +) + +add_test(NAME test_race_conditions COMMAND test_race_conditions) diff --git a/tests/test_race_conditions.cxx b/tests/test_race_conditions.cxx new file mode 100644 index 0000000..b367719 --- /dev/null +++ b/tests/test_race_conditions.cxx @@ -0,0 +1,83 @@ +// test_race_conditions.cxx +// SPDX-License-Identifier: GPL-3.0-or-later +// Author: Unai Blazquez + +#include + +#include +#include +#include +#include +#include +#include + +#include "Consumer.hpp" +#include "UnixIpcBridge.hpp" + +static int argc_ = 0; +static QCoreApplication app_(argc_, nullptr); + + +TEST(RaceConditionTest, RepeatedStartStopWhileProducerSends) +{ + const std::string sock = "/tmp/test_race.sock"; + constexpr int kCycles = 20; + + // Watchdog: if the test takes longer than 15s, declare deadlock. + std::atomic test_done{false}; + std::thread watchdog([&test_done]() { + for (int i = 0; i < 150 && !test_done.load(); ++i) + { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + if (!test_done.load()) + { + std::cerr + << "DEADLOCK DETECTED: RepeatedStartStopWhileProducerSends timed out" + << std::endl; + std::abort(); + } + }); + + // Producer thread: keeps trying to send values. connect() failures + // (consumer mid-restart) are expected and silently ignored. + + std::atomic producer_running{true}; + std::thread producer([&]() { + while (producer_running.load()) + { + try + { + UnixIpcBridge bridge(sock); + bridge.send(42); + } + catch (const std::runtime_error&) + { + // Expected: consumer socket not ready or just torn down. + } + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + }); + + // Main thread: repeatedly start/stop the consumer. + for (int i = 0; i < kCycles; ++i) + { + ConsumerThread consumer(sock); + consumer.start(); + + // Let it run briefly so the producer can connect during some cycles. + std::this_thread::sleep_for(std::chrono::milliseconds(10 + (i % 5) * 5)); + + // stop() must return without deadlock every single time. + consumer.stop(); + } + + producer_running.store(false); + producer.join(); + + test_done.store(true); + watchdog.join(); + + // If we reach here, no deadlock across kCycles start/stop cycles. + SUCCEED(); +} -- 2.49.1 From e8da96391918e7ea79b975708f5d646030d489ec Mon Sep 17 00:00:00 2001 From: unai_71 Date: Tue, 10 Mar 2026 20:12:34 +0000 Subject: [PATCH 3/6] fix: correct comment formatting in main.cxx --- src/app/main.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/main.cxx b/src/app/main.cxx index 04943fe..b533a86 100644 --- a/src/app/main.cxx +++ b/src/app/main.cxx @@ -1,4 +1,4 @@ -^// main.cxx +// main.cxx // SPDX-License-Identifier: GPL-3.0-or-later // Author: Unai Blazquez -- 2.49.1 From 3d2c193db9678f923d59175a12beecb6e55d3bf9 Mon Sep 17 00:00:00 2001 From: unai_71 Date: Tue, 10 Mar 2026 20:13:48 +0000 Subject: [PATCH 4/6] feat: enhance race condition tests to verify producer resilience after consumer crash --- tests/test_race_conditions.cxx | 102 +++++++++++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 4 deletions(-) diff --git a/tests/test_race_conditions.cxx b/tests/test_race_conditions.cxx index b367719..7b06a30 100644 --- a/tests/test_race_conditions.cxx +++ b/tests/test_race_conditions.cxx @@ -5,13 +5,17 @@ #include #include +#include #include #include +#include #include #include #include +#include #include "Consumer.hpp" +#include "Producer.hpp" #include "UnixIpcBridge.hpp" static int argc_ = 0; @@ -39,9 +43,6 @@ TEST(RaceConditionTest, RepeatedStartStopWhileProducerSends) } }); - // Producer thread: keeps trying to send values. connect() failures - // (consumer mid-restart) are expected and silently ignored. - std::atomic producer_running{true}; std::thread producer([&]() { while (producer_running.load()) @@ -59,7 +60,6 @@ TEST(RaceConditionTest, RepeatedStartStopWhileProducerSends) } }); - // Main thread: repeatedly start/stop the consumer. for (int i = 0; i < kCycles; ++i) { ConsumerThread consumer(sock); @@ -81,3 +81,97 @@ TEST(RaceConditionTest, RepeatedStartStopWhileProducerSends) // If we reach here, no deadlock across kCycles start/stop cycles. SUCCEED(); } + +TEST(RaceConditionTest, ProducerSurvivesConsumerCrash) +{ + const std::string sock = "/tmp/test_crash.sock"; + const std::string sysfs = "./fake_sysfs_race"; + + // Prepare sysfs file so the producer is in Enabled state. + { std::ofstream(sysfs) << "1\n"; } + + // Track what the producer sends. + std::vector sent_values; + std::mutex sent_mutex; + std::vector logs; + std::mutex log_mutex; + + auto make_safe_send = [&](const std::string& path) { + return [&, path](int value) { + try + { + UnixIpcBridge bridge(path); + bridge.send(value); + std::lock_guard lk(sent_mutex); + sent_values.push_back(value); + } + catch (const std::runtime_error&) + { + // Consumer is down — expected during the "crash" window. + } + }; + }; + + Producer producer( + sysfs, make_safe_send(sock), []() { return 123; }, + [&](const std::string& msg) { + std::lock_guard lk(log_mutex); + logs.push_back(msg); + }, + [](std::chrono::milliseconds) { + // Use a short sleep so the test runs fast. + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + }); + + // Phase 1: start consumer, start producer, let a few values flow. + { + ConsumerThread consumer(sock); + QSignalSpy spy(&consumer, &ConsumerThread::valueReceived); + consumer.start(); + producer.start(); + + // Wait for at least 2 values to arrive. + for (int attempt = 0; spy.count() < 2 && attempt < 50; ++attempt) + { + spy.wait(100); + } + ASSERT_GE(spy.count(), 2) << "Phase 1: producer should have delivered values"; + + // "Crash" the consumer: stop + destroy. + consumer.stop(); + } + + // Phase 2: producer is still running with no consumer (sends will fail). + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + // Phase 3: bring up a fresh consumer. Producer should resume delivering. + { + ConsumerThread consumer2(sock); + QSignalSpy spy2(&consumer2, &ConsumerThread::valueReceived); + consumer2.start(); + + for (int attempt = 0; spy2.count() < 2 && attempt < 50; ++attempt) + { + spy2.wait(100); + } + + consumer2.stop(); + + ASSERT_GE(spy2.count(), 2) + << "Phase 3: producer must deliver to a new consumer after crash"; + + // Values received by the second consumer should all be 123. + for (int i = 0; i < spy2.count(); ++i) + { + EXPECT_EQ(spy2.at(i).at(0).toInt(), 123); + } + } + + producer.stop(); + + // Producer logged throughout all three phases. + { + std::lock_guard lk(log_mutex); + EXPECT_GE(logs.size(), 3u) << "Producer should have kept logging"; + } +} -- 2.49.1 From 79f5ad10b6f33584b180e421fa33e9de25d92743 Mon Sep 17 00:00:00 2001 From: unai_71 Date: Tue, 10 Mar 2026 20:19:09 +0000 Subject: [PATCH 5/6] feat: simulate consumer crash in race condition tests to verify producer behavior --- tests/test_race_conditions.cxx | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/tests/test_race_conditions.cxx b/tests/test_race_conditions.cxx index 7b06a30..1791152 100644 --- a/tests/test_race_conditions.cxx +++ b/tests/test_race_conditions.cxx @@ -3,6 +3,9 @@ // Author: Unai Blazquez #include +#include +#include +#include #include #include @@ -137,8 +140,28 @@ TEST(RaceConditionTest, ProducerSurvivesConsumerCrash) } ASSERT_GE(spy.count(), 2) << "Phase 1: producer should have delivered values"; - // "Crash" the consumer: stop + destroy. - consumer.stop(); + // Simulate a hard crash: force-close the consumer's server fd from + // outside its thread, causing accept() to fail with EBADF. This is + // what happens when the kernel reclaims fds on SIGKILL / abort(). + // + // We find the server fd by calling getsockname() on open fds and + // matching against our socket path. + for (int fd = 3; fd < 1024; ++fd) + { + struct sockaddr_un addr = {}; + socklen_t len = sizeof(addr); + if (getsockname(fd, reinterpret_cast(&addr), &len) == 0 && + addr.sun_family == AF_UNIX && + std::string(addr.sun_path) == sock) + { + ::close(fd); // Yank the fd — consumer thread crashes out of accept() + break; + } + } + + // Destructor calls stop(), which joins the (now-exited) thread and + // cleans up. In a real crash no cleanup runs, but we can't leak + // threads in a test process. } // Phase 2: producer is still running with no consumer (sends will fail). -- 2.49.1 From f51e65444ff8a501a61216666edee846b0737906 Mon Sep 17 00:00:00 2001 From: unai_71 Date: Tue, 10 Mar 2026 20:23:07 +0000 Subject: [PATCH 6/6] feat: add quality description to document TDD benefits for race condition tests --- docs/quality_description.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/quality_description.md diff --git a/docs/quality_description.md b/docs/quality_description.md new file mode 100644 index 0000000..dff4f53 --- /dev/null +++ b/docs/quality_description.md @@ -0,0 +1,5 @@ +# Quality Description + +Writing tests first forced every component to be injectable and independently exercisable before any integration happened. That constraint turned out to matter more than expected when the race-condition tests were added at the end: because the producer, sysfs reader, and IPC bridge had already been broken into units with explicit interfaces (`std::function` callbacks, injected sleep, injected logger), the stress tests could be wired up without touching any production code. Nothing needed to be refactored to be testable — it already was. That is the practical benefit of TDD for concurrent embedded software: the discipline of writing the test first tends to eliminate shared mutable state and deep call chains by making them painful to test, which in turn reduces cyclomatic complexity almost as a side effect. + + -- 2.49.1