sc/inc/global.hxx | 18 +++++++++++------- sc/source/core/data/global.cxx | 41 ++++++++++++++++++----------------------- 2 files changed, 29 insertions(+), 30 deletions(-)
New commits: commit f6bd95704e46f53fd976519cc7d916f28a4ddc94 Author: Luboš Luňák <l.lu...@collabora.com> Date: Fri Jun 15 11:33:39 2018 +0200 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.me...@collabora.com> diff --git a/sc/inc/global.hxx b/sc/inc/global.hxx index 56fe91be4aca..bdef61bf5c39 100644 --- a/sc/inc/global.hxx +++ b/sc/inc/global.hxx @@ -29,6 +29,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> class SfxItemSet; @@ -496,8 +500,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; @@ -516,12 +520,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 775db26946fb..5f5fe519c2c6 100644 --- a/sc/source/core/data/global.cxx +++ b/sc/source/core/data/global.cxx @@ -82,19 +82,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; @@ -132,24 +132,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; } @@ -574,8 +569,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); @@ -587,17 +582,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); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits