Remove static initialization order dependency with SingletonHolder

Something changed in VS2017 such that SingletonHolder::once_flag
was not always initialized before SingletonHolder::instance and
this caused multiple singleton instances to be created. In some
ways this was a timebomb as static initialization order is
undefined.

Framework/Kernel/inc/MantidKernel/SingletonHolder.h

-#ifndef SINGLETON_HOLDER_H
-#define SINGLETON_HOLDER_H
+#ifndef MANTID_KERNEL_SINGLETON_HOLDER_H
+#define MANTID_KERNEL_SINGLETON_HOLDER_H
 
 ////////////////////////////////////////////////////////////////////////////////
 // The Loki Library
@@ -25,83 +25,66 @@
 #include <MantidKernel/DllConfig.h>
 #include <cassert>
 #include <cstdlib>
-#include <mutex>
-#include <stdexcept>
-#include <string>
-#include <typeinfo>
+#include <functional>
 
 namespace Mantid {
 namespace Kernel {
 
-/// prototype for function passed to atexit()
-using atexit_func_t = void (*)();
+/// Type of deleter function
+using deleter_t = std::function<void()>;
 
-extern MANTID_KERNEL_DLL void CleanupSingletons();
-extern MANTID_KERNEL_DLL void AddSingleton(atexit_func_t func);
+/// Register the given deleter function to be called
+/// at exit
+extern MANTID_KERNEL_DLL void deleteOnExit(deleter_t);
 
-/// class to manage an instance of an object as a singleton
+/// Manage the lifetime of a class intended to be a singleton
 template <typename T> class SingletonHolder {
 public:
-  /// Allow users to access to the type returned by Instance()
   using HeldType = T;
 
-  static T &Instance();
-
-private:
-  static void DestroySingleton();
-  /// default constructor marked private so only access is via the Instance()
-  /// method
-  SingletonHolder();
+  SingletonHolder() = delete;
 
-  static T *pInstance;
-  static std::once_flag flag;
-  static bool destroyed;
+  static T &Instance();
 };
 
-/// Implementation of the SingletonHolder create policy using the new and delete
-/// operators
+/// Policy class controlling creation of the singleton
+/// Implementation classes should mark their default
+/// constructors private and insert a friend declaration
+/// for this class, e.g.:
+///
+///   friend struct Mantid::Kernel::CreateUsingNew<SingletonImplClass>;
+/// 
 template <typename T> struct CreateUsingNew {
   /// create an object using the new operator
   /// @returns New instance
-  static T *Create() { return new T; }
+  static T *create() { return new T; }
   /// delete an object instantiated using Create
   /// @param p :: pointer to instance to destroy
-  static void Destroy(T *p) { delete p; }
+  static void destroy(T *p) { delete p; }
 };
 
 /// Return a reference to the Singleton instance, creating it if it does not
 /// already exist
 /// Creation is done using the CreateUsingNew policy at the moment
 template <typename T> inline T &SingletonHolder<T>::Instance() {
-  if (destroyed) {
-    std::string s("Attempt to use destroyed singleton ");
-    s += typeid(T).name();
-    throw std::runtime_error(s.c_str());
-  }
-  std::call_once(flag, [] {
-    pInstance = CreateUsingNew<T>::Create();
-    AddSingleton(&DestroySingleton);
-  });
-  return *pInstance;
-}
-
-/// Destroy the singleton
-template <typename T> void SingletonHolder<T>::DestroySingleton() {
-  // std::cerr << "destroying singleton " << typeid(T).name() << '\n';
+  // Local static instance initialized by an immediately invoked lambda.
+  // A second nested lambda captures the singleton instance pointer
+  // and forms the deleter function that is registered with
+  // the atexit deleters.
+  static bool destroyed(false);
+  static T *instance = [] {
+    auto *singleton = CreateUsingNew<T>::create();
+    deleteOnExit(deleter_t([singleton]() {
+      destroyed = true;
+      CreateUsingNew<T>::destroy(singleton);
+    }));
+    return singleton;
+  }();
   assert(!destroyed);
-  CreateUsingNew<T>::Destroy(pInstance);
-  pInstance = nullptr;
-  destroyed = true;
+  return *instance;
 }
 
-/// global variable holding pointer to singleton instance
-template <typename T> T *SingletonHolder<T>::pInstance = nullptr;
-
-template <typename T> std::once_flag SingletonHolder<T>::flag;
-
-/// variable to allow trapping of attempts to destroy a singleton more than once
-template <typename T> bool SingletonHolder<T>::destroyed = false;
-}
-}
+} // namespace Kernel
+} // namespace Mantid
 
 #endif // SINGLETON_HOLDER_H

Framework/Kernel/src/SingletonHolder.cpp

-#include <list>
 #include <MantidKernel/SingletonHolder.h>
+#include <list>
 
 namespace Mantid {
 namespace Kernel {
 
+namespace {
 /// List of functions to call on program exit
-static std::list<atexit_func_t> *cleanup_list = nullptr;
+std::list<deleter_t> DELETERS;
+} // namespace
 
-/// Function registed to atexit() that will clean up
-/// all our singletons
+/// Intended to be registered to atexit() that will clean up
+/// all registered singletons
 /// This function may be registed with atexit() more than once, so it needs to
 /// clear the list once it has called all the functions
-MANTID_KERNEL_DLL void CleanupSingletons() {
-  if (cleanup_list == nullptr) {
-    return;
-  }
-  std::list<atexit_func_t>::const_iterator it;
-  for (it = cleanup_list->begin(); it != cleanup_list->end(); ++it) {
-    (*(*it))();
+MANTID_KERNEL_DLL void cleanupSingletons() {
+  for (auto &deleter : DELETERS) {
+    deleter();
   }
-  delete cleanup_list;
-  cleanup_list = nullptr;
+  DELETERS.clear();
 }
 
 /// Adds singleton cleanup function to our atexit list
 /// functions are added to the start of the list so on deletion it is last in,
 /// first out
 /// @param func :: Exit function to call - the singleton destructor function
-MANTID_KERNEL_DLL void AddSingleton(atexit_func_t func) {
-  if (cleanup_list == nullptr) {
-    cleanup_list = new std::list<atexit_func_t>;
-    atexit(&CleanupSingletons);
+MANTID_KERNEL_DLL void deleteOnExit(deleter_t func) {
+  if (DELETERS.empty()) {
+    atexit(&cleanupSingletons);
   }
-  cleanup_list->push_front(func);
-}
-}
+  DELETERS.push_front(func);
 }
+} // namespace Kernel
+} // namespace Mantid