From 50578161078819a129402d59f04010ca19503670 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 13 Jan 2022 08:39:51 -0500 Subject: [PATCH] Make IntrusiveList lifetime documentation a little clearer. (#13525) 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. --- src/lib/support/IntrusiveList.h | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/lib/support/IntrusiveList.h b/src/lib/support/IntrusiveList.h index 7555b1659aa873..dde33a8e52a313 100644 --- a/src/lib/support/IntrusiveList.h +++ b/src/lib/support/IntrusiveList.h @@ -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); } @@ -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. /// @@ -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) requires T inherit from IntrusiveListNodeBase. +/// The default hook (IntrusiveListBaseHook) 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. /// @@ -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 > class IntrusiveList : public IntrusiveListBase {