From bd2ead37d591f495fc5c94449362c193262a4d97 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 26 Dec 2008 05:44:49 +0100 Subject: [PATCH] Refactoring V: get lifecycle of a class-based monitor correct. --- src/lib/sync-classlock.hpp | 74 ++++++++++++++++++----- tests/40components.tests | 20 +++++-- tests/lib/sync-classlock-test.cpp | 98 +++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 20 deletions(-) create mode 100644 tests/lib/sync-classlock-test.cpp diff --git a/src/lib/sync-classlock.hpp b/src/lib/sync-classlock.hpp index 991291806..2cb37d17a 100644 --- a/src/lib/sync-classlock.hpp +++ b/src/lib/sync-classlock.hpp @@ -27,6 +27,8 @@ ** being problematic in conjunction with static startup/shutdown, doing so is sometimes ** necessary to setup type based dispatcher tables, managing singleton creation etc. ** + ** @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 singletonfactory.hpp usage example */ @@ -39,31 +41,73 @@ namespace lib { + namespace { // implementation details + + template + struct NiftyHolder + { + static uint accessed_; + static char content_[sizeof(X)]; + + NiftyHolder() + { + if (!accessed_) + new(content_) X(); + ++accessed_; + } + + ~NiftyHolder() + { + --accessed_; + if (0==accessed_) + get().~X(); + } + + X& + get() + { + X* obj = reinterpret_cast (&content_); + ASSERT (obj, "Logic of Schwartz counter broken."); + return *obj; + } + }; + + template + uint NiftyHolder::accessed_; + + template + char NiftyHolder::content_[sizeof(X)]; + + } // (End) implementation details + + + + /** + * A synchronisation protection guard employing a lock scoped + * to the parameter type as a whole, not an individual instance. + * After creating an instance, every other access specifying the same + * type is blocked. + * @warn beware of recursion when using a nonrecursive Mutex + * @see Sync::Lock the usual simple instance-bound variant + */ template class ClassLock : public Sync::Lock { typedef typename Sync::Lock Lock; - typedef typename Sync::Monitor Monitor; + typedef typename sync::Monitor Monitor; - - static Monitor& - getMonitor() + Monitor& + getPerClassMonitor() { - //TODO: a rather obscure race condition is hidden here: - //TODO: depending on the build order, the dtor of this static variable may be called, while another thread is still holding an ClassLock. - //TODO: An possible solution would be to use an shared_ptr to the Monitor in case of a ClassLock and to protect access with another Mutex. - //TODO. But I am really questioning if we can't ignore this case and state: "don't hold a ClassLock when your code maybe still running in shutdown phase!" - //TODO: probably best Idea is to detect this situation in DEBUG or ALPHA mode - - static scoped_ptr classMonitor_ (0); - if (!classMonitor_) classMonitor_.reset (new Monitor ()); - return *classMonitor_; + static NiftyHolder __used_here; + return __used_here.get(); } - public: - ClassLock() : Lock (getMonitor()) {} + ClassLock() : Lock (getPerClassMonitor()) {} + + uint use_count() { return NiftyHolder::accessed_; } }; diff --git a/tests/40components.tests b/tests/40components.tests index c1bf07b0c..ca079d7ef 100644 --- a/tests/40components.tests +++ b/tests/40components.tests @@ -158,11 +158,6 @@ return: 0 END -PLANNED "Multithread Locking by Monitor" ConcurrencyLocking_test < Testgroup=ALL diff --git a/tests/lib/sync-classlock-test.cpp b/tests/lib/sync-classlock-test.cpp new file mode 100644 index 000000000..d53c031ef --- /dev/null +++ b/tests/lib/sync-classlock-test.cpp @@ -0,0 +1,98 @@ +/* + SyncClasslock(Test) - validate the type-based Monitor locking + + Copyright (C) Lumiera.org + 2008, Hermann Vosseler + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of the + License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + +* *****************************************************/ + + +#include "lib/test/run.hpp" +#include "include/error.hpp" + +#include "lib/sync-classlock.hpp" + +//#include + +//using std::cout; +using test::Test; + + +namespace lib { + namespace test { + + namespace { // private test classes and data... + + const uint NUM_INSTANCES = 20; ///< number of probe instances to create + + + /** + * instances of this probe class will be created statically. + * They utilise the class-based locking within ctor and dtor + */ + struct Probe + { + Probe() { ClassLock ctor_lock; } + ~Probe() { ClassLock dtor_lock; } + }; + + + } // (End) test classes and data.... + + + + + + + + + + + /************************************************************************** + * @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. + * + * @see sync.hpp + */ + class SyncClasslock_test : public Test + { + + virtual void + run (Arg) + { + static Probe objs[NUM_INSTANCES]; + + ClassLock get_class_lock; + ASSERT ( 1 == get_class_lock.use_count()); // ClassLock got created exactly once + } + + }; + + + + /** Register this test class... */ + LAUNCHER (SyncClasslock_test, "unit common"); + + + + } // namespace test + +} // namespace lumiera