Fix singleton::is_destroyed and add comments

- A subclass of T is need to correctly track the lifetime of the
  singleton, so is_destroyed works reliably.
- singleton<T> ctor is made protected so it cannot be created accidentally
- Existing comments (mostly typos) are fixed
- Additional comments are added detailing the usage and design choices
  made for the singleton to avoid people accidentally breaking it (again)

Fixes boostorg#104

include/boost/serialization/singleton.hpp

@@ -58,7 +58,7 @@ namespace serialization {
 // details.
 //
 
-// singletons created by this code are guarenteed to be unique
+// Singletons created by this code are guaranteed to be unique
 // within the executable or shared library which creates them.
 // This is sufficient and in fact ideal for the serialization library.
 // The singleton is created when the module is loaded and destroyed
@@ -74,14 +74,21 @@ namespace serialization {
 // Second, it provides a mechanism to detect when a non-const function
 // is called after initialization.
 
-// make a singleton to lock/unlock all singletons for alteration.
+// Make a singleton to lock/unlock all singletons for alteration.
 // The intent is that all singletons created/used by this code
 // are to be initialized before main is called. A test program
-// can lock all the singletons when main is entereed.  This any
-// attempt to retieve a mutable instances while locked will
-// generate a assertion if compiled for debug.
-
-// note usage of BOOST_DLLEXPORT.  These functions are in danger of
+// can lock all the singletons when main is entered.  Thus any
+// attempt to retrieve a mutable instance while locked will
+// generate an assertion if compiled for debug.
+
+// The singleton template can be used in 2 ways:
+// 1 (Recommended): Publicly inherit your type T from singleton<T>,
+// make its ctor protected and access it via T::get_const_instance()
+// 2: Simply access singleton<T> without changing T. Note that this only
+// provides a global instance accesible by singleton<T>::get_const_instance()
+// To prevent using multiple instances of T make its ctor protected
+
+// Note on usage of BOOST_DLLEXPORT: These functions are in danger of
 // being eliminated by the optimizer when building an application in
 // release mode. Usage of the macro is meant to signal the compiler/linker
 // to avoid dropping these functions which seem to be unreferenced.
@@ -113,34 +120,69 @@ static inline singleton_module & get_singleton_module(){
     return m;
 }
 
+namespace detail {
+
+// This is the class actually instantiated and hence the real singleton.
+// So there will only be one instance of this class. This does not hold
+// for singleton<T> as a class derived from singleton<T> could be
+// instantiated multiple times.
+// It also provides a flag `is_destroyed` which returns true, when the
+// class was destructed. It is static and hence accesible even after
+// destruction. This can be used to check, if the singleton is still
+// accesible e.g. in destructors of other singletons.
+template<class T>
+class singleton_wrapper : public T
+{
+    static bool & get_is_destroyed(){
+        // Prefer a static function member to avoid LNK1179.
+        // Note: As this is for a singleton (1 instance only) it must be set
+        // never be reset (to false)!
+        static bool is_destroyed_flag = false;
+        return is_destroyed_flag;
+    }
+public:
+    singleton_wrapper(){
+        BOOST_ASSERT(! is_destroyed());
+    }
+    ~singleton_wrapper(){
+        get_is_destroyed() = true;
+    }
+    static bool is_destroyed(){
+        return get_is_destroyed();
+    }
+};
+
+} // detail
+
 template <class T>
 class singleton {
 private:
-    // note presumption that T has a default constructor
     static T & m_instance;
     // include this to provoke instantiation at pre-execution time
     static void use(T const &) {}
     static T & get_instance() {
-        static T t;
-
-        // refer to instance, causing it to be instantiated (and
-        // initialized at startup on working compilers)
         BOOST_ASSERT(! is_destroyed());
 
+        // use a wrapper so that types T with protected constructors can be used
+        // Using a static function member avoids LNK1179
+        static detail::singleton_wrapper< T > t;
+
         // note that the following is absolutely essential.
         // commenting out this statement will cause compilers to fail to
         // construct the instance at pre-execution time.  This would prevent
         // our usage/implementation of "locking" and introduce uncertainty into
-        // the sequence of object initializaition.
+        // the sequence of object initialization.
         use(m_instance);
 
         return static_cast<T &>(t);
     }
 
-    static bool & get_is_destroyed(){
-        static bool is_destroyed;
-        return is_destroyed;
-    }
+
+protected:
+    // Do not allow instantiation of a singleton<T>. But we want to allow
+    // `class T: public singleton<T>` so we can't delete this ctor
+    BOOST_DLLEXPORT singleton(){}
+
 public:
     BOOST_DLLEXPORT static T & get_mutable_instance(){
         BOOST_ASSERT(! get_singleton_module().is_locked());
@@ -150,16 +192,12 @@ class singleton {
         return get_instance();
     }
     BOOST_DLLEXPORT static bool is_destroyed(){
-        return get_is_destroyed();
-    }
-    BOOST_DLLEXPORT singleton(){
-        get_is_destroyed() = false;
-    }
-    BOOST_DLLEXPORT ~singleton() {
-        get_is_destroyed() = true;
+        return detail::singleton_wrapper< T >::is_destroyed();
     }
 };
 
+// Assigning the instance reference to a static member forces initialization
+// at startup time as described in http://tinyurl.com/ljdp8
 template<class T>
 T & singleton< T >::m_instance = singleton< T >::get_instance();