Skip to content

Commit 78be284

Browse files
committed
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
1 parent d5f7875 commit 78be284

File tree

3 files changed

+89
-28
lines changed

3 files changed

+89
-28
lines changed

test/test_ipc_channel.cpp

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,38 @@
2222
#include <vector>
2323
#include <string>
2424
#include <cstring>
25+
#include <mutex>
26+
#include <condition_variable>
2527
#include "libipc/ipc.h"
2628
#include "libipc/buffer.h"
2729

2830
using namespace ipc;
2931

3032
namespace {
3133

34+
// Simple latch implementation for C++14 (similar to C++20 std::latch)
35+
class latch {
36+
public:
37+
explicit latch(std::ptrdiff_t count) : count_(count) {}
38+
39+
void count_down() {
40+
std::unique_lock<std::mutex> lock(mutex_);
41+
if (--count_ <= 0) {
42+
cv_.notify_all();
43+
}
44+
}
45+
46+
void wait() {
47+
std::unique_lock<std::mutex> lock(mutex_);
48+
cv_.wait(lock, [this] { return count_ <= 0; });
49+
}
50+
51+
private:
52+
std::ptrdiff_t count_;
53+
std::mutex mutex_;
54+
std::condition_variable cv_;
55+
};
56+
3257
std::string generate_unique_ipc_name(const char* prefix) {
3358
static int counter = 0;
3459
return std::string(prefix) + "_ipc_" + std::to_string(++counter);
@@ -516,24 +541,15 @@ TEST_F(ChannelTest, MultipleSendersReceivers) {
516541
std::atomic<int> sent_count{0};
517542
std::atomic<int> received_count{0};
518543

519-
std::vector<std::thread> senders;
520-
for (int i = 0; i < num_senders; ++i) {
521-
senders.emplace_back([&, i]() {
522-
channel ch(name.c_str(), sender);
523-
for (int j = 0; j < messages_per_sender; ++j) {
524-
std::string msg = "S" + std::to_string(i) + "M" + std::to_string(j);
525-
if (ch.send(msg, 1000)) {
526-
++sent_count;
527-
}
528-
std::this_thread::sleep_for(std::chrono::milliseconds(10));
529-
}
530-
});
531-
}
544+
// Use latch to ensure receivers are ready before senders start
545+
latch receivers_ready(num_receivers);
532546

533547
std::vector<std::thread> receivers;
534548
for (int i = 0; i < num_receivers; ++i) {
535549
receivers.emplace_back([&, i]() {
536550
channel ch(name.c_str(), receiver);
551+
receivers_ready.count_down(); // Signal this receiver is ready
552+
537553
for (int j = 0; j < messages_per_sender; ++j) {
538554
buffer buf = ch.recv(2000);
539555
if (!buf.empty()) {
@@ -543,6 +559,23 @@ TEST_F(ChannelTest, MultipleSendersReceivers) {
543559
});
544560
}
545561

562+
// Wait for all receivers to be ready
563+
receivers_ready.wait();
564+
565+
std::vector<std::thread> senders;
566+
for (int i = 0; i < num_senders; ++i) {
567+
senders.emplace_back([&, i]() {
568+
channel ch(name.c_str(), sender);
569+
for (int j = 0; j < messages_per_sender; ++j) {
570+
std::string msg = "S" + std::to_string(i) + "M" + std::to_string(j);
571+
if (ch.send(msg, 1000)) {
572+
++sent_count;
573+
}
574+
std::this_thread::sleep_for(std::chrono::milliseconds(10));
575+
}
576+
});
577+
}
578+
546579
for (auto& t : senders) {
547580
t.join();
548581
}

test/test_locks.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -383,20 +383,23 @@ TEST_F(RWLockTest, ReadersWritersNoOverlap) {
383383
TEST_F(RWLockTest, ReadWriteReadPattern) {
384384
rw_lock lock;
385385
int data = 0;
386+
std::atomic<int> iterations{0};
386387

387388
auto pattern_task = [&](int id) {
388389
for (int i = 0; i < 20; ++i) {
389-
// Read
390-
lock.lock_shared();
391-
int read_val = data;
392-
lock.unlock_shared();
390+
// Write: increment based on thread id
391+
lock.lock();
392+
data += id;
393+
lock.unlock();
393394

395+
iterations.fetch_add(1);
394396
std::this_thread::yield();
395397

396-
// Write
397-
lock.lock();
398-
data = read_val + 1;
399-
lock.unlock();
398+
// Read: verify data is consistent
399+
lock.lock_shared();
400+
int read_val = data;
401+
EXPECT_GE(read_val, 0); // Data should be non-negative
402+
lock.unlock_shared();
400403

401404
std::this_thread::yield();
402405
}
@@ -408,7 +411,10 @@ TEST_F(RWLockTest, ReadWriteReadPattern) {
408411
t1.join();
409412
t2.join();
410413

411-
EXPECT_EQ(data, 40);
414+
// Each thread increments by its id (1 or 2), 20 times each
415+
// Total = 1*20 + 2*20 = 20 + 40 = 60
416+
EXPECT_EQ(data, 60);
417+
EXPECT_EQ(iterations.load(), 40);
412418
}
413419

414420
// Test many readers, one writer

test/test_shm.cpp

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,13 @@ TEST_F(ShmTest, ReleaseMemory) {
126126
shm::id_t id = shm::acquire(name.c_str(), 128, shm::create);
127127
ASSERT_NE(id, nullptr);
128128

129+
// Must call get_mem to map memory and set reference count
130+
void* mem = shm::get_mem(id, nullptr);
131+
ASSERT_NE(mem, nullptr);
132+
133+
// release returns the reference count before decrement, or -1 on error
129134
std::int32_t ref_count = shm::release(id);
130-
EXPECT_GE(ref_count, 0);
135+
EXPECT_EQ(ref_count, 1); // Should be 1 (set by get_mem, before decrement)
131136

132137
shm::remove(name.c_str());
133138
}
@@ -161,14 +166,25 @@ TEST_F(ShmTest, ReferenceCount) {
161166
shm::id_t id1 = shm::acquire(name.c_str(), 512, shm::create);
162167
ASSERT_NE(id1, nullptr);
163168

169+
// Reference count is 0 after acquire (memory not mapped yet)
170+
std::int32_t ref_before_get_mem = shm::get_ref(id1);
171+
EXPECT_EQ(ref_before_get_mem, 0);
172+
173+
// get_mem maps memory and sets reference count to 1
174+
void* mem1 = shm::get_mem(id1, nullptr);
175+
ASSERT_NE(mem1, nullptr);
176+
164177
std::int32_t ref1 = shm::get_ref(id1);
165-
EXPECT_GT(ref1, 0);
178+
EXPECT_EQ(ref1, 1);
166179

167-
// Acquire again (should increase reference count)
180+
// Acquire again and get_mem (should increase reference count)
168181
shm::id_t id2 = shm::acquire(name.c_str(), 512, shm::open);
169182
if (id2 != nullptr) {
183+
void* mem2 = shm::get_mem(id2, nullptr);
184+
ASSERT_NE(mem2, nullptr);
185+
170186
std::int32_t ref2 = shm::get_ref(id2);
171-
EXPECT_GE(ref2, ref1);
187+
EXPECT_EQ(ref2, 2); // Should be 2 now
172188

173189
shm::release(id2);
174190
}
@@ -184,11 +200,17 @@ TEST_F(ShmTest, SubtractReference) {
184200
shm::id_t id = shm::acquire(name.c_str(), 256, shm::create);
185201
ASSERT_NE(id, nullptr);
186202

203+
// Must call get_mem first to map memory and initialize reference count
204+
void* mem = shm::get_mem(id, nullptr);
205+
ASSERT_NE(mem, nullptr);
206+
187207
std::int32_t ref_before = shm::get_ref(id);
208+
EXPECT_EQ(ref_before, 1); // Should be 1 after get_mem
209+
188210
shm::sub_ref(id);
189-
std::int32_t ref_after = shm::get_ref(id);
190211

191-
EXPECT_EQ(ref_after, ref_before - 1);
212+
std::int32_t ref_after = shm::get_ref(id);
213+
EXPECT_EQ(ref_after, 0); // Should be 0 after sub_ref
192214

193215
// Use remove(id) to clean up - it internally calls release()
194216
shm::remove(id);

0 commit comments

Comments
 (0)