From 55447b05ac880b3ea979fc16bcdb6d458a4032e9 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Thu, 12 Sep 2013 04:35:13 -0700 Subject: [PATCH] New SharedSingleton, resolves destruction of objects with static storage duration. --- Builds/VisualStudio2012/beast.vcxproj | 3 +- Builds/VisualStudio2012/beast.vcxproj.filters | 9 +- TODO.txt | 7 +- modules/beast_core/beast_core.h | 3 +- modules/beast_core/files/beast_File.cpp | 13 +- modules/beast_core/memory/SharedPtr.h | 38 ++-- ...st_SharedSingleton.h => SharedSingleton.h} | 172 +++++++++--------- .../memory/beast_GlobalFifoFreeStore.h | 10 +- .../memory/beast_GlobalPagedFreeStore.cpp | 9 +- .../memory/beast_GlobalPagedFreeStore.h | 8 +- .../beast_core/thread/beast_DeadlineTimer.cpp | 16 +- .../beast_core/thread/beast_DeadlineTimer.h | 2 +- .../thread/beast_GlobalThreadGroup.h | 49 ----- modules/beast_core/thread/beast_Listeners.cpp | 2 +- modules/beast_core/thread/beast_ParallelFor.h | 5 +- modules/beast_sqdb/api/session.h | 2 +- modules/beast_sqdb/source/session.cpp | 31 ++-- 17 files changed, 150 insertions(+), 229 deletions(-) rename modules/beast_core/memory/{beast_SharedSingleton.h => SharedSingleton.h} (57%) delete mode 100644 modules/beast_core/thread/beast_GlobalThreadGroup.h diff --git a/Builds/VisualStudio2012/beast.vcxproj b/Builds/VisualStudio2012/beast.vcxproj index 422e763b14..1fbc577a85 100644 --- a/Builds/VisualStudio2012/beast.vcxproj +++ b/Builds/VisualStudio2012/beast.vcxproj @@ -210,7 +210,7 @@ - + @@ -285,7 +285,6 @@ - diff --git a/Builds/VisualStudio2012/beast.vcxproj.filters b/Builds/VisualStudio2012/beast.vcxproj.filters index 48c0f1d7c8..7e8967d03e 100644 --- a/Builds/VisualStudio2012/beast.vcxproj.filters +++ b/Builds/VisualStudio2012/beast.vcxproj.filters @@ -557,9 +557,6 @@ beast_core\memory - - beast_core\memory - beast_core\maths @@ -728,9 +725,6 @@ beast_core\thread - - beast_core\thread - beast_core\thread @@ -1028,6 +1022,9 @@ beast_asio\basics + + beast_core\memory + diff --git a/TODO.txt b/TODO.txt index bf1437e8e0..777b5dcc9f 100644 --- a/TODO.txt +++ b/TODO.txt @@ -2,9 +2,12 @@ BEAST TODO -------------------------------------------------------------------------------- -- Use new file naming convention +- Remove ReadWriteMutex and replace it with a CriticalSection + since the implementation is broken. -- Rename ReadWriteMutex to SharedMutex +- Rewrite SharedData to work with a CriticalSection + +- Use new file naming convention - Use SemanticVersion for beast version numbers to replace BEAST_VERSION diff --git a/modules/beast_core/beast_core.h b/modules/beast_core/beast_core.h index 1e0e7a0527..cb65de75c4 100644 --- a/modules/beast_core/beast_core.h +++ b/modules/beast_core/beast_core.h @@ -363,7 +363,7 @@ extern BEAST_API void BEAST_CALLTYPE logAssertion (char const* file, int line) n #include "memory/beast_ByteOrder.h" #include "memory/beast_Memory.h" #include "memory/beast_OptionalScopedPointer.h" -#include "memory/beast_SharedSingleton.h" +#include "memory/SharedSingleton.h" #include "memory/beast_WeakReference.h" #include "memory/beast_RecycledObjectPool.h" #include "misc/beast_Main.h" @@ -421,7 +421,6 @@ extern BEAST_API void BEAST_CALLTYPE logAssertion (char const* file, int line) n #include "thread/beast_InterruptibleThread.h" #include "thread/beast_ThreadGroup.h" #include "thread/beast_CallQueue.h" -#include "thread/beast_GlobalThreadGroup.h" #include "thread/beast_Listeners.h" #include "thread/beast_ManualCallQueue.h" #include "thread/beast_ParallelFor.h" diff --git a/modules/beast_core/files/beast_File.cpp b/modules/beast_core/files/beast_File.cpp index 319f4ab965..9717dca5da 100644 --- a/modules/beast_core/files/beast_File.cpp +++ b/modules/beast_core/files/beast_File.cpp @@ -24,20 +24,15 @@ // We need to make a shared singleton or else there are // issues with the leak detector and order of detruction. // -class NonexistentHolder : public SharedSingleton +class NonexistentHolder { public: - NonexistentHolder () - : SharedSingleton (SingletonLifetime::persistAfterCreation) + static NonexistentHolder* getInstance() { + return SharedSingleton ::getInstance(); } - static NonexistentHolder* createInstance () - { - return new NonexistentHolder; - } - - File const file; + File file; }; File const& File::nonexistent () diff --git a/modules/beast_core/memory/SharedPtr.h b/modules/beast_core/memory/SharedPtr.h index e619a64805..989950d24a 100644 --- a/modules/beast_core/memory/SharedPtr.h +++ b/modules/beast_core/memory/SharedPtr.h @@ -74,7 +74,7 @@ public: Requirement: U* must be convertible to T* */ - /// @{ + /** @{ */ SharedPtr (T* t) noexcept : m_p (acquire (t)) { @@ -85,14 +85,14 @@ public: : m_p (acquire (u)) { } - /// @} + /** @} */ /** Construct a container holding an object from another container. This will increment the object's reference-count (if it is non-null). Requirement: U* must be convertible to T* */ - /// @{ + /** @{ */ #if BEAST_SHAREDPTR_PROVIDE_COMPILER_WORKAROUNDS SharedPtr (SharedPtr const& sp) noexcept : m_p (acquire (sp.get ())) @@ -105,7 +105,7 @@ public: : m_p (acquire (sp.get ())) { } - /// @} + /** @} */ /** Assign a different object to the container. The previous object beind held, if any, loses a reference count and @@ -113,7 +113,7 @@ public: Requirement: U* must be convertible to T* */ - /// @{ + /** @{ */ #if BEAST_SHAREDPTR_PROVIDE_COMPILER_WORKAROUNDS SharedPtr& operator= (T* t) { @@ -126,10 +126,10 @@ public: { return assign (u); } - /// @} + /** @} */ /** Assign an object from another container to this one. */ - /// @{ + /** @{ */ SharedPtr& operator= (SharedPtr const& sp) { return assign (sp.get ()); @@ -141,7 +141,7 @@ public: { return assign (sp.get ()); } - /// @} + /** @} */ #if BEAST_COMPILER_SUPPORTS_MOVE_SEMANTICS /** Construct a container with an object transferred from another container. @@ -149,7 +149,7 @@ public: Requires: U* must be convertible to T* */ - /// @{ + /** @{ */ #if BEAST_SHAREDPTR_PROVIDE_COMPILER_WORKAROUNDS SharedPtr (SharedPtr && sp) noexcept : m_p (sp.swap (nullptr)) @@ -162,14 +162,14 @@ public: : m_p (sp.swap (nullptr)) { } - /// @} + /** @} */ /** Transfer ownership of another object to this container. The originating container loses its reference to the object. Requires: U* must be convertible to T* */ - /// @{ + /** @{ */ #if BEAST_SHAREDPTR_PROVIDE_COMPILER_WORKAROUNDS SharedPtr& operator= (SharedPtr && sp) { @@ -182,7 +182,7 @@ public: { return assign (sp.swap (nullptr)); } - /// @} + /** @} */ #endif /** Destroy the container and release the held reference, if any. @@ -262,8 +262,8 @@ private: //------------------------------------------------------------------------------ -/// SharedPtr comparisons. -/// @{ +/** SharedPtr comparisons. */ +/** @{ */ template bool operator== (SharedPtr const& lhs, U* const rhs) noexcept { @@ -277,28 +277,28 @@ bool operator== (SharedPtr const& lhs, SharedPtr const& rhs) noexcept } template -bool operator== (T const* lhs, SharedPtr const& rhs) noexcept +bool operator== (T const* lhs, SharedPtr const& rhs) noexcept { return lhs == rhs.get(); } template -bool operator!= (SharedPtr const& lhs, T const* rhs) noexcept +bool operator!= (SharedPtr const& lhs, U const* rhs) noexcept { return lhs.get() != rhs; } template -bool operator!= (SharedPtr const& lhs, SharedPtr const& rhs) noexcept +bool operator!= (SharedPtr const& lhs, SharedPtr const& rhs) noexcept { return lhs.get() != rhs.get(); } template -bool operator!= (T const* lhs, SharedPtr const& rhs) noexcept +bool operator!= (T const* lhs, SharedPtr const& rhs) noexcept { return lhs != rhs.get(); } -/// @} +/** @} */ #endif diff --git a/modules/beast_core/memory/beast_SharedSingleton.h b/modules/beast_core/memory/SharedSingleton.h similarity index 57% rename from modules/beast_core/memory/beast_SharedSingleton.h rename to modules/beast_core/memory/SharedSingleton.h index 8e8b146113..a14a146ef5 100644 --- a/modules/beast_core/memory/beast_SharedSingleton.h +++ b/modules/beast_core/memory/SharedSingleton.h @@ -17,8 +17,8 @@ */ //============================================================================== -#ifndef BEAST_REFERENCECOUNTEDSINGLETON_H_INCLUDED -#define BEAST_REFERENCECOUNTEDSINGLETON_H_INCLUDED +#ifndef BEAST_SHAREDSINGLETON_H_INCLUDED +#define BEAST_SHAREDSINGLETON_H_INCLUDED /** Thread-safe singleton which comes into existence on first use. Use this instead of creating objects with static storage duration. These singletons @@ -52,11 +52,6 @@ public: */ createOnDemand, - /** Like createOnDemand, but after the Singleton is destroyed an - exception will be thrown if an attempt is made to create it again. - */ - createOnDemandOnce, - /** The singleton is created on first use and persists until program exit. */ persistAfterCreation, @@ -71,131 +66,128 @@ public: //------------------------------------------------------------------------------ +/** Wraps object to produce a reference counted singleton. */ template class SharedSingleton - : public SingletonLifetime - , private PerformedAtExit + : public Object + , private SharedObject { -protected: - typedef SpinLock LockType; - - /** Create the singleton. - - @param lifetime The lifetime management option. - */ - explicit SharedSingleton (Lifetime const lifetime) - : m_lifetime (lifetime) - { - bassert (s_instance == nullptr); - - if (m_lifetime == persistAfterCreation || - m_lifetime == neverDestroyed) - { - incReferenceCount (); - } - } - - virtual ~SharedSingleton () - { - bassert (s_instance == nullptr); - } - public: - typedef SharedPtr Ptr; + typedef SharedPtr > Ptr; - /** Retrieve a reference to the singleton. - */ - static Ptr getInstance () + static Ptr getInstance (SingletonLifetime::Lifetime lifetime + = SingletonLifetime::persistAfterCreation) { - Ptr instance; - - instance = s_instance; - + StaticData& staticData (getStaticData ()); + SharedSingleton* instance = staticData.instance; if (instance == nullptr) { - LockType::ScopedLockType lock (*s_mutex); - - instance = s_instance; - + LockType::ScopedLockType lock (staticData.mutex); + instance = staticData.instance; if (instance == nullptr) { - s_instance = Object::createInstance (); - - instance = s_instance; + staticData.instance = &staticData.object; + ::new (staticData.instance) SharedSingleton (lifetime); + instance = staticData.instance; } } - return instance; } - inline void incReferenceCount () noexcept - { - ++m_refCount; - } - - inline void decReferenceCount () noexcept - { - if (--m_refCount == 0) - destroySingleton (); - } - - // Caller must synchronize. - inline bool isBeingReferenced () const - { - return m_refCount.get () != 0; - } - private: + explicit SharedSingleton (SingletonLifetime::Lifetime lifetime) + : m_lifetime (lifetime) + , m_exitHook (this) + { + if (m_lifetime == SingletonLifetime::persistAfterCreation) + this->incReferenceCount (); + } + + ~SharedSingleton () + { + } + void performAtExit () { if (m_lifetime == SingletonLifetime::persistAfterCreation) - decReferenceCount (); + this->decReferenceCount (); } - void destroySingleton () + void destroy () const { - bool destroy; + bool callDestructor; // Handle the condition where one thread is releasing the last // reference just as another thread is trying to acquire it. // { - LockType::ScopedLockType lock (*s_mutex); + StaticData& staticData (getStaticData ()); + LockType::ScopedLockType lock (staticData.mutex); - if (isBeingReferenced ()) + if (this->getReferenceCount() != 0) { - destroy = false; + callDestructor = false; } else { - destroy = true; - s_instance = 0; + callDestructor = true; + staticData.instance = nullptr; } } - if (destroy) + if (callDestructor) { - bassert (m_lifetime != neverDestroyed); + bassert (m_lifetime != SingletonLifetime::neverDestroyed); - delete static_cast (this); + this->~SharedSingleton(); } } -private: - Lifetime const m_lifetime; - Atomic m_refCount; + typedef SpinLock LockType; -private: - static Object* s_instance; - static Static::Storage > s_mutex; + class ExitHook + { + public: + explicit ExitHook (SharedSingleton* owner) + : m_owner (owner) + { + } + + void performaAtExit () + { + m_owner->performAtExit(); + } + + private: + SharedSingleton* m_owner; + }; + + // This structure gets zero-filled at static initialization time. + // No constructors are called. + // + struct StaticData + { + LockType mutex; + SharedSingleton* instance; + SharedSingleton object; + + private: + StaticData(); + ~StaticData(); + }; + + static StaticData& getStaticData () + { + static uint8 storage [sizeof (StaticData)]; + return *(reinterpret_cast (&storage [0])); + } + + friend class SharedPtr ; + + SingletonLifetime::Lifetime m_lifetime; + ExitHook m_exitHook; }; -/** @{ */ -template -Object* SharedSingleton ::s_instance; - -template -Static::Storage ::LockType, SharedSingleton > -SharedSingleton ::s_mutex; +//------------------------------------------------------------------------------ #endif diff --git a/modules/beast_core/memory/beast_GlobalFifoFreeStore.h b/modules/beast_core/memory/beast_GlobalFifoFreeStore.h index 1c43cc15e7..a7c6ecc9e2 100644 --- a/modules/beast_core/memory/beast_GlobalFifoFreeStore.h +++ b/modules/beast_core/memory/beast_GlobalFifoFreeStore.h @@ -27,7 +27,7 @@ @ingroup beast_concurrent */ template -class GlobalFifoFreeStore : public SharedSingleton > +class GlobalFifoFreeStore { public: inline void* allocate (size_t bytes) @@ -40,15 +40,15 @@ public: FifoFreeStoreType::deallocate (p); } - static GlobalFifoFreeStore* createInstance () + typedef SharedPtr > Ptr; + + static Ptr getInstance () { - return new GlobalFifoFreeStore; + return SharedSingleton ::getInstance(); } public: GlobalFifoFreeStore () - : SharedSingleton > - (SingletonLifetime::persistAfterCreation) { } diff --git a/modules/beast_core/memory/beast_GlobalPagedFreeStore.cpp b/modules/beast_core/memory/beast_GlobalPagedFreeStore.cpp index 1074728c51..8a464ed673 100644 --- a/modules/beast_core/memory/beast_GlobalPagedFreeStore.cpp +++ b/modules/beast_core/memory/beast_GlobalPagedFreeStore.cpp @@ -27,8 +27,7 @@ static const size_t globalPageBytes = 8 * 1024; } GlobalPagedFreeStore::GlobalPagedFreeStore () - : SharedSingleton (SingletonLifetime::persistAfterCreation) - , m_allocator (globalPageBytes) + : m_allocator (globalPageBytes) { } @@ -36,7 +35,9 @@ GlobalPagedFreeStore::~GlobalPagedFreeStore () { } -GlobalPagedFreeStore* GlobalPagedFreeStore::createInstance () +GlobalPagedFreeStore::Ptr GlobalPagedFreeStore::getInstance () { - return new GlobalPagedFreeStore; + return SharedSingleton ::getInstance(); } + + diff --git a/modules/beast_core/memory/beast_GlobalPagedFreeStore.h b/modules/beast_core/memory/beast_GlobalPagedFreeStore.h index 5e2d4e3ca4..9b573dfbdd 100644 --- a/modules/beast_core/memory/beast_GlobalPagedFreeStore.h +++ b/modules/beast_core/memory/beast_GlobalPagedFreeStore.h @@ -26,9 +26,7 @@ @ingroup beast_concurrent */ -class BEAST_API GlobalPagedFreeStore - : public SharedSingleton - , LeakChecked +class BEAST_API GlobalPagedFreeStore : public LeakChecked { public: GlobalPagedFreeStore (); @@ -50,7 +48,9 @@ public: PagedFreeStore::deallocate (p); } - static GlobalPagedFreeStore* createInstance (); + typedef SharedPtr > Ptr; + + static Ptr getInstance (); private: PagedFreeStore m_allocator; diff --git a/modules/beast_core/thread/beast_DeadlineTimer.cpp b/modules/beast_core/thread/beast_DeadlineTimer.cpp index fc8bef5a65..318aa65c6c 100644 --- a/modules/beast_core/thread/beast_DeadlineTimer.cpp +++ b/modules/beast_core/thread/beast_DeadlineTimer.cpp @@ -17,18 +17,14 @@ */ //============================================================================== -class DeadlineTimer::Manager - : public SharedSingleton - , protected Thread +class DeadlineTimer::Manager : protected Thread { private: typedef CriticalSection LockType; typedef List Items; public: - Manager () - : SharedSingleton (SingletonLifetime::persistAfterCreation) - , Thread ("DeadlineTimer::Manager") + Manager () : Thread ("DeadlineTimer::Manager") { startThread (); } @@ -200,11 +196,6 @@ public: } } - static Manager* createInstance () - { - return new Manager; - } - private: CriticalSection m_mutex; Items m_items; @@ -214,7 +205,8 @@ private: DeadlineTimer::DeadlineTimer (Listener* listener) : m_listener (listener) - , m_manager (Manager::getInstance ()) + , m_manager (SharedSingleton ::getInstance ( + SingletonLifetime::persistAfterCreation)) , m_isActive (false) { } diff --git a/modules/beast_core/thread/beast_DeadlineTimer.h b/modules/beast_core/thread/beast_DeadlineTimer.h index a34e5903e4..debfebe810 100644 --- a/modules/beast_core/thread/beast_DeadlineTimer.h +++ b/modules/beast_core/thread/beast_DeadlineTimer.h @@ -112,7 +112,7 @@ private: class Manager; Listener* const m_listener; - SharedPtr m_manager; + SharedPtr > m_manager; bool m_isActive; Time m_notificationTime; double m_secondsRecurring; // non zero if recurring diff --git a/modules/beast_core/thread/beast_GlobalThreadGroup.h b/modules/beast_core/thread/beast_GlobalThreadGroup.h deleted file mode 100644 index 689b258b9c..0000000000 --- a/modules/beast_core/thread/beast_GlobalThreadGroup.h +++ /dev/null @@ -1,49 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of Beast: https://github.com/vinniefalco/Beast - Copyright 2013, Vinnie Falco - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#ifndef BEAST_GLOBALTHREADGROUP_H_INCLUDED -#define BEAST_GLOBALTHREADGROUP_H_INCLUDED - -/*============================================================================*/ -/** - A ThreadGroup singleton. - - @see ThreadGroup - - @ingroup beast_concurrent -*/ -class BEAST_API GlobalThreadGroup : public ThreadGroup, - public SharedSingleton -{ -private: - friend class SharedSingleton ; - - GlobalThreadGroup () - : SharedSingleton ( - SingletonLifetime::persistAfterCreation) - { - } - - static GlobalThreadGroup* createInstance () - { - return new GlobalThreadGroup; - } -}; - -#endif diff --git a/modules/beast_core/thread/beast_Listeners.cpp b/modules/beast_core/thread/beast_Listeners.cpp index 620cda2d82..72924a8f8b 100644 --- a/modules/beast_core/thread/beast_Listeners.cpp +++ b/modules/beast_core/thread/beast_Listeners.cpp @@ -616,7 +616,7 @@ void ListenersBase::remove_void (void* const listener) // Remove it from the list and manually release // the reference since the list uses raw pointers. - m_groups.erase (m_groups.iterator_to (*group); + m_groups.erase (m_groups.iterator_to (*group)); group->decReferenceCount (); // It is still possible for the group to exist at this diff --git a/modules/beast_core/thread/beast_ParallelFor.h b/modules/beast_core/thread/beast_ParallelFor.h index f6634357b0..30dcc50992 100644 --- a/modules/beast_core/thread/beast_ParallelFor.h +++ b/modules/beast_core/thread/beast_ParallelFor.h @@ -60,10 +60,9 @@ public: It is best to keep this object around instead of creating and destroying it every time you need to run a loop. - @param pool The ThreadGroup to use. If this is omitted then a singleton - ThreadGroup is used which contains one thread per CPU. + @param pool The ThreadGroup to use. */ - explicit ParallelFor (ThreadGroup& pool = *GlobalThreadGroup::getInstance ()); + explicit ParallelFor (ThreadGroup& pool); /** Determine the number of threads in the group. diff --git a/modules/beast_sqdb/api/session.h b/modules/beast_sqdb/api/session.h index 8008c01bac..3a77093764 100644 --- a/modules/beast_sqdb/api/session.h +++ b/modules/beast_sqdb/api/session.h @@ -135,7 +135,7 @@ private: private: class Sqlite3; - SharedPtr m_instance; + SharedPtr > m_instance; bool m_bInTransaction; sqlite3* m_connection; String m_fileName; diff --git a/modules/beast_sqdb/source/session.cpp b/modules/beast_sqdb/source/session.cpp index f3a1ba8e18..517cb3fcd6 100644 --- a/modules/beast_sqdb/source/session.cpp +++ b/modules/beast_sqdb/source/session.cpp @@ -60,12 +60,10 @@ namespace sqdb { -class session::Sqlite3 : public SharedSingleton +class session::Sqlite3 { public: - friend class SharedSingleton ; - - Sqlite3() : SharedSingleton (SingletonLifetime::persistAfterCreation) + Sqlite3() { int threadSafe = sqlite3_threadsafe(); @@ -84,30 +82,25 @@ public: { sqlite3_shutdown(); } - - static Sqlite3* createInstance() - { - return new Sqlite3; - } }; //------------------------------------------------------------------------------ session::session() - : prepare(this) - , m_instance(Sqlite3::getInstance()) - , m_bInTransaction(false) - , m_connection(nullptr) + : prepare (this) + , m_instance (SharedSingleton ::getInstance ()) + , m_bInTransaction (false) + , m_connection (nullptr) { } session::session(const session& deferredClone) - : prepare(this) - , m_instance(Sqlite3::getInstance()) - , m_bInTransaction(false) - , m_connection(nullptr) - , m_fileName(deferredClone.m_fileName) - , m_connectString(deferredClone.m_connectString) + : prepare (this) + , m_instance (SharedSingleton ::getInstance ()) + , m_bInTransaction (false) + , m_connection (nullptr) + , m_fileName (deferredClone.m_fileName) + , m_connectString (deferredClone.m_connectString) { // shouldn't be needed since deferredClone did it //Sqlite::initialize();