Problem:
- Two test cases in test_buffer.cpp used structured bindings (auto [a, b])
- Structured bindings are a C++17 feature
- Project requires C++14 compatibility
Solution:
- Replace 'auto [ptr, size] = buf.to_tuple()' with C++14 compatible code
- Use std::get<N>() to extract tuple elements
- Modified tests: ToTupleNonConst, ToTupleConst
Changes:
- Line 239: Use std::get<0/1>(tuple) instead of structured binding
- Line 252: Use std::get<0/1>(tuple) instead of structured binding
- Add explanatory comments for clarity
This ensures the test suite compiles with C++14 standard.
- Remove useless 'ii->size_ = ii->size_;' statement at line 140
- The user-requested size is already set in acquire() function
- Simplify else branch to just a comment for clarity
- No functional change, just code cleanup
Problem:
- Reference counting tests fail on Windows (ReleaseMemory, ReferenceCount,
SubtractReference, HandleRef, HandleSubRef)
- get_ref() and sub_ref() were stub implementations returning 0/doing nothing
- CreateFileMapping HANDLE lacks built-in reference counting mechanism
Solution:
- Implement reference counting using std::atomic<std::int32_t> stored at
the end of shared memory (same strategy as POSIX version)
- Add calc_size() helper to allocate extra space for atomic counter
- Add acc_of() helper to access the atomic counter at the end of memory
- Modify acquire() to allocate calc_size(size) instead of size
- Modify get_mem() to initialize counter to 1 on first mapping
- Modify release() to decrement counter and return ref count before decrement
- Implement get_ref() to return current reference count
- Implement sub_ref() to atomically decrement reference count
- Convert file from Windows (CRLF) to Unix (LF) line endings for consistency
Key Implementation Details:
1. Reference counter stored at end of shared memory (aligned to info_t)
2. First get_mem() call: fetch_add(1) initializes counter to 1
3. release() returns ref count before decrement (for semantics compatibility)
4. Memory layout: [user data][padding][atomic<int32_t> counter]
5. Uses memory_order_acquire/release/acq_rel for proper synchronization
This makes Windows implementation match POSIX behavior and ensures all
reference counting tests pass on Windows platform.
Problem:
- Both linux/get_wait_time.h and posix/get_wait_time.h define inline functions
make_timespec() and calc_wait_time() in namespace ipc::detail
- On Linux, semaphore uses posix implementation, but may include both headers
- This causes ODR (One Definition Rule) violation - undefined behavior
- Different inline function definitions with same name violates C++ standard
- Manifested as test failures in SemaphoreTest::WaitTimeout
Solution:
- Add platform-specific namespace layer between ipc and detail:
- linux/get_wait_time.h: ipc::linux::detail::make_timespec()
- posix/get_wait_time.h: ipc::posix::detail::make_timespec()
- Update all call sites to use fully qualified names:
- linux/condition.h: linux::detail::make_timespec()
- linux/mutex.h: linux::detail::make_timespec() (2 places)
- posix/condition.h: posix::detail::make_timespec()
- posix/mutex.h: posix::detail::make_timespec() (2 places)
- posix/semaphore_impl.h: posix::detail::make_timespec()
This ensures each platform's implementation is uniquely named, preventing
ODR violations and ensuring correct function resolution at compile time.
Problem:
- Receivers were exiting after receiving only messages_per_sender (5) messages
- In broadcast mode, each message is sent to ALL receivers
- If sender1 completes quickly, all receivers get 5 messages and exit
- This causes sender2's messages to fail (no active receivers)
Solution:
- Each receiver should loop for num_senders * messages_per_sender (2 * 5 = 10) messages
- This ensures all receivers stay active until ALL senders complete
- Now receivers wait for: 2 senders × 5 messages each = 10 total messages
- Expected received_count: 2 senders × 5 messages × 2 receivers = 20 messages
This fix ensures all senders can successfully complete their sends before
any receiver exits.
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
- Problem: calling h2.release() followed by shm::remove(id) causes use-after-free
- h2.release() internally calls shm::release(id) which frees the id structure
- shm::remove(id) then accesses the freed id pointer -> crash
- Solution: detach the id from handle first, then call shm::remove(id)
- h2.detach() returns the id without releasing it
- shm::remove(id) can then safely clean up both memory and disk file
- This completes the fix for all ShmTest double-free issues
- Add comprehensive documentation for shm::release(id), shm::remove(id), and shm::remove(name)
- release(id): Decrements ref count, cleans up memory and disk file when count reaches zero
- remove(id): Calls release(id) internally, then forces disk file cleanup (WARNING: do not use after release)
- remove(name): Only removes disk file, safe to use anytime
- Fix critical double-free bug in ShmTest test cases
- Problem: calling release(id) followed by remove(id) causes use-after-free crash
because release() already frees the id structure
- Solution: replace 'release(id); remove(id);' pattern with just 'remove(id)'
- Fixed tests: AcquireCreate, AcquireCreateOrOpen, GetMemory, GetMemoryNoSize,
RemoveById, SubtractReference
- Kept 'release(id); remove(name);' pattern unchanged (safe usage)
- Add explanatory comments in test code to prevent future misuse
Header changes (include/libipc/buffer.h):
- Rename: additional → mem_to_free (better semantic name)
- Add documentation comments explaining the parameter's purpose
- Clarifies that mem_to_free is passed to destructor instead of p
- Use case: when data pointer is offset into a larger allocation
Implementation changes (src/libipc/buffer.cpp):
- Update parameter name in constructor implementation
- No logic changes, just naming improvement
Test changes (test/test_buffer.cpp):
- Fix TEST_F(BufferTest, ConstructorWithMemToFree)
- Previous test caused crash: passed stack variable address to destructor
- New test correctly demonstrates the parameter's purpose:
* Allocate 100-byte block
* Use offset portion (bytes 25-75) as data
* Destructor receives original block pointer for proper cleanup
- Prevents double-free and invalid free errors
Semantic explanation:
buffer(data_ptr, size, destructor, mem_to_free)
On destruction:
destructor(mem_to_free ? mem_to_free : data_ptr, size)
This allows:
char* block = new char[100];
char* data = block + 25;
buffer buf(data, 50, my_free, block); // Frees 'block', not 'data'
- Change: byte_t const (& data)[N] → byte_t (& data)[N]
- Allows non-const byte arrays to be accepted by the constructor
- Fixes defect discovered by TEST_F(BufferTest, ConstructorFromByteArray)
- The const qualifier on array elements was too restrictive
- Keep char const & c unchanged as it's correct for single char reference
- Fix handle class member functions: remove incorrect shm:: prefix
- h.acquire() not h.shm::acquire()
- h.release() not h.shm::release()
- h.sub_ref() not h.shm::sub_ref()
- Keep shm:: prefix for namespace-level global functions:
- shm::acquire(), shm::release(id), shm::get_mem()
- shm::remove(), shm::get_ref(id), shm::sub_ref(id)
- Fix comments to use correct terminology
- Resolves: 'shm::acquire is not a class member' compilation errors
- Remove 'using namespace ipc::shm;' to avoid id_t conflict with system id_t
- Add explicit shm:: namespace prefix to all shm types and functions
- Apply to: id_t, handle, acquire, get_mem, release, remove, get_ref, sub_ref
- Apply to: create and open constants
- Fix comments to avoid incorrect namespace references
- Resolves compilation error: 'reference to id_t is ambiguous'
