Skip to content

Commit 92ca3b9

Browse files
committed
add test
1 parent 7d9bece commit 92ca3b9

1 file changed

Lines changed: 66 additions & 0 deletions

File tree

src/paimon/common/memory/bytes_test.cpp

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,70 @@ TEST(BytesTest, TestCompare) {
8282
ASSERT_LT(*bytes1, *bytes2);
8383
ASSERT_FALSE(*bytes1 < *bytes1);
8484
}
85+
86+
// Test to verify that move assignment correctly handles memory and prevents double-free.
87+
// Before the fix, the old implementation used memcpy + destructor which caused:
88+
// 1. The target's original memory was freed in destructor
89+
// 2. After memcpy, both source and target pointed to same memory
90+
// 3. When source was "reset" via placement new, it became empty
91+
// 4. But if move assignment was called again on the same target, the memcpy'd
92+
// pointer would be freed again (double-free) or memory accounting would be wrong.
93+
TEST(BytesTest, TestMoveAssignmentNoDoubleFree) {
94+
auto pool = paimon::GetMemoryPool();
95+
96+
// Create three Bytes objects on stack
97+
Bytes a("aaaa", pool.get()); // 4 bytes
98+
Bytes b("bb", pool.get()); // 2 bytes
99+
Bytes c("cccccc", pool.get()); // 6 bytes
100+
ASSERT_EQ(12, pool->CurrentUsage()); // 4 + 2 + 6 = 12
101+
102+
// First move: b = std::move(a)
103+
// Should free b's original memory (2 bytes), transfer a's memory to b
104+
b = std::move(a);
105+
ASSERT_EQ(10, pool->CurrentUsage()); // 4 + 6 = 10 (b's 2 bytes freed)
106+
ASSERT_EQ("aaaa", std::string(b.data(), b.size()));
107+
ASSERT_EQ(nullptr, a.data());
108+
ASSERT_EQ(0, a.size());
109+
110+
// Second move: b = std::move(c)
111+
// Should free b's current memory (4 bytes from a), transfer c's memory to b
112+
// This is where the old implementation would cause issues:
113+
// - Old code would call destructor on b, freeing the 4 bytes
114+
// - Then memcpy c into b, making b point to c's 6-byte buffer
115+
// - Memory accounting would be wrong because Free was called on wrong data
116+
b = std::move(c);
117+
ASSERT_EQ(6, pool->CurrentUsage()); // Only c's 6 bytes remain (now owned by b)
118+
ASSERT_EQ("cccccc", std::string(b.data(), b.size()));
119+
ASSERT_EQ(nullptr, c.data());
120+
ASSERT_EQ(0, c.size());
121+
122+
// Self-assignment should be safe
123+
b = std::move(b);
124+
ASSERT_EQ(6, pool->CurrentUsage());
125+
ASSERT_EQ("cccccc", std::string(b.data(), b.size()));
126+
}
127+
128+
// Test move assignment with heap-allocated Bytes to verify no double-free
129+
// when combining unique_ptr semantics with move assignment
130+
TEST(BytesTest, TestMoveAssignmentHeapAllocated) {
131+
auto pool = paimon::GetMemoryPool();
132+
133+
auto bytes1 = Bytes::AllocateBytes("hello", pool.get()); // 5 bytes + sizeof(Bytes)
134+
auto bytes2 = Bytes::AllocateBytes("world!", pool.get()); // 6 bytes + sizeof(Bytes)
135+
size_t expected = 5 + 6 + 2 * sizeof(Bytes);
136+
ASSERT_EQ(expected, pool->CurrentUsage());
137+
138+
// Move the content of bytes1 into bytes2's Bytes object
139+
// This should free "world!" (6 bytes) and transfer "hello" ownership
140+
*bytes2 = std::move(*bytes1);
141+
expected = 5 + 2 * sizeof(Bytes); // "world!" freed, "hello" transferred
142+
ASSERT_EQ(expected, pool->CurrentUsage());
143+
ASSERT_EQ("hello", std::string(bytes2->data(), bytes2->size()));
144+
ASSERT_EQ(nullptr, bytes1->data());
145+
146+
// Reset bytes2, which should free "hello"
147+
bytes2.reset();
148+
expected = sizeof(Bytes); // Only bytes1's empty Bytes struct remains
149+
ASSERT_EQ(expected, pool->CurrentUsage());
150+
}
85151
} // namespace paimon::test

0 commit comments

Comments
 (0)