[Libreoffice-commits] core.git: Branch 'libreoffice-6-1' - 2 commits - sc/inc sc/source

Luboš Luňák l.lunak at collabora.com
Mon Jun 4 14:13:10 UTC 2018


 sc/inc/document.hxx                 |   12 ++++++------
 sc/inc/global.hxx                   |    2 ++
 sc/source/core/data/column2.cxx     |    2 +-
 sc/source/core/data/documen2.cxx    |   11 +++++------
 sc/source/core/data/documen8.cxx    |    4 ++--
 sc/source/core/data/document.cxx    |    2 +-
 sc/source/core/data/formulacell.cxx |   14 +++++++-------
 sc/source/core/data/global.cxx      |   21 +++++++++++++++++++++
 8 files changed, 45 insertions(+), 23 deletions(-)

New commits:
commit d26bad04ebc7f23523b4ac37c7a1046023b475ef
Author: Luboš Luňák <l.lunak at collabora.com>
Date:   Fri May 25 12:29:12 2018 +0200

    assert(!bThreadedGroupCalcInProgress) in ScGlobal get-or-create functions
    
    Similarly to ScGlobal::Get(Case)Collator() these could possibly lead
    to a race condition of several singleton creations at the same time. That
    doesn't seem to be the case after few tests, but better check.
    
    Change-Id: I1edb613b5e034fcc952a43afc91f1d7288028861
    Reviewed-on: https://gerrit.libreoffice.org/54797
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/55278
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
    Tested-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index 531619a101a5..0ac839200cb3 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -205,6 +205,7 @@ sal_uInt16 ScGlobal::GetStandardRowHeight()
 
 SvNumberFormatter* ScGlobal::GetEnglishFormatter()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pEnglishFormatter )
     {
         pEnglishFormatter = new SvNumberFormatter(
@@ -251,6 +252,7 @@ bool ScGlobal::CheckWidthInvalidate( bool& bNumFormatChanged,
 
 const SvxSearchItem& ScGlobal::GetSearchItem()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if (!pSearchItem)
     {
         pSearchItem = new SvxSearchItem( SID_SEARCH_ITEM );
@@ -261,6 +263,7 @@ const SvxSearchItem& ScGlobal::GetSearchItem()
 
 void ScGlobal::SetSearchItem( const SvxSearchItem& rNew )
 {
+    assert(!bThreadedGroupCalcInProgress);
     // FIXME: An assignment operator would be nice here
     delete pSearchItem;
     pSearchItem = static_cast<SvxSearchItem*>(rNew.Clone());
@@ -271,6 +274,7 @@ void ScGlobal::SetSearchItem( const SvxSearchItem& rNew )
 
 void ScGlobal::ClearAutoFormat()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if (pAutoFormat)
     {
         //  When modified via StarOne then only the SaveLater flag is set and no saving is done.
@@ -289,6 +293,7 @@ ScAutoFormat* ScGlobal::GetAutoFormat()
 
 ScAutoFormat* ScGlobal::GetOrCreateAutoFormat()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pAutoFormat )
     {
         pAutoFormat = new ScAutoFormat;
@@ -300,6 +305,7 @@ ScAutoFormat* ScGlobal::GetOrCreateAutoFormat()
 
 LegacyFuncCollection* ScGlobal::GetLegacyFuncCollection()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if (!pLegacyFuncCollection)
         pLegacyFuncCollection = new LegacyFuncCollection();
     return pLegacyFuncCollection;
@@ -307,6 +313,7 @@ LegacyFuncCollection* ScGlobal::GetLegacyFuncCollection()
 
 ScUnoAddInCollection* ScGlobal::GetAddInCollection()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if (!pAddInCollection)
         pAddInCollection = new ScUnoAddInCollection();
     return pAddInCollection;
@@ -314,6 +321,7 @@ ScUnoAddInCollection* ScGlobal::GetAddInCollection()
 
 ScUserList* ScGlobal::GetUserList()
 {
+    assert(!bThreadedGroupCalcInProgress);
     // Hack: Load Cfg item at the App
     global_InitAppOptions();
 
@@ -324,6 +332,7 @@ ScUserList* ScGlobal::GetUserList()
 
 void ScGlobal::SetUserList( const ScUserList* pNewList )
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( pNewList )
     {
         if ( !pUserList )
@@ -459,6 +468,7 @@ OUString ScGlobal::GetLongErrorString(FormulaError nErr)
 
 SvxBrushItem* ScGlobal::GetButtonBrushItem()
 {
+    assert(!bThreadedGroupCalcInProgress);
     pButtonBrushItem->SetColor( Application::GetSettings().GetStyleSettings().GetFaceColor() );
     return pButtonBrushItem;
 }
@@ -517,6 +527,7 @@ const OUString& ScGlobal::GetClipDocName()
 
 void ScGlobal::SetClipDocName( const OUString& rNew )
 {
+    assert(!bThreadedGroupCalcInProgress);
     *pStrClipDocName = rNew;
 }
 
@@ -658,6 +669,7 @@ bool ScGlobal::HasStarCalcFunctionList()
 
 ScFunctionList* ScGlobal::GetStarCalcFunctionList()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pStarCalcFunctionList )
         pStarCalcFunctionList = new ScFunctionList;
 
@@ -666,6 +678,7 @@ ScFunctionList* ScGlobal::GetStarCalcFunctionList()
 
 ScFunctionMgr* ScGlobal::GetStarCalcFunctionMgr()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pStarCalcFunctionMgr )
         pStarCalcFunctionMgr = new ScFunctionMgr;
 
@@ -681,6 +694,7 @@ void ScGlobal::ResetFunctionList()
 
 ScUnitConverter* ScGlobal::GetUnitConverter()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pUnitConverter )
         pUnitConverter = new ScUnitConverter;
 
@@ -994,6 +1008,7 @@ void ScGlobal::AddLanguage( SfxItemSet& rSet, const SvNumberFormatter& rFormatte
 
 utl::TransliterationWrapper* ScGlobal::GetpTransliteration()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pTransliteration )
     {
         const LanguageType eOfficeLanguage = Application::GetSettings().GetLanguageTag().getLanguageType();
@@ -1016,6 +1031,7 @@ const LocaleDataWrapper* ScGlobal::GetpLocaleData()
 }
 CalendarWrapper*     ScGlobal::GetCalendar()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pCalendar )
     {
         pCalendar = new CalendarWrapper( ::comphelper::getProcessComponentContext() );
@@ -1045,6 +1061,7 @@ CollatorWrapper*        ScGlobal::GetCaseCollator()
 }
 ::utl::TransliterationWrapper* ScGlobal::GetCaseTransliteration()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pCaseTransliteration )
     {
         const LanguageType eOfficeLanguage = Application::GetSettings().GetLanguageTag().getLanguageType();
@@ -1055,6 +1072,7 @@ CollatorWrapper*        ScGlobal::GetCaseCollator()
 }
 css::lang::Locale*     ScGlobal::GetLocale()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if ( !pLocale )
     {
         pLocale = new css::lang::Locale( Application::GetSettings().GetLanguageTag().getLocale());
@@ -1064,6 +1082,7 @@ css::lang::Locale*     ScGlobal::GetLocale()
 
 ScFieldEditEngine& ScGlobal::GetStaticFieldEditEngine()
 {
+    assert(!bThreadedGroupCalcInProgress);
     if (!pFieldEditEngine)
     {
         // Creating a ScFieldEditEngine with pDocument=NULL leads to document
commit 6a1a0ce97b56d869969d601e75995dca9fc6fd50
Author: Luboš Luňák <l.lunak at collabora.com>
Date:   Wed May 30 09:22:03 2018 +0200

    move mbThreadedGroupCalcInProgress from ScDocument to ScGlobal
    
    Some code, such as statics in ScGlobal, cannot(?) easily access ScDocument.
    Moreover whether Calc is currently performing threaded calculations
    is not really a state of a document but of Calc itself. Still keep
    the state accessible via ScDocument for the pipe-dream future where it
    possibly really will be a state of the document.
    
    Change-Id: I2b112221e7fa1b2b4469cfd289fc201025585a5f
    Reviewed-on: https://gerrit.libreoffice.org/54796
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/55277
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
    Tested-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index 77dfca451d2a..ab80510ac1b4 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -434,8 +434,6 @@ private:
     css::uno::Reference< css::script::vba::XVBAEventProcessor >
                         mxVbaEvents;
 public:
-    bool                mbThreadedGroupCalcInProgress;
-
     /// list of ScInterpreterTableOpParams currently in use
     std::vector<std::unique_ptr<ScInterpreterTableOpParams>> m_TableOpList;
     ScInterpreterTableOpParams  aLastTableOpParams;     // remember last params
@@ -574,6 +572,8 @@ public:
         maInterpreterContext.mpFormatter = GetFormatTable();
         return maInterpreterContext;
     }
+    void SetThreadedGroupCalcInProgress( bool set ) { (void)this; ScGlobal::bThreadedGroupCalcInProgress = set; }
+    bool IsThreadedGroupCalcInProgress() const { (void)this; return ScGlobal::bThreadedGroupCalcInProgress; }
 
     SC_DLLPUBLIC sfx2::LinkManager*       GetLinkManager();
     SC_DLLPUBLIC const sfx2::LinkManager* GetLinkManager() const;
@@ -2215,26 +2215,26 @@ public:
 
     void                IncInterpretLevel()
                             {
-                                assert(!mbThreadedGroupCalcInProgress);
+                                assert(!IsThreadedGroupCalcInProgress());
                                 if ( nInterpretLevel < USHRT_MAX )
                                     nInterpretLevel++;
                             }
     void                DecInterpretLevel()
                             {
-                                assert(!mbThreadedGroupCalcInProgress);
+                                assert(!IsThreadedGroupCalcInProgress());
                                 if ( nInterpretLevel )
                                     nInterpretLevel--;
                             }
     sal_uInt16          GetMacroInterpretLevel() { return nMacroInterpretLevel; }
     void                IncMacroInterpretLevel()
                             {
-                                assert(!mbThreadedGroupCalcInProgress);
+                                assert(!IsThreadedGroupCalcInProgress());
                                 if ( nMacroInterpretLevel < USHRT_MAX )
                                     nMacroInterpretLevel++;
                             }
     void                DecMacroInterpretLevel()
                             {
-                                assert(!mbThreadedGroupCalcInProgress);
+                                assert(!IsThreadedGroupCalcInProgress());
                                 if ( nMacroInterpretLevel )
                                     nMacroInterpretLevel--;
                             }
diff --git a/sc/inc/global.hxx b/sc/inc/global.hxx
index bbdd26d9d38a..bb63731c1a12 100644
--- a/sc/inc/global.hxx
+++ b/sc/inc/global.hxx
@@ -806,6 +806,8 @@ public:
             FormulaError & rError, FormulaError nStringNoValueError,
             SvNumberFormatter* pFormatter, SvNumFormatType & rCurFmtType );
 
+    /// Calc's threaded group calculation is in progress.
+    static bool bThreadedGroupCalcInProgress;
 };
 
 // maybe move to dbdata.hxx (?):
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 79654808a14f..3c856fd161cd 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -2912,7 +2912,7 @@ void ScColumn::SetFormulaResults( SCROW nRow, const formula::FormulaConstTokenRe
 
 void ScColumn::CalculateInThread( ScInterpreterContext& rContext, SCROW nRow, size_t nLen, unsigned nThisThread, unsigned nThreadsTotal)
 {
-    assert(GetDoc()->mbThreadedGroupCalcInProgress);
+    assert(GetDoc()->IsThreadedGroupCalcInProgress());
 
     sc::CellStoreType::position_type aPos = maCells.position(nRow);
     sc::CellStoreType::iterator it = aPos.first;
diff --git a/sc/source/core/data/documen2.cxx b/sc/source/core/data/documen2.cxx
index ba48bff57ced..dd807c3c24fd 100644
--- a/sc/source/core/data/documen2.cxx
+++ b/sc/source/core/data/documen2.cxx
@@ -169,7 +169,6 @@ ScDocument::ScDocument( ScDocumentMode eMode, SfxObjectShell* pDocShell ) :
         nUnoObjectId( 0 ),
         nRangeOverflowType( 0 ),
         aCurTextWidthCalcPos(MAXCOL,0,0),
-        mbThreadedGroupCalcInProgress( false ),
         nFormulaCodeInTree(0),
         nXMLImportedFormulaCount( 0 ),
         nInterpretLevel(0),
@@ -482,7 +481,7 @@ void ScDocument::InitClipPtrs( ScDocument* pSourceDoc )
 
 SvNumberFormatter* ScDocument::GetFormatTable() const
 {
-    assert(!mbThreadedGroupCalcInProgress);
+    assert(!IsThreadedGroupCalcInProgress());
     return mxPoolHelper->GetFormTable();
 }
 
@@ -1199,7 +1198,7 @@ ScRecursionHelper* ScDocument::CreateRecursionHelperInstance()
 ScLookupCache & ScDocument::GetLookupCache( const ScRange & rRange )
 {
     ScLookupCache* pCache = nullptr;
-    if (!mbThreadedGroupCalcInProgress)
+    if (!IsThreadedGroupCalcInProgress())
     {
         if (!maNonThreaded.pLookupCacheMapImpl)
             maNonThreaded.pLookupCacheMapImpl = new ScLookupCacheMapImpl;
@@ -1230,7 +1229,7 @@ ScLookupCache & ScDocument::GetLookupCache( const ScRange & rRange )
 
 void ScDocument::AddLookupCache( ScLookupCache & rCache )
 {
-    if (!mbThreadedGroupCalcInProgress)
+    if (!IsThreadedGroupCalcInProgress())
     {
         if (!maNonThreaded.pLookupCacheMapImpl->aCacheMap.insert( ::std::pair< const ScRange,
                 ScLookupCache*>(rCache.getRange(), &rCache)).second)
@@ -1254,7 +1253,7 @@ void ScDocument::AddLookupCache( ScLookupCache & rCache )
 
 void ScDocument::RemoveLookupCache( ScLookupCache & rCache )
 {
-    if (!mbThreadedGroupCalcInProgress)
+    if (!IsThreadedGroupCalcInProgress())
     {
         auto it(maNonThreaded.pLookupCacheMapImpl->aCacheMap.find(rCache.getRange()));
         if (it == maNonThreaded.pLookupCacheMapImpl->aCacheMap.end())
@@ -1286,7 +1285,7 @@ void ScDocument::RemoveLookupCache( ScLookupCache & rCache )
 
 void ScDocument::ClearLookupCaches()
 {
-    if (!mbThreadedGroupCalcInProgress)
+    if (!IsThreadedGroupCalcInProgress())
     {
         if (maNonThreaded.pLookupCacheMapImpl )
             maNonThreaded.pLookupCacheMapImpl->clear();
diff --git a/sc/source/core/data/documen8.cxx b/sc/source/core/data/documen8.cxx
index 53866ade7701..293eb556d676 100644
--- a/sc/source/core/data/documen8.cxx
+++ b/sc/source/core/data/documen8.cxx
@@ -433,12 +433,12 @@ const ScDocumentThreadSpecific& ScDocument::CalculateInColumnInThread( ScInterpr
     if (!pTab)
         return maNonThreaded;
 
-    assert(mbThreadedGroupCalcInProgress);
+    assert(IsThreadedGroupCalcInProgress());
 
     maThreadSpecific.SetupFromNonThreadedData(maNonThreaded);
     pTab->CalculateInColumnInThread(rContext, rTopPos.Col(), rTopPos.Row(), nLen, nThisThread, nThreadsTotal);
 
-    assert(mbThreadedGroupCalcInProgress);
+    assert(IsThreadedGroupCalcInProgress());
 
     return maThreadSpecific;
 }
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index ad3c6252965c..5b81a6a1b09f 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -6729,7 +6729,7 @@ thread_local ScDocumentThreadSpecific ScDocument::maThreadSpecific;
 
 ScRecursionHelper& ScDocument::GetRecursionHelper()
 {
-    if (!mbThreadedGroupCalcInProgress)
+    if (!IsThreadedGroupCalcInProgress())
     {
         if (!maNonThreaded.pRecursionHelper)
             maNonThreaded.pRecursionHelper = CreateRecursionHelperInstance();
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index daaa80e5b921..1bec26aab99a 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -1529,7 +1529,7 @@ void ScFormulaCell::Interpret()
     {
         pDocument->IncInterpretLevel();
 
-        bool bCheckForFGCycle = mxGroup && !pDocument->mbThreadedGroupCalcInProgress &&
+        bool bCheckForFGCycle = mxGroup && !pDocument->IsThreadedGroupCalcInProgress() &&
             mxGroup->meCalcState == sc::GroupCalcEnabled &&
             ScCalcConfig::isThreadingEnabled();
         bool bPopFormulaGroup = false;
@@ -2108,7 +2108,7 @@ void ScFormulaCell::InterpretTail( ScInterpreterContext& rContext, ScInterpretTa
             // a changed result must still reset the stream flag
             pDocument->SetStreamValid(aPos.Tab(), false, true);
         }
-        if ( !pDocument->mbThreadedGroupCalcInProgress && !pCode->IsRecalcModeAlways() )
+        if ( !pDocument->IsThreadedGroupCalcInProgress() && !pCode->IsRecalcModeAlways() )
             pDocument->RemoveFromFormulaTree( this );
 
         //  FORCED cells also immediately tested for validity (start macro possibly)
@@ -2127,7 +2127,7 @@ void ScFormulaCell::InterpretTail( ScInterpreterContext& rContext, ScInterpretTa
         }
 
         // Reschedule slows the whole thing down considerably, thus only execute on percent change
-        if (!pDocument->mbThreadedGroupCalcInProgress)
+        if (!pDocument->IsThreadedGroupCalcInProgress())
         {
             ScProgress *pProgress = ScProgress::GetInterpretProgress();
             if (pProgress && pProgress->Enabled())
@@ -2594,7 +2594,7 @@ void ScFormulaCell::MaybeInterpret()
 {
     if (NeedsInterpret())
     {
-        assert(!pDocument->mbThreadedGroupCalcInProgress);
+        assert(!pDocument->IsThreadedGroupCalcInProgress());
         Interpret();
     }
 }
@@ -4478,8 +4478,8 @@ bool ScFormulaCell::InterpretFormulaGroup()
         SAL_INFO("sc.threaded", "Running " << nThreadCount << " threads");
 
         {
-            assert(!pDocument->mbThreadedGroupCalcInProgress);
-            pDocument->mbThreadedGroupCalcInProgress = true;
+            assert(!pDocument->IsThreadedGroupCalcInProgress());
+            pDocument->SetThreadedGroupCalcInProgress(true);
 
             ScMutationDisable aGuard(pDocument, ScMutationGuardFlags::CORE);
 
@@ -4493,7 +4493,7 @@ bool ScFormulaCell::InterpretFormulaGroup()
             SAL_INFO("sc.threaded", "Joining threads");
             rThreadPool.waitUntilDone(aTag);
 
-            pDocument->mbThreadedGroupCalcInProgress = false;
+            pDocument->SetThreadedGroupCalcInProgress(false);
 
             SAL_INFO("sc.threaded", "Done");
         }
diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index 4ab08a7bc1bd..531619a101a5 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -125,6 +125,8 @@ SfxViewShell* pScActiveViewShell = nullptr; //FIXME: Make this a member
 sal_uInt16 nScClickMouseModifier = 0;    //FIXME: This too
 sal_uInt16 nScFillModeMouseModifier = 0; //FIXME: And this
 
+bool ScGlobal::bThreadedGroupCalcInProgress = false;
+
 // Thread-safe singleton creation. Ideally rtl_Instance should be used, but that one doesn't
 // allow accessing the pointer (so ScGlobal::Clear() cannot free the objects). So this function
 // is basically rtl_Instance::create() that uses a given pointer.


More information about the Libreoffice-commits mailing list