From eaedab90ea5635dd93d2c5d1707015f7d1c628de Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 26 Dec 2008 03:47:12 +0100 Subject: [PATCH] Refactoring IV: move the (still problematic) ClassLock out of the Sync compound (no semantic change, but better notation) --- src/lib/allocationcluster.cpp | 12 +++--- src/lib/allocationcluster.hpp | 2 +- src/lib/scopedholder.hpp | 2 +- src/lib/singletonpolicies.hpp | 10 ++--- src/lib/sync.hpp | 51 +++++++++++++---------- src/lib/visitordispatcher.hpp | 6 +-- src/proc/mobject/session/defsregistry.hpp | 4 +- 7 files changed, 47 insertions(+), 40 deletions(-) diff --git a/src/lib/allocationcluster.cpp b/src/lib/allocationcluster.cpp index 4dafb7d48..7e13e1538 100644 --- a/src/lib/allocationcluster.cpp +++ b/src/lib/allocationcluster.cpp @@ -81,7 +81,7 @@ namespace lib { void AllocationCluster::MemoryManager::reset (TypeInfo info) { - Sync<>::ClassLock guard(); + ClassLock guard(); if (0 < mem_.size()) purge(); type_ = info; @@ -96,7 +96,7 @@ namespace lib { void AllocationCluster::MemoryManager::purge() { - Sync<>::ClassLock guard(); + ClassLock guard(); REQUIRE (type_.killIt, "we need a deleter function"); REQUIRE (0 < type_.allocSize, "allocation size unknown"); @@ -120,7 +120,7 @@ namespace lib { inline void* AllocationCluster::MemoryManager::allocate() { - Sync<>::ClassLock guard(); + ClassLock guard(); REQUIRE (0 < type_.allocSize); REQUIRE (top_ <= mem_.size()); @@ -140,7 +140,7 @@ namespace lib { inline void AllocationCluster::MemoryManager::commit (void* pendingAlloc) { - Sync<>::ClassLock guard(); + ClassLock guard(); REQUIRE (pendingAlloc); ASSERT (top_ < mem_.size()); @@ -175,7 +175,7 @@ namespace lib { { try { - Sync<>::ClassLock guard(); + ClassLock guard(); TRACE (memory, "shutting down AllocationCluster"); for (size_t i = typeHandlers_.size(); 0 < i; --i) @@ -214,7 +214,7 @@ namespace lib { ASSERT (0 < slot); { - Sync<>::ClassLock guard(); /////TODO: decide tradeoff: lock just the instance, or lock the AllocationCluster class? + ClassLock guard(); /////TODO: decide tradeoff: lock just the instance, or lock the AllocationCluster class? if (slot > typeHandlers_.size()) typeHandlers_.resize(slot); diff --git a/src/lib/allocationcluster.hpp b/src/lib/allocationcluster.hpp index 51200e740..6b7a4838c 100644 --- a/src/lib/allocationcluster.hpp +++ b/src/lib/allocationcluster.hpp @@ -222,7 +222,7 @@ namespace lib { static TypeInfo setup() { - Sync<>::ClassLock guard(); + ClassLock guard(); if (!id_) id_= ++maxTypeIDs; diff --git a/src/lib/scopedholder.hpp b/src/lib/scopedholder.hpp index 986ba956b..f23bffeaa 100644 --- a/src/lib/scopedholder.hpp +++ b/src/lib/scopedholder.hpp @@ -221,7 +221,7 @@ namespace lib { return created_? &_ThisType::created_ : 0; } - bool operator! () const { return !created_; } + bool operator! () const { return !created_; } friend void diff --git a/src/lib/singletonpolicies.hpp b/src/lib/singletonpolicies.hpp index 83c76f5ca..ce01f6ce7 100644 --- a/src/lib/singletonpolicies.hpp +++ b/src/lib/singletonpolicies.hpp @@ -40,10 +40,10 @@ This code is heavily inspired by #include -namespace lumiera - { - namespace singleton - { +namespace lumiera { + namespace singleton { + + /* === several Policies usable in conjunction with lumiera::Singleton === */ /** @@ -139,7 +139,7 @@ namespace lumiera struct Multithreaded { typedef volatile S VolatileType; - typedef lib::Sync<>::ClassLock Lock; + typedef lib::ClassLock Lock; }; diff --git a/src/lib/sync.hpp b/src/lib/sync.hpp index b0a9df32a..01930506e 100644 --- a/src/lib/sync.hpp +++ b/src/lib/sync.hpp @@ -284,7 +284,7 @@ namespace lib { * @todo actually implement this facility using the Lumiera backend. */ template - struct Sync + class Sync { typedef sync::Monitor Monitor; Monitor objectMonitor_; @@ -296,22 +296,8 @@ namespace lib { return forThis->objectMonitor_; } - template - static Monitor& - getMonitor() - { - //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_; - } - + public: class Lock { Monitor& mon_; @@ -332,14 +318,35 @@ namespace lib { }; - template - struct ClassLock : Lock - { - ClassLock() : Lock (getMonitor()) {} - }; - }; + + template + class ClassLock + : public Sync::Lock + { + typedef typename Sync::Lock Lock; + typedef typename Sync::Monitor Monitor; + + + static Monitor& + getMonitor() + { + //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_; + } + + + public: + ClassLock() : Lock (getMonitor()) {} + }; } // namespace lumiera diff --git a/src/lib/visitordispatcher.hpp b/src/lib/visitordispatcher.hpp index 4c9145716..59784b398 100644 --- a/src/lib/visitordispatcher.hpp +++ b/src/lib/visitordispatcher.hpp @@ -36,7 +36,7 @@ namespace lumiera { namespace visitor { - using lib::Sync; + using lib::ClassLock; template class Tag; @@ -62,7 +62,7 @@ namespace lumiera { static void generateID (size_t& id) { - Sync<>::ClassLock guard(); + ClassLock guard(); if (!id) id = ++lastRegisteredID; } @@ -138,7 +138,7 @@ namespace lumiera { void accomodate (size_t index) { - Sync<>::ClassLock guard(); + ClassLock guard(); if (index > table_.size()) table_.resize (index); // performance bottleneck?? TODO: measure the real impact! } diff --git a/src/proc/mobject/session/defsregistry.hpp b/src/proc/mobject/session/defsregistry.hpp index f377c3fe6..3fc37e5d7 100644 --- a/src/proc/mobject/session/defsregistry.hpp +++ b/src/proc/mobject/session/defsregistry.hpp @@ -58,7 +58,7 @@ namespace mobject { using lumiera::P; using lumiera::Query; - using lib::Sync; + using lib::ClassLock; using std::tr1::weak_ptr; using std::string; @@ -162,7 +162,7 @@ namespace mobject { static void createSlot (Table& table) { - Sync<>::ClassLock guard(); + ClassLock guard(); if (!index) index = ++maxSlots; if (index > table.size())