use std::atomic rather than OSL_DOUBLE_CHECKED_LOCKING_MEMORY_BARRIER

Sources such as
http://preshing.com/20130930/double-checked-locking-is-fixed-in-cpp11/
or https://en.wikipedia.org/wiki/Double-checked_locking suggest that
it wasn't possible to reliably do a portable double-checked initialization
before C++11. It may be true that for all platforms we support those
memory barriers are in fact not needed (which seems to be the assumption
behind OSL_DOUBLE_CHECKED_LOCKING_MEMORY_BARRIER being empty), and
looking at the generated assembly here on x86-64 seems to confirm that, but
in the worst case then this is a more chatty and standard way of writing
a no-op.

I don't want to use threadsafe statics or std::call_once() because
ScGlobal::Clear() does cleanup, which would be non-trivial to do with these,
and also some functions may not necessarily always force
creation of the singleton when touching the pointer, so it can't be easily
hidden behind a single function call.

The need to explicitly use load() with delete (thus preventing DELETEZ)
looks like a Clang bug to me.

Change-Id: Id3b0ef4b273ed25a5c154f90cde090ea1f9674fb
Reviewed-on: https://gerrit.libreoffice.org/55851
Tested-by: Jenkins
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/56141
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
Tested-by: Luboš Luňák <l.lunak@collabora.com>

2 files changed, 29 insertions, 30 deletions

diff --git a/sc/inc/global.hxx b/sc/inc/global.hxx
index bb63731c1a12..b9ef661f43db 100644
--- a/sc/inc/global.hxx
+++ b/sc/inc/global.hxx
@@ -28,6 +28,10 @@
 #include "scdllapi.h"
 #include <rtl/ustring.hxx>
 
+#include <atomic>
+// HACK: <atomic> includes <stdbool.h>, which in some Clang versions does '#define bool bool',
+// which confuses clang plugins.
+#undef bool
 #include <map>
 #include <vector>
 
