From bfbcc5de439ad4128dc4cc1799547b5dd9885dfc Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 1 Apr 2018 05:33:44 +0200 Subject: [PATCH] simplify ClassLock by use of Meyer's Singleton with zombie check ...and package the ZombieCheck as helper object. Also rewrite the SyncClassLock_test to perform an multithreaded contended test to prove the lock is shared and effective --- src/lib/depend.hpp | 14 +---- src/lib/sync-classlock.hpp | 57 ++----------------- src/lib/zombie-check.hpp | 70 +++++++++--------------- tests/backend/sync-classlock-test.cpp | 79 ++++++++++++++------------- 4 files changed, 76 insertions(+), 144 deletions(-) diff --git a/src/lib/depend.hpp b/src/lib/depend.hpp index 3f4ccd85e..23da7f514 100644 --- a/src/lib/depend.hpp +++ b/src/lib/depend.hpp @@ -93,6 +93,7 @@ #include "lib/nocopy.hpp" #include "lib/nobug-init.hpp" #include "lib/sync-classlock.hpp" +#include "lib/zombie-check.hpp" #include "lib/meta/util.hpp" #include @@ -125,25 +126,16 @@ namespace lib { Creator creator_; Deleter deleter_; - bool deceased_ =false; public: + ZombieCheck zombieCheck; + DependencyFactory() = default; ~DependencyFactory() { - deceased_ = true; if (deleter_) deleter_(); } - void - zombieCheck() - { - if (deceased_) - throw error::Fatal("DependencyFactory invoked out of order during Application shutdown. " - "Lumiera Policy violated: Dependencies must not be used from destructors." - ,error::LUMIERA_ERROR_LIFECYCLE); - } - OBJ* operator() () { diff --git a/src/lib/sync-classlock.hpp b/src/lib/sync-classlock.hpp index 67279be03..de21dfe74 100644 --- a/src/lib/sync-classlock.hpp +++ b/src/lib/sync-classlock.hpp @@ -29,7 +29,7 @@ ** ** @note simply using the ClassLock may cause a Monitor object (with a mutex) to be ** created at static initialisation and destroyed on application shutdown. - ** @see singleton-factory.hpp usage example + ** @see depend.hpp usage example */ @@ -37,51 +37,12 @@ #define LIB_SYNC_CLASSLOCK_H #include "lib/nobug-init.hpp" +#include "lib/zombie-check.hpp" #include "lib/sync.hpp" namespace lib { - namespace nifty { // implementation details - - template - struct Holder - { - static uint accessed_; - static char content_[sizeof(X)]; - - Holder() - { - if (!accessed_) - new(content_) X(); - ++accessed_; - } - - ~Holder() - { - --accessed_; - if (0==accessed_) - get().~X(); - } - - X& - get() - { - X* obj = reinterpret_cast (&content_); - ASSERT (obj, "Logic of Schwartz counter broken."); - return *obj; - } - }; - - template - uint Holder::accessed_; - - template - char Holder::content_[sizeof(X)]; - - } // (End) nifty implementation details - - /** * A synchronisation protection guard employing a lock scoped @@ -109,21 +70,15 @@ namespace lib { Monitor& getPerClassMonitor() { - static nifty::Holder __used_here; - if (1 != use_count()) - { - ///////////////////////////////////////////////////////////////////////////TICKET #1133 investigate Problems with shutdown order - ERROR (progress, "AUA %d", use_count()); - } - ASSERT (1==use_count(), "static init broken"); + static PerClassMonitor classMonitor; + static ZombieCheck zombieCheck; - return __used_here.get(); + zombieCheck(); + return classMonitor; } public: ClassLock() : Lock (getPerClassMonitor()) { } - - uint use_count() { return nifty::Holder::accessed_; } }; diff --git a/src/lib/zombie-check.hpp b/src/lib/zombie-check.hpp index 79a707fc2..81ffa4fff 100644 --- a/src/lib/zombie-check.hpp +++ b/src/lib/zombie-check.hpp @@ -22,6 +22,14 @@ /** @file dependable-base.hpp ** Detector to set off alarm when (re)using deceased objects. + ** When implementing services based on static fields or objects, + ** an invocation after static shutdown can not be precluded -- be it by + ** re-entrance, be through indirect reference to some dependency within a + ** static function residing in another translation unit. Since typically the + ** values in static storage are not overwritten after invoking the destructor, + ** we may plant an automatic "zombie detector" to give a clear indication of + ** such a policy violation (Lumiera forbids to use dependencies from dtors). + ** ** @see sync-classlock.hpp ** @see depend.hpp */ @@ -30,69 +38,43 @@ #ifndef LIB_ZOMBIE_CHECK_H #define LIB_ZOMBIE_CHECK_H -#include "lib/del-stash.hpp" -#include "lib/nocopy.hpp" +#include "lib/error.hpp" -#include -#include namespace lib { - - namespace nifty { // implementation details - - template - struct Holder - { - char payload_[sizeof(X)]; - - //NOTE: deliberately no ctor/dtor - - X& - access() - { - return *reinterpret_cast (&payload_); - } - }; - - template - uint Holder::accessed_; - - } // (End) nifty implementation details - + namespace error = lumiera::error; /** - * A dependable data container available with extended lifespan. - * Automatically plants a ref-count to ensure the data stays alive - * until the last static reference goes out of scope. + * Automatic lifecycle tracker, to produce an alarm when accessing objects after deletion. */ - template - class DependableBase - : util::NonCopyable + class ZombieCheck { - static nifty::Holder storage_; + bool deceased_ = false; public: - template - explicit - DependableBase (ARGS&& ...args) + ZombieCheck() = default; + ~ZombieCheck() { - storage_.maybeInit (std::forward (args)...); + deceased_ = true; } - operator X& () const + operator bool() const { - return storage_.access(); + return deceased_; } - uint use_count() { return nifty::Holder::accessed_; } + void + operator() () const + { + if (deceased_) + throw error::Fatal("Already deceased object called out of order during Application shutdown. " + "Lumiera Policy violated: Dependencies must not be used from destructors." + ,error::LUMIERA_ERROR_LIFECYCLE); + } }; - /** plant a static buffer to hold the payload X */ - template - nifty::Holder DependableBase::storage_; - } // namespace lib #endif /*LIB_ZOMBIE_CHECK_H*/ diff --git a/tests/backend/sync-classlock-test.cpp b/tests/backend/sync-classlock-test.cpp index f8389dd22..cff0649e6 100644 --- a/tests/backend/sync-classlock-test.cpp +++ b/tests/backend/sync-classlock-test.cpp @@ -26,42 +26,22 @@ #include "lib/test/run.hpp" -#include "lib/error.hpp" - #include "lib/sync-classlock.hpp" +#include "lib/scoped-collection.hpp" +#include "backend/thread-wrapper.hpp" using test::Test; - +using backend::ThreadJoinable; namespace lib { namespace test { - namespace { // private test classes and data... + namespace { // Parameters for multithreaded contention test - const uint NUM_INSTANCES = 20; ///< number of probe instances to create + const uint NUM_THREADS = 20; ///< number of contending threads to create + const uint NUM_LOOP = 1000; ///< number of loop iterations per thread - - /** - * Several instances of this probe class will be created. - * Each of them acquires the shared lock; but anyway, just - * by defining this class, the embedded Monitor got created. - */ - struct Probe - { - ClassLock shared_lock_; - - Probe() {} - ~Probe() {} - }; - - - } // (End) test classes and data.... - - - - - - + } @@ -70,9 +50,10 @@ namespace test { * @test check proper handling of class (not instance)-based Monitor locks. * Because no instance is available in this case, a hidden storage for the * Monitor object needs to be provided in a way safe for use even in the - * static startup/shutdown phase. This test validates the associated - * refcounting and object creation works as expected. It does \em not - * validate the locking functionality as such. + * static startup/shutdown phase. This can not directly validate this + * allocation of a shared monitor object behind the scenes, but it + * can verify the monitor is indeed shared by all ClassLock instances + * templated to a specific type. * * @see sync.hpp */ @@ -82,15 +63,37 @@ namespace test { virtual void run (Arg) { - { - Probe objs[NUM_INSTANCES]; - - CHECK (1 == objs[0].shared_lock_.use_count()); - } + int contended = 0; - ClassLock get_class_lock; - CHECK ( 1 == get_class_lock.use_count()); // embedded PerClassMonitor got created exactly once - } // and stays alive until static dtors are called.... + using Threads = lib::ScopedCollection; + + // Start a bunch of threads with random access pattern + Threads threads{NUM_THREADS, + [&](Threads::ElementHolder& storage) + { + storage.create ("Sync-ClassLock stress test" + ,[&]{ + for (uint i=0; i guard; + ++contended; + } + } + }); + } + }; + + for (auto& thread : threads) + thread.join(); // block until thread terminates + + CHECK (contended == NUM_THREADS * NUM_LOOP, + "ALARM: Lock failed, concurrent modification " + "during counter increment. Delta == %d" + ,NUM_THREADS * NUM_LOOP - contended); + } };