Resolves: tdf#160368 crash on save after deleting sheet
to reproduce the underlying problem: data, calc, recalculate hard:
which asserts that cell I367 is dirty during parallel calc
checking the dependencies for a parallel calc is supposed to find what
cells it depends on and either: ensure they are not dirty or detect its
not possible to do the parallel calc
checking starts in J9 where::
J9: =SUM(H$8:H9)-SUM(I9:I$9)
J10 =SUM(H$8:H10)-SUM(I10:I$9)
for the first sum it detects that the input range is H9:H367 and checks
that for dirty results, but for the second sum it detects a range of
just I9 and the dirty I367 is not detected and the problem arises on
calculation
The code to detect the range is:
// The first row that will be referenced through the doubleref.
SCROW nFirstRefRow = bIsRef1RowRel ? aAbs.aStart.Row() + mnStartOffset : aAbs.aStart.Row();
// The last row that will be referenced through the doubleref.
SCROW nLastRefRow = bIsRef2RowRel ? aAbs.aEnd.Row() + mnEndOffset : aAbs.aEnd.Row();
where for the I9 case has nFirstRefRow true and nLastRefRow false so we
just get a range of I9:I9 instead of I9:I367.
Trying to create a doc from scratch to reproduce this case proves
tricky, so trim down the original document to the sheet and subset
of columns that can trigger it.
Change-Id: I44bfd1f6d3a3ee13db9024c5b2efa2588cc30521
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165478
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
diff --git a/sc/CppunitTest_sc_parallelism.mk b/sc/CppunitTest_sc_parallelism.mk
new file mode 100644
index 0000000..f7f3cc9
--- /dev/null
+++ b/sc/CppunitTest_sc_parallelism.mk
@@ -0,0 +1,81 @@
# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
#*************************************************************************
#
# This file is part of the LibreOffice project.
#
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#
#*************************************************************************
$(eval $(call gb_CppunitTest_CppunitTest,sc_parallelism))
$(eval $(call gb_CppunitTest_use_common_precompiled_header,sc_parallelism))
$(eval $(call gb_CppunitTest_add_exception_objects,sc_parallelism, \
sc/qa/unit/parallelism \
))
$(eval $(call gb_CppunitTest_use_externals,sc_parallelism, \
boost_headers \
mdds_headers \
libxml2 \
$(call gb_Helper_optional,OPENCL, \
clew) \
))
$(eval $(call gb_CppunitTest_use_libraries,sc_parallelism, \
basegfx \
comphelper \
cppu \
cppuhelper \
i18nlangtag \
sal \
sax \
sc \
scqahelper \
sfx \
subsequenttest \
svl \
svx \
svxcore \
test \
tl \
unotest \
utl \
vcl \
))
$(eval $(call gb_CppunitTest_set_include,sc_parallelism,\
-I$(SRCDIR)/sc/source/ui/inc \
-I$(SRCDIR)/sc/inc \
$$(INCLUDE) \
))
$(eval $(call gb_CppunitTest_use_api,sc_parallelism,\
offapi \
udkapi \
))
$(eval $(call gb_CppunitTest_use_custom_headers,sc_parallelism,\
officecfg/registry \
))
$(eval $(call gb_CppunitTest_use_sdk_api,sc_parallelism))
$(eval $(call gb_CppunitTest_use_ure,sc_parallelism))
$(eval $(call gb_CppunitTest_use_vcl,sc_parallelism))
$(eval $(call gb_CppunitTest_use_rdb,sc_parallelism,services))
$(eval $(call gb_CppunitTest_use_components,sc_parallelism))
$(eval $(call gb_CppunitTest_use_configuration,sc_parallelism))
$(eval $(call gb_CppunitTest_add_arguments,sc_parallelism, \
-env:arg-env=$(gb_Helper_LIBRARY_PATH_VAR)"$$$${$(gb_Helper_LIBRARY_PATH_VAR)+=$$$$$(gb_Helper_LIBRARY_PATH_VAR)}" \
))
# vim: set noet sw=4 ts=4:
diff --git a/sc/Module_sc.mk b/sc/Module_sc.mk
index a01c911..3281184 100644
--- a/sc/Module_sc.mk
+++ b/sc/Module_sc.mk
@@ -62,6 +62,7 @@ $(eval $(call gb_Module_add_check_targets,sc,\
CppunitTest_sc_core \
CppunitTest_sc_dataprovider \
CppunitTest_sc_cache_test \
CppunitTest_sc_parallelism \
CppunitTest_sc_shapetest \
))
endif
diff --git a/sc/qa/unit/data/ods/tdf160368.ods b/sc/qa/unit/data/ods/tdf160368.ods
new file mode 100644
index 0000000..f9da81d
--- /dev/null
+++ b/sc/qa/unit/data/ods/tdf160368.ods
Binary files differ
diff --git a/sc/qa/unit/parallelism.cxx b/sc/qa/unit/parallelism.cxx
new file mode 100644
index 0000000..0ced71c
--- /dev/null
+++ b/sc/qa/unit/parallelism.cxx
@@ -0,0 +1,88 @@
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
/*
* This file is part of the LibreOffice project.
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
#include <sal/config.h>
#include "helper/qahelper.hxx"
#include <document.hxx>
#include <formulagroup.hxx>
#include <officecfg/Office/Calc.hxx>
using namespace sc;
// test-suite suitable for loading documents to test parallelism in
// with access only to exported symbols
class ScParallelismTest : public ScModelTestBase
{
public:
ScParallelismTest()
: ScModelTestBase("sc/qa/unit/data")
{
}
virtual void setUp() override;
virtual void tearDown() override;
private:
bool getThreadingFlag() const;
void setThreadingFlag(bool bSet);
bool m_bThreadingFlagCfg;
};
bool ScParallelismTest::getThreadingFlag() const
{
return officecfg::Office::Calc::Formula::Calculation::UseThreadedCalculationForFormulaGroups::
get();
}
void ScParallelismTest::setThreadingFlag(bool bSet)
{
std::shared_ptr<comphelper::ConfigurationChanges> xBatch(
comphelper::ConfigurationChanges::create());
officecfg::Office::Calc::Formula::Calculation::UseThreadedCalculationForFormulaGroups::set(
bSet, xBatch);
xBatch->commit();
}
void ScParallelismTest::setUp()
{
ScModelTestBase::setUp();
sc::FormulaGroupInterpreter::disableOpenCL_UnitTestsOnly();
m_bThreadingFlagCfg = getThreadingFlag();
if (!m_bThreadingFlagCfg)
setThreadingFlag(true);
}
void ScParallelismTest::tearDown()
{
// Restore threading flag
if (!m_bThreadingFlagCfg)
setThreadingFlag(false);
ScModelTestBase::tearDown();
}
// Dependency range in this document was detected as I9:I9 instead of expected I9:I367
CPPUNIT_TEST_FIXTURE(ScParallelismTest, testTdf160368)
{
createScDoc("ods/tdf160368.ods");
ScDocShell* pDocSh = getScDocShell();
// without fix: ScFormulaCell::MaybeInterpret(): Assertion `!rDocument.IsThreadedGroupCalcInProgress()' failed.
pDocSh->DoHardRecalc();
}
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/sc/qa/unit/ucalc_parallelism.cxx b/sc/qa/unit/ucalc_parallelism.cxx
index 2ee5570..8996fb5 100644
--- a/sc/qa/unit/ucalc_parallelism.cxx
+++ b/sc/qa/unit/ucalc_parallelism.cxx
@@ -21,6 +21,9 @@
using namespace css;
using namespace css::uno;
// test-suite suitable for creating documents to test parallelism in
// with access to internal unexported symbols
class ScParallelismTest : public ScUcalcTestBase
{
public:
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index f278492..93d712d10 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -4607,10 +4607,17 @@ struct ScDependantsCalculator
continue;
}
SCROW nFirstRefStartRow = bIsRef1RowRel ? aAbs.aStart.Row() + mnStartOffset : aAbs.aStart.Row();
SCROW nLastRefEndRow = bIsRef2RowRel ? aAbs.aEnd.Row() + mnEndOffset : aAbs.aEnd.Row();
SCROW nFirstRefEndRow = bIsRef1RowRel ? aAbs.aStart.Row() + mnEndOffset : aAbs.aStart.Row();
SCROW nLastRefStartRow = bIsRef2RowRel ? aAbs.aEnd.Row() + mnStartOffset : aAbs.aEnd.Row();
// The first row that will be referenced through the doubleref.
SCROW nFirstRefRow = bIsRef1RowRel ? aAbs.aStart.Row() + mnStartOffset : aAbs.aStart.Row();
SCROW nFirstRefRow = std::min(nFirstRefStartRow, nLastRefStartRow);
// The last row that will be referenced through the doubleref.
SCROW nLastRefRow = bIsRef2RowRel ? aAbs.aEnd.Row() + mnEndOffset : aAbs.aEnd.Row();
SCROW nLastRefRow = std::max(nLastRefEndRow, nFirstRefEndRow);
// Number of rows to be evaluated from nFirstRefRow.
SCROW nArrayLength = nLastRefRow - nFirstRefRow + 1;
assert(nArrayLength > 0);
