Skip to content

Commit

Permalink
Make IntrusiveList lifetime documentation a little clearer. (#13525)
Browse files Browse the repository at this point in the history
Also adds verification that a nonempty list is never destroyed, since
that would leave dangling pointers back to the list from the nodes
that weren't removed from it.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Mar 3, 2022
1 parent 74e11d7 commit 5057816
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions src/lib/support/IntrusiveList.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,12 @@ class IntrusiveListBase
// \------------------------------------------/
//
IntrusiveListBase() : mNode(&mNode, &mNode) {}
~IntrusiveListBase() { mNode.Remove(); /* clear mNode such that the destructor checking mNode.IsInList doesn't fail */ }
~IntrusiveListBase()
{
VerifyOrDie(Empty());
/* clear mNode such that the destructor checking mNode.IsInList doesn't fail */
mNode.Remove();
}

ConstIteratorBase begin() const { return ConstIteratorBase(mNode.mNext); }
ConstIteratorBase end() const { return ConstIteratorBase(&mNode); }
Expand Down Expand Up @@ -216,7 +221,7 @@ class IntrusiveListBase
IntrusiveListNodeBase mNode;
};

/// The hook convert between node object T and IntrusiveListNodeBase
/// The hook converts between node object T and IntrusiveListNodeBase
///
/// When using this hook, the node type (T) MUST inherit from IntrusiveListNodeBase.
///
Expand All @@ -235,10 +240,12 @@ class IntrusiveListBaseHook

/// A double-linked list where the data is stored together with the previous/next pointers for cache efficiency / and compactness.
///
/// The default hook (IntrusiveListBaseHook<T>) requires T inherit from IntrusiveListNodeBase.
/// The default hook (IntrusiveListBaseHook<T>) requires T to inherit from IntrusiveListNodeBase.
///
/// Consumers must ensure that the IntrusiveListNodeBase object associated with
/// a node is removed from any list it might belong to before it is destroyed.
///
/// IntrusiveListNodeBase object associated with a node is assumed to be longer than the list they belong to. A list is effcively a
/// single node into / a chain of other nodes referenced by pointers.
/// Consumers must ensure that a list is empty before it is destroyed.
///
/// A node may only belong to a single list. The code will assert (via VerifyOrDie) on this invariant.
///
Expand All @@ -252,6 +259,8 @@ class IntrusiveListBaseHook
/// list.PushBack(&a);
/// list.PushFront(&b);
/// assert(list.Contains(&a) && list.Contains(&b) && !list.Contains(&c));
/// list.Remove(&a);
/// list.Remove(&b);
template <typename T, typename Hook = IntrusiveListBaseHook<T>>
class IntrusiveList : public IntrusiveListBase
{
Expand Down

0 comments on commit 5057816

Please sign in to comment.