From 78be284668d79553a36bce53fa2d078d205d5df1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=A8=E5=A4=B4=E4=BA=91?= Date: Sun, 30 Nov 2025 06:06:54 +0000 Subject: [PATCH] fix(test): correct test logic and semantics in multiple test cases 1. ChannelTest::MultipleSendersReceivers - Add C++14-compatible latch implementation (similar to C++20 std::latch) - Ensure receivers are ready before senders start sending messages - This prevents race condition where senders might send before receivers are listening 2. RWLockTest::ReadWriteReadPattern - Fix test logic: lock_shared allows multiple concurrent readers - Previous test had race condition where both threads could read same value - New test: each thread writes based on thread id (1 or 2), then reads - Expected result: 1*20 + 2*20 = 60 3. ShmTest::ReleaseMemory - Correct return value semantics: release() returns ref count before decrement, or -1 on error - Must call get_mem() to map memory and set ref count to 1 before release - Expected: release() returns 1 (ref count before decrement) 4. ShmTest::ReferenceCount - Correct semantics: ref count is 0 after acquire (memory not mapped) - get_mem() maps memory and sets ref count to 1 - Second acquire+get_mem increases ref count to 2 - Test now properly validates reference counting behavior 5. ShmTest::SubtractReference - sub_ref() only works after get_mem() has mapped the memory - Must call get_mem() first to initialize ref count to 1 - sub_ref() then decrements it to 0 - Test now follows correct API usage pattern --- test/test_ipc_channel.cpp | 59 ++++++++++++++++++++++++++++++--------- test/test_locks.cpp | 24 ++++++++++------ test/test_shm.cpp | 40 ++++++++++++++++++++------ 3 files changed, 92 insertions(+), 31 deletions(-) diff --git a/test/test_ipc_channel.cpp b/test/test_ipc_channel.cpp index c565b3e..11c98a2 100644 --- a/test/test_ipc_channel.cpp +++ b/test/test_ipc_channel.cpp @@ -22,6 +22,8 @@ #include #include #include +#include +#include #include "libipc/ipc.h" #include "libipc/buffer.h" @@ -29,6 +31,29 @@ using namespace ipc; namespace { +// Simple latch implementation for C++14 (similar to C++20 std::latch) +class latch { +public: + explicit latch(std::ptrdiff_t count) : count_(count) {} + + void count_down() { + std::unique_lock lock(mutex_); + if (--count_ <= 0) { + cv_.notify_all(); + } + } + + void wait() { + std::unique_lock lock(mutex_); + cv_.wait(lock, [this] { return count_ <= 0; }); + } + +private: + std::ptrdiff_t count_; + std::mutex mutex_; + std::condition_variable cv_; +}; + std::string generate_unique_ipc_name(const char* prefix) { static int counter = 0; return std::string(prefix) + "_ipc_" + std::to_string(++counter); @@ -516,6 +541,27 @@ TEST_F(ChannelTest, MultipleSendersReceivers) { std::atomic sent_count{0}; std::atomic received_count{0}; + // Use latch to ensure receivers are ready before senders start + latch receivers_ready(num_receivers); + + std::vector receivers; + for (int i = 0; i < num_receivers; ++i) { + receivers.emplace_back([&, i]() { + channel ch(name.c_str(), receiver); + receivers_ready.count_down(); // Signal this receiver is ready + + for (int j = 0; j < messages_per_sender; ++j) { + buffer buf = ch.recv(2000); + if (!buf.empty()) { + ++received_count; + } + } + }); + } + + // Wait for all receivers to be ready + receivers_ready.wait(); + std::vector senders; for (int i = 0; i < num_senders; ++i) { senders.emplace_back([&, i]() { @@ -530,19 +576,6 @@ TEST_F(ChannelTest, MultipleSendersReceivers) { }); } - std::vector receivers; - for (int i = 0; i < num_receivers; ++i) { - receivers.emplace_back([&, i]() { - channel ch(name.c_str(), receiver); - for (int j = 0; j < messages_per_sender; ++j) { - buffer buf = ch.recv(2000); - if (!buf.empty()) { - ++received_count; - } - } - }); - } - for (auto& t : senders) { t.join(); } diff --git a/test/test_locks.cpp b/test/test_locks.cpp index 3dc0c50..336b8c1 100644 --- a/test/test_locks.cpp +++ b/test/test_locks.cpp @@ -383,20 +383,23 @@ TEST_F(RWLockTest, ReadersWritersNoOverlap) { TEST_F(RWLockTest, ReadWriteReadPattern) { rw_lock lock; int data = 0; + std::atomic iterations{0}; auto pattern_task = [&](int id) { for (int i = 0; i < 20; ++i) { - // Read - lock.lock_shared(); - int read_val = data; - lock.unlock_shared(); + // Write: increment based on thread id + lock.lock(); + data += id; + lock.unlock(); + iterations.fetch_add(1); std::this_thread::yield(); - // Write - lock.lock(); - data = read_val + 1; - lock.unlock(); + // Read: verify data is consistent + lock.lock_shared(); + int read_val = data; + EXPECT_GE(read_val, 0); // Data should be non-negative + lock.unlock_shared(); std::this_thread::yield(); } @@ -408,7 +411,10 @@ TEST_F(RWLockTest, ReadWriteReadPattern) { t1.join(); t2.join(); - EXPECT_EQ(data, 40); + // Each thread increments by its id (1 or 2), 20 times each + // Total = 1*20 + 2*20 = 20 + 40 = 60 + EXPECT_EQ(data, 60); + EXPECT_EQ(iterations.load(), 40); } // Test many readers, one writer diff --git a/test/test_shm.cpp b/test/test_shm.cpp index e284dd6..98b3531 100644 --- a/test/test_shm.cpp +++ b/test/test_shm.cpp @@ -126,8 +126,13 @@ TEST_F(ShmTest, ReleaseMemory) { shm::id_t id = shm::acquire(name.c_str(), 128, shm::create); ASSERT_NE(id, nullptr); + // Must call get_mem to map memory and set reference count + void* mem = shm::get_mem(id, nullptr); + ASSERT_NE(mem, nullptr); + + // release returns the reference count before decrement, or -1 on error std::int32_t ref_count = shm::release(id); - EXPECT_GE(ref_count, 0); + EXPECT_EQ(ref_count, 1); // Should be 1 (set by get_mem, before decrement) shm::remove(name.c_str()); } @@ -161,14 +166,25 @@ TEST_F(ShmTest, ReferenceCount) { shm::id_t id1 = shm::acquire(name.c_str(), 512, shm::create); ASSERT_NE(id1, nullptr); - std::int32_t ref1 = shm::get_ref(id1); - EXPECT_GT(ref1, 0); + // Reference count is 0 after acquire (memory not mapped yet) + std::int32_t ref_before_get_mem = shm::get_ref(id1); + EXPECT_EQ(ref_before_get_mem, 0); - // Acquire again (should increase reference count) + // get_mem maps memory and sets reference count to 1 + void* mem1 = shm::get_mem(id1, nullptr); + ASSERT_NE(mem1, nullptr); + + std::int32_t ref1 = shm::get_ref(id1); + EXPECT_EQ(ref1, 1); + + // Acquire again and get_mem (should increase reference count) shm::id_t id2 = shm::acquire(name.c_str(), 512, shm::open); if (id2 != nullptr) { + void* mem2 = shm::get_mem(id2, nullptr); + ASSERT_NE(mem2, nullptr); + std::int32_t ref2 = shm::get_ref(id2); - EXPECT_GE(ref2, ref1); + EXPECT_EQ(ref2, 2); // Should be 2 now shm::release(id2); } @@ -184,11 +200,17 @@ TEST_F(ShmTest, SubtractReference) { shm::id_t id = shm::acquire(name.c_str(), 256, shm::create); ASSERT_NE(id, nullptr); - std::int32_t ref_before = shm::get_ref(id); - shm::sub_ref(id); - std::int32_t ref_after = shm::get_ref(id); + // Must call get_mem first to map memory and initialize reference count + void* mem = shm::get_mem(id, nullptr); + ASSERT_NE(mem, nullptr); - EXPECT_EQ(ref_after, ref_before - 1); + std::int32_t ref_before = shm::get_ref(id); + EXPECT_EQ(ref_before, 1); // Should be 1 after get_mem + + shm::sub_ref(id); + + std::int32_t ref_after = shm::get_ref(id); + EXPECT_EQ(ref_after, 0); // Should be 0 after sub_ref // Use remove(id) to clean up - it internally calls release() shm::remove(id);