@@ -501,8 +505,8 @@ class ScGlobal
 {
     static SvxSearchItem*   pSearchItem;
     static ScAutoFormat*    pAutoFormat;
-    static LegacyFuncCollection* pLegacyFuncCollection;
-    static ScUnoAddInCollection* pAddInCollection;
+    static std::atomic<LegacyFuncCollection*> pLegacyFuncCollection;
+    static std::atomic<ScUnoAddInCollection*> pAddInCollection;
     static ScUserList*      pUserList;
     static std::map<const char*, OUString>* pRscString;
     static OUString*        pStrScDoc;
@@ -521,12 +525,12 @@ class ScGlobal
 
     static css::uno::Reference< css::i18n::XOrdinalSuffix> xOrdinalSuffix;
     static CalendarWrapper*     pCalendar;
-    static CollatorWrapper*     pCaseCollator;
-    static CollatorWrapper*     pCollator;
-    static ::utl::TransliterationWrapper* pTransliteration;
-    static ::utl::TransliterationWrapper* pCaseTransliteration;
+    static std::atomic<CollatorWrapper*>     pCaseCollator;
+    static std::atomic<CollatorWrapper*>     pCollator;
+    static std::atomic<::utl::TransliterationWrapper*> pTransliteration;
+    static std::atomic<::utl::TransliterationWrapper*> pCaseTransliteration;
     static IntlWrapper*         pScIntlWrapper;
-    static css::lang::Locale*   pLocale;
+    static std::atomic<css::lang::Locale*>   pLocale;
 
     static ScFieldEditEngine*   pFieldEditEngine;
 
diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index 3f33f97cf685..01b770415b03 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -83,19 +83,19 @@
 tools::SvRef<ScDocShell>  ScGlobal::xDrawClipDocShellRef;
 SvxSearchItem*  ScGlobal::pSearchItem = nullptr;
 ScAutoFormat*   ScGlobal::pAutoFormat = nullptr;
-LegacyFuncCollection* ScGlobal::pLegacyFuncCollection = nullptr;
-ScUnoAddInCollection* ScGlobal::pAddInCollection = nullptr;
+std::atomic<LegacyFuncCollection*> ScGlobal::pLegacyFuncCollection(nullptr);
+std::atomic<ScUnoAddInCollection*> ScGlobal::pAddInCollection(nullptr);
 ScUserList*     ScGlobal::pUserList = nullptr;
 LanguageType    ScGlobal::eLnge = LANGUAGE_SYSTEM;
-css::lang::Locale*     ScGlobal::pLocale = nullptr;
+std::atomic<css::lang::Locale*> ScGlobal::pLocale(nullptr);
 SvtSysLocale*   ScGlobal::pSysLocale = nullptr;
 const CharClass*  ScGlobal::pCharClass = nullptr;
 const LocaleDataWrapper*  ScGlobal::pLocaleData = nullptr;
 CalendarWrapper* ScGlobal::pCalendar = nullptr;
-CollatorWrapper* ScGlobal::pCollator = nullptr;
-CollatorWrapper* ScGlobal::pCaseCollator = nullptr;
-::utl::TransliterationWrapper* ScGlobal::pTransliteration = nullptr;
-::utl::TransliterationWrapper* ScGlobal::pCaseTransliteration = nullptr;
+std::atomic<CollatorWrapper*> ScGlobal::pCollator(nullptr);
+std::atomic<CollatorWrapper*> ScGlobal::pCaseCollator(nullptr);
+std::atomic<::utl::TransliterationWrapper*> ScGlobal::pTransliteration(nullptr);
+std::atomic<::utl::TransliterationWrapper*> ScGlobal::pCaseTransliteration(nullptr);
 css::uno::Reference< css::i18n::XOrdinalSuffix> ScGlobal::xOrdinalSuffix = nullptr;
 sal_Unicode     ScGlobal::cListDelimiter = ',';
 OUString*       ScGlobal::pEmptyOUString = nullptr;
@@ -133,24 +133,19 @@ bool ScGlobal::bThreadedGroupCalcInProgress = false;
 template< typename Type, typename Function = std::function< Type*() >,
           typename Guard = osl::MutexGuard, typename GuardCtor = osl::GetGlobalMutex >
 static inline
-Type* doubleCheckedInit( Type*& pointer, Function function, GuardCtor guardCtor = osl::GetGlobalMutex())
+Type* doubleCheckedInit( std::atomic<Type*>& pointer, Function function, GuardCtor guardCtor = osl::GetGlobalMutex())
 {
-    Type* p = pointer;
+    Type* p = pointer.load( std::memory_order_acquire );
     if (!p)
     {
         Guard guard(guardCtor());
-        p = pointer;
+        p = pointer.load( std::memory_order_relaxed );
         if (!p)
         {
             p = function();
-            OSL_DOUBLE_CHECKED_LOCKING_MEMORY_BARRIER();
-            pointer = p;
+            pointer.store( p, std::memory_order_release );
         }
     }
-    else
-    {
-        OSL_DOUBLE_CHECKED_LOCKING_MEMORY_BARRIER();
-    }
     return p;
 }
 
@@ -575,8 +570,8 @@ void ScGlobal::Clear()
     ExitExternalFunc();
     ClearAutoFormat();
     DELETEZ(pSearchItem);
-    DELETEZ(pLegacyFuncCollection);
-    DELETEZ(pAddInCollection);
+    delete pLegacyFuncCollection.load(); pLegacyFuncCollection = nullptr;
+    delete pAddInCollection.load(); pAddInCollection = nullptr;
     DELETEZ(pUserList);
     DELETEZ(pStarCalcFunctionList); // Destroy before ResMgr!
     DELETEZ(pStarCalcFunctionMgr);
@@ -588,17 +583,17 @@ void ScGlobal::Clear()
     DELETEZ(pButtonBrushItem);
     DELETEZ(pEmbeddedBrushItem);
     DELETEZ(pEnglishFormatter);
-    DELETEZ(pCaseTransliteration);
-    DELETEZ(pTransliteration);
-    DELETEZ(pCaseCollator);
-    DELETEZ(pCollator);
+    delete pCaseTransliteration.load(); pCaseTransliteration = nullptr;
+    delete pTransliteration.load(); pTransliteration = nullptr;
+    delete pCaseCollator.load(); pCaseCollator = nullptr;
+    delete pCollator.load(); pCollator = nullptr;
     DELETEZ(pCalendar);
     // Do NOT delete pCharClass since it is a pointer to the single SvtSysLocale instance !
     pCharClass = nullptr;
     // Do NOT delete pLocaleData since it is a pointer to the single SvtSysLocale instance !
     pLocaleData = nullptr;
     DELETEZ(pSysLocale);
-    DELETEZ(pLocale);
+    delete pLocale.load(); pLocale = nullptr;
     DELETEZ(pStrClipDocName);
 
     DELETEZ(pUnitConverter);