sc/inc/formulacell.hxx | 3 +++ sc/source/core/data/conditio.cxx | 4 ++++ sc/source/core/data/formulacell.cxx | 32 ++++++++++++++++++++++++++++++-- vcl/source/window/paint.cxx | 9 ++------- 4 files changed, 39 insertions(+), 9 deletions(-)
New commits: commit ce8a7278e1304f7aaa65bce34aeeda5e83b231f1 Author: Eike Rathke <er...@redhat.com> AuthorDate: Tue Oct 5 20:04:19 2021 +0200 Commit: Eike Rathke <er...@redhat.com> CommitDate: Tue Oct 5 22:01:58 2021 +0200 Fix crash if conditional format triggers recursion with iterations enabled Could occur if a conditional format is evaluated (for example if row height is to be obtained) while a formula cell it references is still running and iterations are enabled so the conditional format's temporary formula cell is added to the iteration recursion list but iterations are not triggered if there are no circular references. In that case the temporary formula cell's pointer remained in the recursion list and it's dangling instance was accessed in the next round. Mark such formula cell as free-flying and remove from recursion list if it was added. Observed at https://ask.libreoffice.org/t/lo-calc-crashes-when-calling-a-macro/68800 with the original attached file that now got replaced with another version that doesn't have iterations enabled so wouldn't trigger the bug (and apparently even doesn't if enabling iterations). Change-Id: I23a023356f920b8413874cab14acdc8b25580052 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123115 Reviewed-by: Eike Rathke <er...@redhat.com> Tested-by: Jenkins diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx index 9ad2ba7b16f7..fd57bde39976 100644 --- a/sc/inc/formulacell.hxx +++ b/sc/inc/formulacell.hxx @@ -137,6 +137,7 @@ private: bool mbPostponedDirty : 1; // if cell needs to be set dirty later bool mbIsExtRef : 1; // has references in ScExternalRefManager; never cleared after set bool mbSeenInPath : 1; // For detecting cycle involving formula groups and singleton formulacells + bool mbFreeFlying : 1; // Cell is out of sheets interpreted, like in conditional format ScMatrixMode cMatrixFlag : 8; sal_uInt16 nSeenInIteration : 16; // Iteration cycle in which the cell was last encountered SvNumFormatType nFormatType : 16; @@ -222,6 +223,8 @@ public: ScFormulaCell(const ScFormulaCell& rCell, ScDocument& rDoc, const ScAddress& rPos, ScCloneFlags nCloneFlags = ScCloneFlags::Default); + void SetFreeFlying( bool b ) { mbFreeFlying = b; } + size_t GetHash() const; void GetFormula( OUString& rFormula, diff --git a/sc/source/core/data/conditio.cxx b/sc/source/core/data/conditio.cxx index 008592410a75..8707e7eb4c67 100644 --- a/sc/source/core/data/conditio.cxx +++ b/sc/source/core/data/conditio.cxx @@ -401,6 +401,7 @@ void ScConditionEntry::MakeCells( const ScAddress& rPos ) // pFCell1 will hold a flat-copied ScTokenArray sharing ref-counted // code tokens with pFormula1 pFCell1.reset( new ScFormulaCell(*mpDoc, rPos, *pFormula1) ); + pFCell1->SetFreeFlying(true); pFCell1->StartListeningTo( *mpDoc ); } @@ -409,6 +410,7 @@ void ScConditionEntry::MakeCells( const ScAddress& rPos ) // pFCell2 will hold a flat-copied ScTokenArray sharing ref-counted // code tokens with pFormula2 pFCell2.reset( new ScFormulaCell(*mpDoc, rPos, *pFormula2) ); + pFCell2->SetFreeFlying(true); pFCell2->StartListeningTo( *mpDoc ); } } @@ -653,6 +655,7 @@ void ScConditionEntry::Interpret( const ScAddress& rPos ) { pTemp1.reset(pFormula1 ? new ScFormulaCell(*mpDoc, rPos, *pFormula1) : new ScFormulaCell(*mpDoc, rPos)); pEff1 = pTemp1.get(); + pEff1->SetFreeFlying(true); } if ( pEff1 ) { @@ -683,6 +686,7 @@ void ScConditionEntry::Interpret( const ScAddress& rPos ) { pTemp2.reset(pFormula2 ? new ScFormulaCell(*mpDoc, rPos, *pFormula2) : new ScFormulaCell(*mpDoc, rPos)); pEff2 = pTemp2.get(); + pEff2->SetFreeFlying(true); } if ( pEff2 ) { diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx index 1600a1248141..254f4bcee1d7 100644 --- a/sc/source/core/data/formulacell.cxx +++ b/sc/source/core/data/formulacell.cxx @@ -599,6 +599,7 @@ ScFormulaCell::ScFormulaCell( ScDocument& rDoc, const ScAddress& rPos ) : mbPostponedDirty(false), mbIsExtRef(false), mbSeenInPath(false), + mbFreeFlying(false), cMatrixFlag(ScMatrixMode::NONE), nSeenInIteration(0), nFormatType(SvNumFormatType::NUMBER), @@ -631,6 +632,7 @@ ScFormulaCell::ScFormulaCell( ScDocument& rDoc, const ScAddress& rPos, mbPostponedDirty(false), mbIsExtRef(false), mbSeenInPath(false), + mbFreeFlying(false), cMatrixFlag ( cMatInd ), nSeenInIteration(0), nFormatType ( SvNumFormatType::NUMBER ), @@ -666,6 +668,7 @@ ScFormulaCell::ScFormulaCell( mbPostponedDirty(false), mbIsExtRef(false), mbSeenInPath(false), + mbFreeFlying(false), cMatrixFlag ( cMatInd ), nSeenInIteration(0), nFormatType ( SvNumFormatType::NUMBER ), @@ -718,6 +721,7 @@ ScFormulaCell::ScFormulaCell( mbPostponedDirty(false), mbIsExtRef(false), mbSeenInPath(false), + mbFreeFlying(false), cMatrixFlag ( cMatInd ), nSeenInIteration(0), nFormatType ( SvNumFormatType::NUMBER ), @@ -767,6 +771,7 @@ ScFormulaCell::ScFormulaCell( mbPostponedDirty(false), mbIsExtRef(false), mbSeenInPath(false), + mbFreeFlying(false), cMatrixFlag ( cInd ), nSeenInIteration(0), nFormatType(xGroup->mnFormatType), @@ -798,6 +803,7 @@ ScFormulaCell::ScFormulaCell(const ScFormulaCell& rCell, ScDocument& rDoc, const mbPostponedDirty(false), mbIsExtRef(false), mbSeenInPath(false), + mbFreeFlying(false), cMatrixFlag ( rCell.cMatrixFlag ), nSeenInIteration(0), nFormatType( rCell.nFormatType ), @@ -1627,9 +1633,12 @@ bool ScFormulaCell::Interpret(SCROW nStartOffset, SCROW nEndOffset) // recursion list in reverse order. if (rRecursionHelper.IsInReturn()) { - if (rRecursionHelper.GetRecursionCount() > 0 || - !rRecursionHelper.IsDoingRecursion()) + bool bFreeFlyingInserted = false; + if (rRecursionHelper.GetRecursionCount() > 0 || !rRecursionHelper.IsDoingRecursion()) + { rRecursionHelper.Insert( this, bOldRunning, aResult); + bFreeFlyingInserted = mbFreeFlying; + } bool bIterationFromRecursion = false; bool bResumeIteration = false; do @@ -1849,6 +1858,25 @@ bool ScFormulaCell::Interpret(SCROW nStartOffset, SCROW nEndOffset) rRecursionHelper.Clear(); } } while (bIterationFromRecursion || bResumeIteration); + + if (bFreeFlyingInserted) + { + // Remove this from recursion list, it may get deleted. + // It additionally also should mean that the recursion/iteration + // ends here as it must had been triggered by this free-flying + // out-of-sheets cell + /* TODO: replace by a simple rRecursionHelper.EndIteration() call + * if the assertions hold. */ + const bool bOnlyThis = (rRecursionHelper.GetList().size() == 1); + assert(bOnlyThis); + rRecursionHelper.GetList().remove_if([this](const ScFormulaRecursionEntry& r){return r.pCell == this;}); + if (bOnlyThis) + { + assert(rRecursionHelper.GetList().empty()); + if (rRecursionHelper.GetList().empty()) + rRecursionHelper.EndIteration(); + } + } } #if DEBUG_CALCULATION commit 057968bbce406efe6564347329df45b7e0f823ce Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Tue Oct 5 14:30:44 2021 +0200 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Tue Oct 5 22:01:48 2021 +0200 do not block out PaintImmediately() in LOK mode I'm not sure why I did this when disabling LOK idle painting, even if a window is not to be painted to, it still needs at least invalidating. Change-Id: Iaed6b1071d05d05d6bf5195f3803afb1fc018508 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123097 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lu...@collabora.com> diff --git a/vcl/source/window/paint.cxx b/vcl/source/window/paint.cxx index 291b82094c46..1c983ff7b37a 100644 --- a/vcl/source/window/paint.cxx +++ b/vcl/source/window/paint.cxx @@ -595,7 +595,8 @@ void Window::ImplCallPaint(const vcl::Region* pRegion, ImplPaintFlags nPaintFlag if (!mpWindowImpl->mpFirstChild) mpWindowImpl->mnPaintFlags &= ~ImplPaintFlags::PaintAllChildren; - if (mpWindowImpl->mbPaintDisabled) + // If tiled rendering is used, windows are only invalidated, never painted to. + if (mpWindowImpl->mbPaintDisabled || comphelper::LibreOfficeKit::isActive()) { if (mpWindowImpl->mnPaintFlags & ImplPaintFlags::PaintAll) Invalidate(InvalidateFlags::NoChildren | InvalidateFlags::NoErase | InvalidateFlags::NoTransparent | InvalidateFlags::NoClipChildren); @@ -1293,12 +1294,6 @@ void Window::PaintImmediately() if (!mpWindowImpl) return; - if (comphelper::LibreOfficeKit::isActive()) - { - // Tiled rendering is used, direct paint does not need to do anything. - return; - } - if ( mpWindowImpl->mpBorderWindow ) { mpWindowImpl->mpBorderWindow->PaintImmediately();