Revert (but cleanup) SingletonHolder implementation

MSVC 2017 15.7 contains a bug where constant initialization no longer
takes place for objects with constructors marked constexpr such as
once_flag. This leaves once_flag susceptible to dynamic initialization
order problems and causes some singleton instances to be constructed
incorrectly. We put back our correct implementation (with some cleanup)
and await a fix from MS.
Refs #0

Framework/Kernel/inc/MantidKernel/SingletonHolder.h

@@ -26,16 +26,17 @@
 #include <cassert>
 #include <cstdlib>
 #include <functional>
+#include <mutex>
 
 namespace Mantid {
 namespace Kernel {
 
 /// Type of deleter function
-using deleter_t = std::function<void()>;
+using SingletonDeleterFn = std::function<void()>;
 
 /// Register the given deleter function to be called
 /// at exit
-extern MANTID_KERNEL_DLL void deleteOnExit(deleter_t);
+MANTID_KERNEL_DLL void deleteOnExit(SingletonDeleterFn func);
 
 /// Manage the lifetime of a class intended to be a singleton
 template <typename T> class SingletonHolder {
@@ -45,8 +46,22 @@ public:
   SingletonHolder() = delete;
 
   static T &Instance();
+
+private:
+  static T *instance;
+  static std::once_flag once;
+#ifndef NDEBUG
+  static bool destroyed;
+#endif
 };
 
+// Static field initializers
+template <typename T> T *SingletonHolder<T>::instance = nullptr;
+template <typename T> std::once_flag SingletonHolder<T>::once;
+#ifndef NDEBUG
+template <typename T> bool SingletonHolder<T>::destroyed = false;
+#endif
+
 /// Policy class controlling creation of the singleton
 /// Implementation classes should mark their default
 /// constructors private and insert a friend declaration
@@ -67,25 +82,18 @@ template <typename T> struct CreateUsingNew {
 /// already exist
 /// Creation is done using the CreateUsingNew policy at the moment
 template <typename T> inline T &SingletonHolder<T>::Instance() {
-// 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.
-
-#ifndef NDEBUG
-  static bool destroyed(false);
-#endif
-  static T *instance = [] {
-    auto *singleton = CreateUsingNew<T>::create();
-    deleteOnExit(deleter_t([singleton]() {
+  std::call_once(once, []() {
+    instance = CreateUsingNew<T>::create();
+    deleteOnExit(SingletonDeleterFn([]() {
 #ifndef NDEBUG
       destroyed = true;
 #endif
-      CreateUsingNew<T>::destroy(singleton);
+      CreateUsingNew<T>::destroy(instance);
     }));
-    return singleton;
-  }();
+  });
+#ifndef NDEBUG
   assert(!destroyed);
+#endif
   return *instance;
 }
 

Framework/Kernel/src/SingletonHolder.cpp

@@ -6,29 +6,36 @@ namespace Kernel {
 
 namespace {
 /// List of functions to call on program exit
-std::list<deleter_t> DELETERS;
-} // namespace
+using CleanupList = std::list<SingletonDeleterFn>;
+
+CleanupList &cleanupList() {
+  static CleanupList cleanup;
+  return cleanup;
+}
 
 /// 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() {
-  for (auto &deleter : DELETERS) {
+void cleanupSingletons() {
+  auto &deleters = cleanupList();
+  for (auto &deleter : deleters) {
     deleter();
   }
-  DELETERS.clear();
+  deleters.clear();
 }
+} // namespace
 
 /// 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 deleteOnExit(deleter_t func) {
-  if (DELETERS.empty()) {
+void deleteOnExit(SingletonDeleterFn func) {
+  auto &deleters = cleanupList();
+  if (deleters.empty()) {
     atexit(&cleanupSingletons);
   }
-  DELETERS.push_front(func);
+  deleters.push_front(func);
 }
 } // namespace Kernel
 } // namespace Mantid