From 0209dc24762c5b1f2df3cf4224871afc9c2f176c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Toma=C5=BE=20Vajngerl?= <tomaz.vajngerl@collabora.co.uk>
Date: Wed, 3 Jun 2026 10:57:55 +0900
Subject: [PATCH] SwarmSolver: always unlock the controllers when solve returns
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 sc/qa/unit/SwarmSolverTest.cxx   | 45 ++++++++++++++++++++++++++++++++
 sc/source/solver/SwarmSolver.cxx |  7 +++--
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/sc/qa/unit/SwarmSolverTest.cxx b/sc/qa/unit/SwarmSolverTest.cxx
index 59a0aa12fa15..0df2f5806a2d 100644
--- a/sc/qa/unit/SwarmSolverTest.cxx
+++ b/sc/qa/unit/SwarmSolverTest.cxx
@@ -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);
 }
 
diff --git a/sc/source/solver/SwarmSolver.cxx b/sc/source/solver/SwarmSolver.cxx
index 5cabdf93909d..32c8f88d73f4 100644
--- a/sc/source/solver/SwarmSolver.cxx
+++ b/sc/source/solver/SwarmSolver.cxx
@@ -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)