From 50306db1644b03132f130f573edf04db0dfa7f6a Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Tue, 18 Jun 2024 18:15:58 +0200 Subject: [PATCH] Library: more stringent deleter logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setup for `ArrayBucket` is special, insofar it shell de-allocate itself, which creates the danger of re-entrant calls, or to the contrary, the danger to invoke this clean-up function without actually invoking the destructor. These problems become relevant once the destructor function itself is statefull, as is the case when embedding a non-trivial, instance bound allocator to be used for the clean-up work. Using the new `lib::TrackingAllocator` highlighted this potential problem, since the allocator maintains a use-count. Thus I decided to move the »destruction mechanics« one level down into a dedicated and well encapsulated base class; invoking ArrayBucket's destructor thereby becomes the only way to trigger the clean-up, and even ElementFactory::destroy() can now safely check if the destructor was already invoked, and otherwise re-invoke itself through this embedded destructor function. Moreover, as an additional safety measure, the actual destructor function is now moved into the local stack frame of the object's destructor call, removing any possibility for the de-allocation to interfere with the destructor invokation itself --- src/lib/several-builder.hpp | 12 ++++-- src/lib/several.hpp | 44 ++++++++++++++++---- tests/library/several-builder-test.cpp | 2 +- wiki/thinkPad.ichthyo.mm | 57 ++++++++++++++++++++++---- 4 files changed, 95 insertions(+), 20 deletions(-) diff --git a/src/lib/several-builder.hpp b/src/lib/several-builder.hpp index ded0b5437..bb366aaa7 100644 --- a/src/lib/several-builder.hpp +++ b/src/lib/several-builder.hpp @@ -244,6 +244,12 @@ namespace lib { destroy (ArrayBucket* bucket) { REQUIRE (bucket); + if (bucket->isArmed()) + { // ensure the bucket's destructor is invoked + // and in turn itself invokes this function + bucket->destroy(); + return; + } if (not is_trivially_destructible_v) { size_t cnt = bucket->cnt; @@ -279,7 +285,7 @@ namespace lib { Bucket* newBucket = Fac::create (cnt, spread, alignof(E)); if (data) try { - newBucket->deleter = data->deleter; + newBucket->installDestructor (data->getDtor()); size_t elms = min (cnt, data->cnt); for (size_t idx=0; idx(); - if (Coll::data_->deleter) return; - Coll::data_->deleter = deleterFunctor; + if (Coll::data_->isArmed()) return; + Coll::data_->installDestructor (move (deleterFunctor)); } /** ensure sufficient element capacity or the ability to adapt element spread */ diff --git a/src/lib/several.hpp b/src/lib/several.hpp index bd54806af..6424be073 100644 --- a/src/lib/several.hpp +++ b/src/lib/several.hpp @@ -73,6 +73,40 @@ namespace lib { namespace {// Storage header implementation details + /** @internal mix-in for self-destruction capabilities + * @remark the destructor function is assumed to perform deallocation; + * thus the functor is moved in the local stack frame, where it + * can be invoked safely; this also serves to prevent re-entrance. + */ + template + class SelfDestructor + { + std::function dtor_{}; + + public: + template + void + installDestructor (FUN&& dtor) + { + dtor_ = std::forward (dtor); + } + + ~SelfDestructor() + { + if (isArmed()) + { + auto destructionFun = std::move(dtor_); + ENSURE (not dtor_); + destructionFun (target()); + } + } + + bool isArmed() const { return bool(dtor_); } + auto getDtor() const { return dtor_; } + void destroy() { target()->~TAR(); } + TAR* target() { return static_cast(this); } + }; + /** * Metadata record placed immediately before the data storage. * @remark SeveralBuilder uses a custom allocation scheme to acquire @@ -80,13 +114,13 @@ namespace lib { */ template struct ArrayBucket + : SelfDestructor> { ArrayBucket (size_t storageSize, size_t buffStart, size_t elmSize = sizeof(I)) : cnt{0} , spread{elmSize} , buffSiz{storageSize - buffStart} , buffOffset{buffStart} - , deleter{nullptr} { } using Deleter = std::function; @@ -95,7 +129,6 @@ namespace lib { size_t spread; size_t buffSiz; size_t buffOffset; - Deleter deleter; static constexpr size_t storageOffset = sizeof(ArrayBucket); @@ -114,13 +147,6 @@ namespace lib { ENSURE (storage() <= elm and elm < storage()+buffSiz); return * std::launder (reinterpret_cast (elm)); } - - void - destroy() - { - if (deleter) - deleter (this); - } }; }//(End)implementation details diff --git a/tests/library/several-builder-test.cpp b/tests/library/several-builder-test.cpp index ba2156a2f..a47538e71 100644 --- a/tests/library/several-builder-test.cpp +++ b/tests/library/several-builder-test.cpp @@ -570,7 +570,7 @@ SHOW_EXPR(TrackingAllocator::use_count()); SHOW_TYPE(decltype(builder)) SHOW_EXPR(builder.size()) SHOW_EXPR(builder.capacity()) - builder.fillElm(11); + builder.fillElm(55); SHOW_EXPR(builder.size()) SHOW_EXPR(builder.capacity()) diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 12200fcd7..4e7143f80 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -83973,12 +83973,12 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - + @@ -83994,8 +83994,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - - + + @@ -84022,7 +84022,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - + @@ -84034,10 +84034,12 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 + + - + @@ -84056,7 +84058,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -84091,6 +84093,47 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 + + + + + + + + + + + + + + + + + + + +

+ ...konkret, ich plane einen Satz an Steuer-Flags, und auf dieser Basis dann die Belegung weiterer Storage; im einfachsten Fall gibt es keinen Spread, keinen Deleter und einen Standard-Offset; es muß dann nur die Element-Zahl und Kapazität gespeichert werden. +

+ + + + + + + + + + + + + + + + + + +