Skip to content

Commit 0ecf1a4

Browse files
committed
docs(shm): add semantic comments for release/remove and fix double-free in tests
- 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
1 parent 91e4489 commit 0ecf1a4

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

include/libipc/shm.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,30 @@ enum : unsigned {
1717

1818
IPC_EXPORT id_t acquire(char const * name, std::size_t size, unsigned mode = create | open);
1919
IPC_EXPORT void * get_mem(id_t id, std::size_t * size);
20+
21+
// Release shared memory resource and clean up disk file if reference count reaches zero.
22+
// This function decrements the reference counter. When the counter reaches zero, it:
23+
// 1. Unmaps the shared memory region
24+
// 2. Removes the backing file from disk (shm_unlink on POSIX)
25+
// 3. Frees the id structure
26+
// After calling this function, the id becomes invalid and must not be used again.
27+
// Returns: The reference count before decrement, or -1 on error.
2028
IPC_EXPORT std::int32_t release(id_t id) noexcept;
29+
30+
// Release shared memory resource and force cleanup of disk file.
31+
// This function calls release(id) internally, then unconditionally attempts to
32+
// remove the backing file. WARNING: Do NOT call this after release(id) on the
33+
// same id, as the id is already freed by release(). Use this function alone,
34+
// not in combination with release().
35+
// Typical use case: Force cleanup when you want to ensure the disk file is removed
36+
// regardless of reference count state.
2137
IPC_EXPORT void remove (id_t id) noexcept;
38+
39+
// Remove shared memory backing file by name.
40+
// This function only removes the disk file and does not affect any active memory
41+
// mappings or id structures. Use this for cleanup of orphaned files or for explicit
42+
// file removal without affecting runtime resources.
43+
// Safe to call at any time, even if shared memory is still in use elsewhere.
2244
IPC_EXPORT void remove (char const * name) noexcept;
2345

2446
IPC_EXPORT std::int32_t get_ref(id_t id);

test/test_shm.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ TEST_F(ShmTest, AcquireCreate) {
5050
EXPECT_NE(mem, nullptr);
5151
EXPECT_GE(actual_size, size);
5252

53-
shm::release(id);
53+
// Use remove(id) to clean up - it internally calls release()
5454
shm::remove(id);
5555
}
5656

@@ -78,7 +78,7 @@ TEST_F(ShmTest, AcquireCreateOrOpen) {
7878
EXPECT_NE(mem, nullptr);
7979
EXPECT_GE(actual_size, size);
8080

81-
shm::release(id);
81+
// Use remove(id) to clean up - it internally calls release()
8282
shm::remove(id);
8383
}
8484

@@ -101,7 +101,7 @@ TEST_F(ShmTest, GetMemory) {
101101
std::strcpy(static_cast<char*>(mem), test_data);
102102
EXPECT_STREQ(static_cast<char*>(mem), test_data);
103103

104-
shm::release(id);
104+
// Use remove(id) to clean up - it internally calls release()
105105
shm::remove(id);
106106
}
107107

@@ -115,7 +115,7 @@ TEST_F(ShmTest, GetMemoryNoSize) {
115115
void* mem = shm::get_mem(id, nullptr);
116116
EXPECT_NE(mem, nullptr);
117117

118-
shm::release(id);
118+
// Use remove(id) to clean up - it internally calls release()
119119
shm::remove(id);
120120
}
121121

@@ -139,7 +139,7 @@ TEST_F(ShmTest, RemoveById) {
139139
shm::id_t id = shm::acquire(name.c_str(), 256, shm::create);
140140
ASSERT_NE(id, nullptr);
141141

142-
shm::release(id);
142+
// remove(id) internally calls release(id), so we don't need to call release first
143143
shm::remove(id); // Should succeed
144144
}
145145

@@ -190,7 +190,7 @@ TEST_F(ShmTest, SubtractReference) {
190190

191191
EXPECT_EQ(ref_after, ref_before - 1);
192192

193-
shm::release(id);
193+
// Use remove(id) to clean up - it internally calls release()
194194
shm::remove(id);
195195
}
196196

0 commit comments

Comments
 (0)