8378442: RBTreeCHeap does not deallocate removed nodes when using remove_at_cursor

Reviewed-by: jsikstro, jsjolen
This commit is contained in:
Casper Norrbin 2026-02-25 11:51:51 +00:00
parent 3e087d8ebd
commit 269c9f3ed5
3 changed files with 76 additions and 3 deletions

View File

@ -374,10 +374,10 @@ public:
typedef typename BaseType::Cursor Cursor;
using BaseType::cursor;
using BaseType::insert_at_cursor;
using BaseType::remove_at_cursor;
using BaseType::next;
using BaseType::prev;
void remove_at_cursor(const Cursor& node_cursor);
void replace_at_cursor(RBNode<K, V>* new_node, const Cursor& node_cursor);
RBNode<K, V>* allocate_node(const K& key);

View File

@ -1103,6 +1103,15 @@ bool RBTree<K, V, COMPARATOR, ALLOCATOR>::copy_into(RBTree& other) const {
return true;
}
template <typename K, typename V, typename COMPARATOR, typename ALLOCATOR>
inline void RBTree<K, V, COMPARATOR, ALLOCATOR>::remove_at_cursor(const Cursor& node_cursor) {
precond(node_cursor.valid());
precond(node_cursor.found());
RBNode<K, V>* old_node = node_cursor.node();
BaseType::remove_at_cursor(node_cursor);
free_node(old_node);
}
template <typename K, typename V, typename COMPARATOR, typename ALLOCATOR>
inline void RBTree<K, V, COMPARATOR, ALLOCATOR>::replace_at_cursor(RBNode<K, V>* new_node, const Cursor& node_cursor) {
precond(new_node != nullptr);
@ -1170,7 +1179,7 @@ inline V* RBTree<K, V, COMPARATOR, ALLOCATOR>::find(const K& key) const {
template <typename K, typename V, typename COMPARATOR, typename ALLOCATOR>
inline void RBTree<K, V, COMPARATOR, ALLOCATOR>::remove(RBNode<K, V>* node) {
Cursor node_cursor = cursor(node);
remove_at_cursor(node_cursor);
BaseType::remove_at_cursor(node_cursor);
free_node(node);
}
@ -1181,7 +1190,7 @@ inline bool RBTree<K, V, COMPARATOR, ALLOCATOR>::remove(const K& key) {
return false;
}
RBNode<K, V>* node = node_cursor.node();
remove_at_cursor(node_cursor);
BaseType::remove_at_cursor(node_cursor);
free_node((RBNode<K, V>*)node);
return true;
}

View File

@ -115,6 +115,16 @@ struct ArrayAllocator {
using IntrusiveTreeInt = IntrusiveRBTree<int, IntrusiveCmp>;
using IntrusiveCursor = IntrusiveTreeInt::Cursor;
struct DestructionTracker {
static int destructed_count;
int value;
DestructionTracker(int value) : value(value) {}
~DestructionTracker() { destructed_count++; }
static void reset() { destructed_count = 0; }
};
public:
void inserting_duplicates_results_in_one_value() {
constexpr int up_to = 10;
@ -607,6 +617,55 @@ public:
}
}
void test_remove_destructs() {
using Tree = RBTreeCHeap<int, DestructionTracker, Cmp, mtTest>;
using Node = RBNode<int, DestructionTracker>;
using Cursor = Tree::Cursor;
Tree tree;
// Test the 3 ways of removing a single node
tree.upsert(0, DestructionTracker(0));
tree.upsert(1, DestructionTracker(1));
tree.upsert(2, DestructionTracker(2));
DestructionTracker::reset();
tree.remove(0);
Node* n = tree.find_node(1);
tree.remove(n);
Cursor remove_cursor = tree.cursor(2);
tree.remove_at_cursor(remove_cursor);
EXPECT_EQ(3, DestructionTracker::destructed_count);
// Test clearing the tree
constexpr int num_nodes = 10;
for (int n = 0; n < num_nodes; n++) {
tree.upsert(n, DestructionTracker(n));
}
DestructionTracker::reset();
tree.remove_all();
EXPECT_EQ(num_nodes, DestructionTracker::destructed_count);
// Test replacing a node
tree.upsert(0, DestructionTracker(0));
Cursor replace_cursor = tree.cursor(0);
Node* new_node = tree.allocate_node(1, DestructionTracker(1));
DestructionTracker::reset();
tree.replace_at_cursor(new_node, replace_cursor);
EXPECT_EQ(1, DestructionTracker::destructed_count);
tree.remove_at_cursor(replace_cursor);
EXPECT_EQ(2, DestructionTracker::destructed_count);
}
void test_cursor() {
constexpr int num_nodes = 10;
RBTreeInt tree;
@ -971,6 +1030,11 @@ TEST_VM_F(RBTreeTest, NodeHints) {
this->test_node_hints();
}
int RBTreeTest::DestructionTracker::destructed_count = 0;
TEST_VM_F(RBTreeTest, RemoveDestructs) {
this->test_remove_destructs();
}
TEST_VM_F(RBTreeTest, CursorFind) {
this->test_cursor();
}