ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

SwarmSolver: always unlock the controllers when solve returns

Browse Source 
solve locked the document controllers and only unlocked them at the
very end. Any of the cell accesses in between can throw, and the
unlock was then skipped, leaving the document locked for good.

Unlock from a scope guard so it runs on every exit, including an
exception.

Add a test that points a variable at a sheet that does not exist, so a
cell access throws, and checks the controllers are unlocked
afterwards.

Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: I8fc123ba5fe85dee0ec26119423a1c26f0c0ba4b
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/4055
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
(cherry picked from commit 78a71ff46d077dbe76643b3a2275834cf0f556f1)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208440
Tested-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
This commit is contained in:
Tomaž Vajngerl
2026-07-22 21:52:13 +02:00
committed by Xisco Fauli
parent eb0bacce10
commit 0209dc2476
2 changed files with 50 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sc/qa/unit/SwarmSolverTest.cxx
+45
View File
@@ -13,6 +13,7 @@
#include <com/sun/star/sheet/XSpreadsheetDocument.hpp>
#include <com/sun/star/sheet/XSpreadsheet.hpp>
#include <com/sun/star/beans/XPropertySet.hpp>
#include <com/sun/star/frame/XModel.hpp>
#include <test/unoapi_test.hxx>
@@ -78,6 +79,7 @@ class SwarmSolverTest : public UnoApiTest
void testContradictoryBoundsTerminate();
void testUnboundedIntegerVariable();
void testRepeatedSolveResetsState();
void testControllersUnlockedAfterError();
public:
SwarmSolverTest()
@@ -101,6 +103,7 @@ public:
CPPUNIT_TEST(testContradictoryBoundsTerminate);
CPPUNIT_TEST(testUnboundedIntegerVariable);
CPPUNIT_TEST(testRepeatedSolveResetsState);
CPPUNIT_TEST(testControllersUnlockedAfterError);
CPPUNIT_TEST_SUITE_END();
};
@@ -796,6 +799,48 @@ void SwarmSolverTest::testRepeatedSolveResetsState()
CPPUNIT_ASSERT_DOUBLES_EQUAL(3.0, aSolution[0], 1E-5);
}
void SwarmSolverTest::testControllersUnlockedAfterError()
{
// Regression: solve locked the document controllers and only unlocked them at
// the end. A cell access in between could throw, leaving the document locked
// for good. Point a variable at a sheet that does not exist so a cell access
// throws, then check the controllers are unlocked again.
loadFromFile(u"Simple.ods");
uno::Reference<sheet::XSpreadsheetDocument> xDocument(mxComponent, uno::UNO_QUERY_THROW);
uno::Reference<frame::XModel> xModel(xDocument, uno::UNO_QUERY_THROW);
uno::Reference<sheet::XSolver> xSolver;
xSolver.set(m_xContext->getServiceManager()->createInstanceWithContext(
u"com.sun.star.comp.Calc.SwarmSolver"_ustr, m_xContext),
uno::UNO_QUERY_THROW);
table::CellAddress aObjective(0, 1, 1);
// sheet index 99 does not exist, so reading or writing this cell throws
uno::Sequence<table::CellAddress> aVariables{ { 99, 1, 0 } };
xSolver->setDocument(xDocument);
xSolver->setObjective(aObjective);
xSolver->setVariables(aVariables);
xSolver->setConstraints({});
xSolver->setMaximize(false);
bool bThrew = false;
try
{
xSolver->solve();
}
catch (const uno::Exception&)
{
bThrew = true;
}
CPPUNIT_ASSERT(bThrew);
CPPUNIT_ASSERT_MESSAGE("Controllers must be unlocked after a failed solve",
!xModel->hasControllersLocked());
}
CPPUNIT_TEST_SUITE_REGISTRATION(SwarmSolverTest);
}
sc/source/solver/SwarmSolver.cxx
+5 -2
View File
@@ -27,6 +27,7 @@
#include <cppuhelper/supportsservice.hxx>
#include <comphelper/propcontainerimplhelper.hxx>
#include <comphelper/scopeguard.hxx>
#include <cmath>
#include <vector>
@@ -586,6 +587,10 @@ void SAL_CALL SwarmSolver::solve()
xModel->lockControllers();
// Unlock again on any exit from here on, including an exception thrown by
// one of the cell accesses below, so the document is always left usable.
comphelper::ScopeGuard aUnlockGuard([&xModel] { xModel->unlockControllers(); });
if (mbNonNegative)
{
for (Bound& rBound : maBounds)
@@ -653,8 +658,6 @@ void SAL_CALL SwarmSolver::solve()
// reaching a feasible point.
bool bFeasible = isSolutionFeasible(aSolution);
xModel->unlockControllers();
mbSuccess = bFeasible;
if (bFeasible)