[Libreoffice-commits] core.git: sc/inc sc/source
Luboš Luňák
l.lunak at collabora.com
Mon Jun 18 08:56:58 UTC 2018
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.lunak at 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.meeks at 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);
More information about the Libreoffice-commits
mailing list