diff --git a/AK/IntrusiveDetails.h b/AK/IntrusiveDetails.h new file mode 100644 index 0000000000..7cf9a04093 --- /dev/null +++ b/AK/IntrusiveDetails.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2021, Ali Mohammad Pur + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +namespace AK::Detail { + +template +struct SubstituteIntrusiveContainerType { + using Type = Container; +}; + +template +struct SubstituteIntrusiveContainerType> { + using Type = RefPtr; +}; + +template +struct SelfReferenceIfNeeded { + Container reference = nullptr; +}; +template +struct SelfReferenceIfNeeded { +}; + +} diff --git a/AK/IntrusiveList.h b/AK/IntrusiveList.h index 9aed1f5852..d7dc93c9b3 100644 --- a/AK/IntrusiveList.h +++ b/AK/IntrusiveList.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -17,20 +18,10 @@ namespace AK { namespace Detail { template> class IntrusiveListNode; - -template -struct SubstituteIntrusiveListNodeContainerType { - using Type = Container; -}; - -template -struct SubstituteIntrusiveListNodeContainerType> { - using Type = RefPtr; -}; } template> -using IntrusiveListNode = Detail::IntrusiveListNode::Type>; +using IntrusiveListNode = Detail::IntrusiveListNode::Type>; template class IntrusiveListStorage { @@ -157,14 +148,6 @@ private: IntrusiveListStorage m_storage; }; -template -struct SelfReferenceIfNeeded { - Contained reference = nullptr; -}; -template -struct SelfReferenceIfNeeded { -}; - namespace Detail { template diff --git a/AK/IntrusiveRedBlackTree.h b/AK/IntrusiveRedBlackTree.h index eb4bfaacac..811ff5d23c 100644 --- a/AK/IntrusiveRedBlackTree.h +++ b/AK/IntrusiveRedBlackTree.h @@ -6,15 +6,21 @@ #pragma once +#include #include namespace AK { -template +namespace Detail { +template> class IntrusiveRedBlackTreeNode; +} -template V::*member> -class IntrusiveRedBlackTree final : public BaseRedBlackTree { +template> +using IntrusiveRedBlackTreeNode = Detail::IntrusiveRedBlackTreeNode::Type>; + +template V::*member> +class IntrusiveRedBlackTree : public BaseRedBlackTree { public: IntrusiveRedBlackTree() = default; @@ -24,9 +30,9 @@ public: } using BaseTree = BaseRedBlackTree; - using TreeNode = IntrusiveRedBlackTreeNode; + using TreeNode = IntrusiveRedBlackTreeNode; - V* find(K key) + Container find(K key) { auto* node = static_cast(BaseTree::find(this->m_root, key)); if (!node) @@ -34,7 +40,7 @@ public: return node_to_value(*node); } - V* find_largest_not_above(K key) + Container find_largest_not_above(K key) { auto* node = static_cast(BaseTree::find_largest_not_above(this->m_root, key)); if (!node) @@ -45,7 +51,10 @@ public: void insert(V& value) { auto& node = value.*member; + VERIFY(!node.m_in_tree); BaseTree::insert(&node); + if constexpr (!TreeNode::IsRaw) + node.m_self.reference = &value; // Note: Self-reference ensures that the object will keep a ref to itself when the Container is a smart pointer. node.m_in_tree = true; } @@ -76,7 +85,7 @@ public: VERIFY(m_node); return *node_to_value(*m_node); } - ElementType* operator->() + auto operator->() { VERIFY(m_node); return node_to_value(*m_node); @@ -117,6 +126,8 @@ public: node->right_child = nullptr; node->left_child = nullptr; node->m_in_tree = false; + if constexpr (!TreeNode::IsRaw) + node->m_self.reference = nullptr; return true; } @@ -139,6 +150,8 @@ private: clear_nodes(static_cast(node->left_child)); node->left_child = nullptr; node->m_in_tree = false; + if constexpr (!TreeNode::IsRaw) + node->m_self.reference = nullptr; } static V* node_to_value(TreeNode& node) @@ -147,7 +160,9 @@ private: } }; -template +namespace Detail { + +template class IntrusiveRedBlackTreeNode : public BaseRedBlackTree::Node { public: IntrusiveRedBlackTreeNode(K key) @@ -165,10 +180,28 @@ public: return m_in_tree; } + static constexpr bool IsRaw = IsPointer; + +#ifndef __clang__ private: - template V::*member> - friend class IntrusiveRedBlackTree; + template TV::*member> + friend class ::AK::IntrusiveRedBlackTree; +#endif + bool m_in_tree { false }; + [[no_unique_address]] SelfReferenceIfNeeded m_self; +}; + +} + +// Specialise IntrusiveRedBlackTree for NonnullRefPtr +// By default, red black trees cannot contain null entries anyway, so switch to RefPtr +// and just make the user-facing functions deref the pointers. +template> V::*member> +class IntrusiveRedBlackTree, member> : public IntrusiveRedBlackTree, member> { +public: + [[nodiscard]] NonnullRefPtr find(K key) const { return IntrusiveRedBlackTree, member>::find(key).release_nonnull(); } + [[nodiscard]] NonnullRefPtr find_largest_not_above(K key) const { return IntrusiveRedBlackTree, member>::find_largest_not_above(key).release_nonnull(); } }; } diff --git a/Tests/AK/TestIntrusiveRedBlackTree.cpp b/Tests/AK/TestIntrusiveRedBlackTree.cpp index 207b026cec..3ae0c7fe56 100644 --- a/Tests/AK/TestIntrusiveRedBlackTree.cpp +++ b/Tests/AK/TestIntrusiveRedBlackTree.cpp @@ -18,20 +18,21 @@ public: { } - IntrusiveRedBlackTreeNode m_tree_node; + IntrusiveRedBlackTreeNode> m_tree_node; int m_some_value; }; +using IntrusiveRBTree = IntrusiveRedBlackTree, &IntrusiveTest::m_tree_node>; TEST_CASE(construct) { - IntrusiveRedBlackTree empty; + IntrusiveRBTree empty; EXPECT(empty.is_empty()); EXPECT(empty.size() == 0); } TEST_CASE(ints) { - IntrusiveRedBlackTree test; + IntrusiveRBTree test; IntrusiveTest first { 1, 10 }; test.insert(first); IntrusiveTest second { 3, 20 }; @@ -51,7 +52,7 @@ TEST_CASE(ints) TEST_CASE(largest_smaller_than) { - IntrusiveRedBlackTree test; + IntrusiveRBTree test; IntrusiveTest first { 1, 10 }; test.insert(first); IntrusiveTest second { 11, 20 }; @@ -71,7 +72,7 @@ TEST_CASE(largest_smaller_than) TEST_CASE(key_ordered_iteration) { constexpr auto amount = 10000; - IntrusiveRedBlackTree test; + IntrusiveRBTree test; NonnullOwnPtrVector m_entries; Array keys {}; @@ -104,7 +105,7 @@ TEST_CASE(key_ordered_iteration) TEST_CASE(clear) { - IntrusiveRedBlackTree test; + IntrusiveRBTree test; NonnullOwnPtrVector m_entries; for (size_t i = 0; i < 1000; i++) { auto entry = make(i, i); @@ -114,3 +115,81 @@ TEST_CASE(clear) test.clear(); EXPECT_EQ(test.size(), 0u); } + +class IntrusiveRefPtrTest : public RefCounted { +public: + IntrusiveRefPtrTest(int key) + : m_tree_node(key) + { + } + + IntrusiveRedBlackTreeNode> m_tree_node; +}; +using IntrusiveRefPtrRBTree = IntrusiveRedBlackTree, &IntrusiveRefPtrTest::m_tree_node>; + +TEST_CASE(intrusive_ref_ptr_no_ref_leaks) +{ + auto item = adopt_ref(*new IntrusiveRefPtrTest(0)); + EXPECT_EQ(1u, item->ref_count()); + IntrusiveRefPtrRBTree ref_tree; + + ref_tree.insert(*item); + EXPECT_EQ(2u, item->ref_count()); + + ref_tree.remove(0); + EXPECT_EQ(1u, item->ref_count()); +} + +TEST_CASE(intrusive_ref_ptr_clear) +{ + auto item = adopt_ref(*new IntrusiveRefPtrTest(0)); + EXPECT_EQ(1u, item->ref_count()); + IntrusiveRefPtrRBTree ref_tree; + + ref_tree.insert(*item); + EXPECT_EQ(2u, item->ref_count()); + + ref_tree.clear(); + EXPECT_EQ(1u, item->ref_count()); +} + +TEST_CASE(intrusive_ref_ptr_destructor) +{ + auto item = adopt_ref(*new IntrusiveRefPtrTest(0)); + EXPECT_EQ(1u, item->ref_count()); + + { + IntrusiveRefPtrRBTree ref_tree; + ref_tree.insert(*item); + EXPECT_EQ(2u, item->ref_count()); + } + + EXPECT_EQ(1u, item->ref_count()); +} + +class IntrusiveNonnullRefPtrTest : public RefCounted { +public: + IntrusiveNonnullRefPtrTest(int key) + : m_tree_node(key) + { + } + + IntrusiveRedBlackTreeNode> m_tree_node; +}; +using IntrusiveNonnullRefPtrRBTree = IntrusiveRedBlackTree, &IntrusiveNonnullRefPtrTest::m_tree_node>; + +TEST_CASE(intrusive_nonnull_ref_ptr_intrusive) +{ + auto item = adopt_ref(*new IntrusiveNonnullRefPtrTest(0)); + EXPECT_EQ(1u, item->ref_count()); + IntrusiveNonnullRefPtrRBTree nonnull_ref_tree; + + nonnull_ref_tree.insert(*item); + EXPECT_EQ(2u, item->ref_count()); + EXPECT(!nonnull_ref_tree.is_empty()); + + nonnull_ref_tree.remove(0); + EXPECT_EQ(1u, item->ref_count()); + + EXPECT(nonnull_ref_tree.is_empty()); +}