mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

use std::atomic rather than OSL_DOUBLE_CHECKED_LOCKING_MEMORY_BARRIER

Browse Source 
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>
This commit is contained in:
Luboš Luňák
2018-06-15 11:33:39 +02:00
committed by Michael Meeks
parent fbe6fa330f
commit f6bd95704e
2 changed files with 29 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
sc/inc/global.hxx
View File

@@ -29,6 +29,10 @@
#include "scdllapi.h" #include "scdllapi.h"
#include <rtl/ustring.hxx> #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 <map>
class SfxItemSet; class SfxItemSet;
@@ -496,8 +500,8 @@ class ScGlobal
{ {
static SvxSearchItem* pSearchItem; static SvxSearchItem* pSearchItem;
static ScAutoFormat* pAutoFormat; static ScAutoFormat* pAutoFormat;
static LegacyFuncCollection* pLegacyFuncCollection; static std::atomic<LegacyFuncCollection*> pLegacyFuncCollection;
static ScUnoAddInCollection* pAddInCollection; static std::atomic<ScUnoAddInCollection*> pAddInCollection;
static ScUserList* pUserList; static ScUserList* pUserList;
static std::map<const char*, OUString>* pRscString; static std::map<const char*, OUString>* pRscString;
static OUString* pStrScDoc; static OUString* pStrScDoc;
@@ -516,12 +520,12 @@ class ScGlobal
static css::uno::Reference< css::i18n::XOrdinalSuffix> xOrdinalSuffix; static css::uno::Reference< css::i18n::XOrdinalSuffix> xOrdinalSuffix;
static CalendarWrapper* pCalendar; static CalendarWrapper* pCalendar;
static CollatorWrapper* pCaseCollator; static std::atomic<CollatorWrapper*> pCaseCollator;
static CollatorWrapper* pCollator; static std::atomic<CollatorWrapper*> pCollator;
static ::utl::TransliterationWrapper* pTransliteration; static std::atomic<::utl::TransliterationWrapper*> pTransliteration;
static ::utl::TransliterationWrapper* pCaseTransliteration; static std::atomic<::utl::TransliterationWrapper*> pCaseTransliteration;
static IntlWrapper* pScIntlWrapper; static IntlWrapper* pScIntlWrapper;
static css::lang::Locale* pLocale; static std::atomic<css::lang::Locale*> pLocale;
static ScFieldEditEngine* pFieldEditEngine; static ScFieldEditEngine* pFieldEditEngine;

41
sc/source/core/data/global.cxx
View File

@@ -82,19 +82,19 @@
tools::SvRef<ScDocShell> ScGlobal::xDrawClipDocShellRef; tools::SvRef<ScDocShell> ScGlobal::xDrawClipDocShellRef;
SvxSearchItem* ScGlobal::pSearchItem = nullptr; SvxSearchItem* ScGlobal::pSearchItem = nullptr;
ScAutoFormat* ScGlobal::pAutoFormat = nullptr; ScAutoFormat* ScGlobal::pAutoFormat = nullptr;
LegacyFuncCollection* ScGlobal::pLegacyFuncCollection = nullptr; std::atomic<LegacyFuncCollection*> ScGlobal::pLegacyFuncCollection(nullptr);
ScUnoAddInCollection* ScGlobal::pAddInCollection = nullptr; std::atomic<ScUnoAddInCollection*> ScGlobal::pAddInCollection(nullptr);
ScUserList* ScGlobal::pUserList = nullptr; ScUserList* ScGlobal::pUserList = nullptr;
LanguageType ScGlobal::eLnge = LANGUAGE_SYSTEM; LanguageType ScGlobal::eLnge = LANGUAGE_SYSTEM;
css::lang::Locale* ScGlobal::pLocale = nullptr; std::atomic<css::lang::Locale*> ScGlobal::pLocale(nullptr);
SvtSysLocale* ScGlobal::pSysLocale = nullptr; SvtSysLocale* ScGlobal::pSysLocale = nullptr;
const CharClass* ScGlobal::pCharClass = nullptr; const CharClass* ScGlobal::pCharClass = nullptr;
const LocaleDataWrapper* ScGlobal::pLocaleData = nullptr; const LocaleDataWrapper* ScGlobal::pLocaleData = nullptr;
CalendarWrapper* ScGlobal::pCalendar = nullptr; CalendarWrapper* ScGlobal::pCalendar = nullptr;
CollatorWrapper* ScGlobal::pCollator = nullptr; std::atomic<CollatorWrapper*> ScGlobal::pCollator(nullptr);
CollatorWrapper* ScGlobal::pCaseCollator = nullptr; std::atomic<CollatorWrapper*> ScGlobal::pCaseCollator(nullptr);
::utl::TransliterationWrapper* ScGlobal::pTransliteration = nullptr; std::atomic<::utl::TransliterationWrapper*> ScGlobal::pTransliteration(nullptr);
::utl::TransliterationWrapper* ScGlobal::pCaseTransliteration = nullptr; std::atomic<::utl::TransliterationWrapper*> ScGlobal::pCaseTransliteration(nullptr);
css::uno::Reference< css::i18n::XOrdinalSuffix> ScGlobal::xOrdinalSuffix = nullptr; css::uno::Reference< css::i18n::XOrdinalSuffix> ScGlobal::xOrdinalSuffix = nullptr;
sal_Unicode ScGlobal::cListDelimiter = ','; sal_Unicode ScGlobal::cListDelimiter = ',';
OUString* ScGlobal::pEmptyOUString = nullptr; OUString* ScGlobal::pEmptyOUString = nullptr;
@@ -132,24 +132,19 @@ bool ScGlobal::bThreadedGroupCalcInProgress = false;
template< typename Type, typename Function = std::function< Type*() >, template< typename Type, typename Function = std::function< Type*() >,
typename Guard = osl::MutexGuard, typename GuardCtor = osl::GetGlobalMutex > typename Guard = osl::MutexGuard, typename GuardCtor = osl::GetGlobalMutex >
static inline 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) if (!p)
{ {
Guard guard(guardCtor()); Guard guard(guardCtor());
p = pointer; p = pointer.load( std::memory_order_relaxed );
if (!p) if (!p)
{ {
p = function(); p = function();
OSL_DOUBLE_CHECKED_LOCKING_MEMORY_BARRIER(); pointer.store( p, std::memory_order_release );
pointer = p;
} }
} }
else
{
OSL_DOUBLE_CHECKED_LOCKING_MEMORY_BARRIER();
}
return p; return p;
} }
@@ -574,8 +569,8 @@ void ScGlobal::Clear()
ExitExternalFunc(); ExitExternalFunc();
ClearAutoFormat(); ClearAutoFormat();
DELETEZ(pSearchItem); DELETEZ(pSearchItem);
DELETEZ(pLegacyFuncCollection); delete pLegacyFuncCollection.load(); pLegacyFuncCollection = nullptr;
DELETEZ(pAddInCollection); delete pAddInCollection.load(); pAddInCollection = nullptr;
DELETEZ(pUserList); DELETEZ(pUserList);
DELETEZ(pStarCalcFunctionList); // Destroy before ResMgr! DELETEZ(pStarCalcFunctionList); // Destroy before ResMgr!
DELETEZ(pStarCalcFunctionMgr); DELETEZ(pStarCalcFunctionMgr);
@@ -587,17 +582,17 @@ void ScGlobal::Clear()
DELETEZ(pButtonBrushItem); DELETEZ(pButtonBrushItem);
DELETEZ(pEmbeddedBrushItem); DELETEZ(pEmbeddedBrushItem);
DELETEZ(pEnglishFormatter); DELETEZ(pEnglishFormatter);
DELETEZ(pCaseTransliteration); delete pCaseTransliteration.load(); pCaseTransliteration = nullptr;
DELETEZ(pTransliteration); delete pTransliteration.load(); pTransliteration = nullptr;
DELETEZ(pCaseCollator); delete pCaseCollator.load(); pCaseCollator = nullptr;
DELETEZ(pCollator); delete pCollator.load(); pCollator = nullptr;
DELETEZ(pCalendar); DELETEZ(pCalendar);
// Do NOT delete pCharClass since it is a pointer to the single SvtSysLocale instance ! // Do NOT delete pCharClass since it is a pointer to the single SvtSysLocale instance !
pCharClass = nullptr; pCharClass = nullptr;
// Do NOT delete pLocaleData since it is a pointer to the single SvtSysLocale instance ! // Do NOT delete pLocaleData since it is a pointer to the single SvtSysLocale instance !
pLocaleData = nullptr; pLocaleData = nullptr;
DELETEZ(pSysLocale); DELETEZ(pSysLocale);
DELETEZ(pLocale); delete pLocale.load(); pLocale = nullptr;
DELETEZ(pStrClipDocName); DELETEZ(pStrClipDocName);
DELETEZ(pUnitConverter); DELETEZ(pUnitConverter);