sc/CppunitTest_sc_parallelism.mk | 81 +++++++++++++++++++++++++++++++++
sc/Module_sc.mk | 1
sc/qa/unit/data/ods/tdf160368.ods |binary
sc/qa/unit/parallelism.cxx | 88 ++++++++++++++++++++++++++++++++++++
sc/qa/unit/ucalc_parallelism.cxx | 3 +
sc/source/core/data/formulacell.cxx | 11 +++-
6 files changed, 182 insertions(+), 2 deletions(-)
New commits:
commit 18d0b7ac865f8d905a8b9afbe56677c89b1f406c
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Fri Mar 29 10:00:30 2024 +0000
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Fri Mar 29 20:35:00 2024 +0100
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/+/165510
Reviewed-by: Michael Meeks <michael.me...@collabora.com>
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/sc/CppunitTest_sc_parallelism.mk b/sc/CppunitTest_sc_parallelism.mk
new file mode 100644
index 000000000000..f7f3cc9fa7b3
--- /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 71488d5439e6..d966be26e8dc 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 000000000000..f9da81d27846
Binary files /dev/null and b/sc/qa/unit/data/ods/tdf160368.ods differ
diff --git a/sc/qa/unit/parallelism.cxx b/sc/qa/unit/parallelism.cxx
new file mode 100644
index 000000000000..0ced71c44228
--- /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 56b849b8e058..04f17f383ede 100644
--- a/sc/qa/unit/ucalc_parallelism.cxx
+++ b/sc/qa/unit/ucalc_parallelism.cxx
@@ -16,6 +16,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 b2283f4aa8a8..16aad6fbcfb3 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -4606,10 +4606,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);
