sc/inc/column.hxx | 2 -
sc/inc/document.hxx | 4 ++-
sc/inc/formulacell.hxx | 2 +
sc/inc/rangecache.hxx | 2 -
sc/inc/table.hxx | 2 -
sc/source/core/data/column3.cxx | 44 +++++++++++++++++++++++++------------
sc/source/core/data/documen2.cxx | 11 ++++++---
sc/source/core/data/document.cxx | 13 +++++++---
sc/source/core/data/table1.cxx | 7 ++++-
sc/source/core/tool/rangecache.cxx | 6 ++++-
10 files changed, 65 insertions(+), 28 deletions(-)
New commits:
commit d8eba60009afce4d9cca1488b4ce70e221642865
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Aug 23 19:47:50 2022 +0200
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Thu Aug 25 23:06:17 2022 +0200
try harder to ensure InterpretCellsIfNeeded() interprets (tdf#150499)
ScFormulaCell::Interpret() tries to interpret the whole formula group
(or the given range of it), but it's not guaranteed, and possibly
just the called cell will be interpreted. So if a specific range
really needs to be interpreted, handle that case.
Change-Id: I7fb563ae471eefd49e5bb6c92b6aff98c42a440e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138741
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
(cherry picked from commit e1f02b8d00272be9cbf17cb8c351445a08a4c5f4)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138814
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index e4b7eb148d9a..aa5a5c689c01 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -721,7 +721,7 @@ public:
bool IsDrawObjectsEmptyBlock(SCROW nStartRow, SCROW nEndRow) const;
void InterpretDirtyCells( SCROW nRow1, SCROW nRow2 );
- void InterpretCellsIfNeeded( SCROW nRow1, SCROW nRow2 );
+ bool InterpretCellsIfNeeded( SCROW nRow1, SCROW nRow2 );
static void JoinNewFormulaCell( const sc::CellStoreType::position_type&
aPos, ScFormulaCell& rCell );
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index 959960e15e03..b4552e792000 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -1363,7 +1363,9 @@ public:
void SetTableOpDirty( const ScRange& ); // for Interpreter
TableOp
void InterpretDirtyCells( const ScRangeList& rRanges );
// Interprets cells that have NeedsInterpret(), i.e. the same like calling
MaybeInterpret() on them.
- void InterpretCellsIfNeeded( const ScRangeList& rRanges );
+ // Returns false if some couldn't be interpreted (i.e. they still have
NeedsInterpret()).
+ // Useful to ensure that the given cells will not need interpreting.
+ bool InterpretCellsIfNeeded( const ScRangeList& rRanges );
SC_DLLPUBLIC void CalcAll();
SC_DLLPUBLIC void CalcAfterLoad( bool bStartListening = true );
void CompileAll();
diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index 013a0058ba5d..789353efce9f 100644
--- a/sc/inc/formulacell.hxx
+++ b/sc/inc/formulacell.hxx
@@ -261,6 +261,8 @@ public:
void CompileXML( sc::CompileFormulaContext& rCxt, ScProgress& rProgress );
// compile temporary string tokens
void CalcAfterLoad( sc::CompileFormulaContext& rCxt, bool bStartListening
);
bool MarkUsedExternalReferences();
+ // Returns true if the cell was interpreted as part of the formula group.
+ // The parameters may limit which subset of the formula group should be
interepreted, if possible.
bool Interpret(SCROW nStartOffset = -1, SCROW nEndOffset = -1);
bool IsIterCell() const { return bIsIterCell; }
sal_uInt16 GetSeenInIteration() const { return nSeenInIteration; }
diff --git a/sc/inc/rangecache.hxx b/sc/inc/rangecache.hxx
index 7490a570f20a..c65e8653a458 100644
--- a/sc/inc/rangecache.hxx
+++ b/sc/inc/rangecache.hxx
@@ -46,7 +46,7 @@ class ScSortedRangeCache final : public SvtListener
public:
/// MUST be new'd because Notify() deletes.
ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange, const
ScQueryParam& param,
- ScInterpreterContext* context);
+ ScInterpreterContext* context, bool invalid = false);
/// Returns if the cache is usable.
bool isValid() const { return mValid; }
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 0a3bc6c67956..35388b54ea1f 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -1057,7 +1057,7 @@ public:
void FillMatrix( ScMatrix& rMat, SCCOL nCol1, SCROW nRow1, SCCOL nCol2,
SCROW nRow2, svl::SharedStringPool* pPool ) const;
void InterpretDirtyCells( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW
nRow2 );
- void InterpretCellsIfNeeded( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW
nRow2 );
+ bool InterpretCellsIfNeeded( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW
nRow2 );
void SetFormulaResults( SCCOL nCol, SCROW nRow, const double* pResults,
size_t nLen );
diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index 6436bff414b0..424beb65e500 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -111,11 +111,12 @@ protected:
void Interpret(ScFormulaCell* p)
{
// Interpret() takes a range in a formula group, so group those
together.
- if( firstCell != nullptr && p->GetCellGroup() == p->GetCellGroup()
- && p->aPos.Row() == lastPos.Row() + 1 )
+ if( !groupCells.empty() && p->GetCellGroup() ==
groupCells.back()->GetCellGroup()
+ && p->aPos.Row() == groupCells.back()->aPos.Row() + 1 )
{
- assert( p->aPos.Tab() == lastPos.Tab() && p->aPos.Col() ==
lastPos.Col());
- lastPos = p->aPos; // Extend range.
+ assert( p->aPos.Tab() == groupCells.back()->aPos.Tab()
+ && p->aPos.Col() == groupCells.back()->aPos.Col());
+ groupCells.push_back(p); // Extend range.
return;
}
flushPending();
@@ -124,8 +125,7 @@ protected:
p->Interpret();
return;
}
- firstCell = p;
- lastPos = p->aPos;
+ groupCells.push_back(p);
}
~CellInterpreterBase()
@@ -135,14 +135,21 @@ protected:
private:
void flushPending()
{
- if(firstCell == nullptr)
+ if(groupCells.empty())
return;
- SCROW firstRow = firstCell->GetCellGroup()->mpTopCell->aPos.Row();
- firstCell->Interpret(firstCell->aPos.Row() - firstRow, lastPos.Row() -
firstRow);
- firstCell = nullptr;
+ SCROW firstRow =
groupCells.front()->GetCellGroup()->mpTopCell->aPos.Row();
+ if(!groupCells.front()->Interpret(
+ groupCells.front()->aPos.Row() - firstRow,
groupCells.back()->aPos.Row() - firstRow))
+ {
+ // Interpret() will try to group-interpret the given cell range if
possible, but if that
+ // is not possible, it will interpret just the given cell. So if
group-interpreting
+ // wasn't possible, interpret them one by one.
+ for(ScFormulaCell* cell : groupCells)
+ cell->Interpret();
+ }
+ groupCells.clear();
}
- ScFormulaCell* firstCell = nullptr;
- ScAddress lastPos;
+ std::vector<ScFormulaCell*> groupCells;
};
class DirtyCellInterpreter : public CellInterpreterBase
@@ -161,8 +168,16 @@ public:
void operator() (size_t, ScFormulaCell* p)
{
if(p->NeedsInterpret())
+ {
Interpret(p);
+ // In some cases such as circular dependencies Interpret()
+ // will not reset the dirty flag, check that in order to tell
+ // the caller that the cell range may trigger Interpret() again.
+ if(p->NeedsInterpret())
+ allInterpreted = false;
+ }
}
+ bool allInterpreted = true;
};
}
@@ -176,13 +191,14 @@ void ScColumn::InterpretDirtyCells( SCROW nRow1, SCROW
nRow2 )
sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aFunc);
}
-void ScColumn::InterpretCellsIfNeeded( SCROW nRow1, SCROW nRow2 )
+bool ScColumn::InterpretCellsIfNeeded( SCROW nRow1, SCROW nRow2 )
{
if (!GetDoc().ValidRow(nRow1) || !GetDoc().ValidRow(nRow2) || nRow1 >
nRow2)
- return;
+ return false;
NeedsInterpretCellInterpreter aFunc;
sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aFunc);
+ return aFunc.allInterpreted;
}
void ScColumn::DeleteContent( SCROW nRow, bool bBroadcast )
diff --git a/sc/source/core/data/documen2.cxx b/sc/source/core/data/documen2.cxx
index ed090885ed3b..c29025f43a37 100644
--- a/sc/source/core/data/documen2.cxx
+++ b/sc/source/core/data/documen2.cxx
@@ -89,6 +89,7 @@
#include <datamapper.hxx>
#include <drwlayer.hxx>
#include <sharedstringpoolpurge.hxx>
+#include <dociter.hxx>
#include <config_features.h>
using namespace com::sun::star;
@@ -1213,14 +1214,18 @@ ScSortedRangeCache& ScDocument::GetSortedRangeCache(
const ScRange & rRange, con
}
// Avoid recursive calls because of some cells in the range being dirty
and triggering
// interpreting, which may call into this again. Threaded calculation
makes sure
- // no cells are dirty.
+ // no cells are dirty. If some cells in the range cannot be interpreted
and remain
+ // dirty e.g. because of circular dependencies, create only an invalid
empty cache to prevent
+ // a possible recursive deadlock.
+ bool invalid = false;
if(!IsThreadedGroupCalcInProgress())
- InterpretCellsIfNeeded(rRange);
+ if(!InterpretCellsIfNeeded(rRange))
+ invalid = true;
std::unique_lock guard(mScLookupMutex);
auto [findIt, bInserted] = mxScSortedRangeCache->aCacheMap.emplace(key,
nullptr);
if (bInserted)
{
- findIt->second = std::make_unique<ScSortedRangeCache>(this, rRange,
param, pContext);
+ findIt->second = std::make_unique<ScSortedRangeCache>(this, rRange,
param, pContext, invalid);
StartListeningArea(rRange, false, findIt->second.get());
}
return *findIt->second;
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index e0c0a9e6da8a..fb38e39bcac1 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -3950,8 +3950,9 @@ void ScDocument::InterpretDirtyCells( const ScRangeList&
rRanges )
mpFormulaGroupCxt.reset();
}
-void ScDocument::InterpretCellsIfNeeded( const ScRangeList& rRanges )
+bool ScDocument::InterpretCellsIfNeeded( const ScRangeList& rRanges )
{
+ bool allInterpreted = true;
for (size_t nPos=0, nRangeCount = rRanges.size(); nPos < nRangeCount;
nPos++)
{
const ScRange& rRange = rRanges[nPos];
@@ -3959,12 +3960,16 @@ void ScDocument::InterpretCellsIfNeeded( const
ScRangeList& rRanges )
{
ScTable* pTab = FetchTable(nTab);
if (!pTab)
- return;
+ break;
- pTab->InterpretCellsIfNeeded(
- rRange.aStart.Col(), rRange.aStart.Row(), rRange.aEnd.Col(),
rRange.aEnd.Row());
+ if( !pTab->InterpretCellsIfNeeded(
+ rRange.aStart.Col(), rRange.aStart.Row(), rRange.aEnd.Col(),
rRange.aEnd.Row()))
+ {
+ allInterpreted = false;
+ }
}
}
+ return allInterpreted;
}
void ScDocument::AddTableOpFormulaCell( ScFormulaCell* pCell )
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index 0eaf10c786da..b177bd55b749 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -2599,11 +2599,14 @@ void ScTable::InterpretDirtyCells( SCCOL nCol1, SCROW
nRow1, SCCOL nCol2, SCROW
aCol[nCol].InterpretDirtyCells(nRow1, nRow2);
}
-void ScTable::InterpretCellsIfNeeded( SCCOL nCol1, SCROW nRow1, SCCOL nCol2,
SCROW nRow2 )
+bool ScTable::InterpretCellsIfNeeded( SCCOL nCol1, SCROW nRow1, SCCOL nCol2,
SCROW nRow2 )
{
nCol2 = ClampToAllocatedColumns(nCol2);
+ bool allInterpreted = true;
for (SCCOL nCol = nCol1; nCol <= nCol2; ++nCol)
- aCol[nCol].InterpretCellsIfNeeded(nRow1, nRow2);
+ if(!aCol[nCol].InterpretCellsIfNeeded(nRow1, nRow2))
+ allInterpreted = false;
+ return allInterpreted;
}
void ScTable::SetFormulaResults( SCCOL nCol, SCROW nRow, const double*
pResults, size_t nLen )
diff --git a/sc/source/core/tool/rangecache.cxx
b/sc/source/core/tool/rangecache.cxx
index 7f1e9cfe8235..e762908e219b 100644
--- a/sc/source/core/tool/rangecache.cxx
+++ b/sc/source/core/tool/rangecache.cxx
@@ -50,7 +50,8 @@ static ScSortedRangeCache::ValueType toValueType(const
ScQueryParam& param)
}
ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange,
- const ScQueryParam& param,
ScInterpreterContext* context)
+ const ScQueryParam& param,
ScInterpreterContext* context,
+ bool invalid)
: maRange(rRange)
, mpDoc(pDoc)
, mValid(false)
@@ -67,6 +68,9 @@ ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc,
const ScRange& rRange,
mQueryOp = entry.eOp;
mQueryType = item.meType;
+ if (invalid)
+ return; // leave empty
+
SCROW startRow = maRange.aStart.Row();
SCROW endRow = maRange.aEnd.Row();
SCCOL startCol = maRange.aStart.Col();
