fix: clone the solution before sending it to the consumer

Author: zepfred

core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java

@@ -204,7 +204,7 @@ public void setMonitorTagMap(Map<String, String> monitorTagMap) {
             restartSolver = checkProblemFactChanges();
         }
         outerSolvingEnded(solverScope);
-        return solverScope.getBestSolution();
+        return solverScope.cloneBestSolution();
     }
 
     public void outerSolvingStarted(SolverScope<Solution_> solverScope) {

core/src/main/java/ai/timefold/solver/core/impl/solver/recaller/BestSolutionRecaller.java

@@ -118,19 +118,26 @@ public <Score_ extends Score<Score_>> void processWorkingSolutionDuringMove(Inne
 
     public void updateBestSolutionAndFire(SolverScope<Solution_> solverScope) {
         updateBestSolutionWithoutFiring(solverScope);
-        solverEventSupport.fireBestSolutionChanged(solverScope, solverScope.getBestSolution());
+        // We need to clone the solution again, or we may use the same instance employed by the solver scope,
+        // which is restored between solver phases.
+        // The operation won't block the solving process.
+        solverEventSupport.fireBestSolutionChanged(solverScope, solverScope.cloneBestSolution());
     }
 
     public void updateBestSolutionAndFireIfInitialized(SolverScope<Solution_> solverScope) {
         updateBestSolutionWithoutFiring(solverScope);
         if (solverScope.isBestSolutionInitialized()) {
-            solverEventSupport.fireBestSolutionChanged(solverScope, solverScope.getBestSolution());
+            // We need to clone the solution again, or we may use the same instance employed by the solver scope,
+            // which is restored between solver phases.
+            // The operation won't block the solving process.
+            solverEventSupport.fireBestSolutionChanged(solverScope, solverScope.cloneBestSolution());
         }
     }
 
     private void updateBestSolutionAndFire(SolverScope<Solution_> solverScope, InnerScore<?> bestScore,
             Solution_ bestSolution) {
         updateBestSolutionWithoutFiring(solverScope, bestScore, bestSolution);
+        // There is no need to clone the solution because the caller has already done that
         solverEventSupport.fireBestSolutionChanged(solverScope, solverScope.getBestSolution());
     }
 
@@ -144,11 +151,10 @@ private void updateBestSolutionWithoutFiring(SolverScope<Solution_> solverScope)
 
     private void updateBestSolutionWithoutFiring(SolverScope<Solution_> solverScope, InnerScore<?> bestScore,
             Solution_ bestSolution) {
-        if (bestScore.fullyAssigned()) {
-            if (!solverScope.isBestSolutionInitialized()) {
+        if (bestScore.fullyAssigned() && !solverScope.isBestSolutionInitialized()) {
                 solverScope.setStartingInitializedScore(bestScore.raw());
             }
-        }
+
         solverScope.setBestSolution(bestSolution);
         solverScope.setBestScore(bestScore);
         solverScope.setBestSolutionTimeMillis(solverScope.getClock().millis());

core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java

@@ -224,6 +224,10 @@ public Solution_ getBestSolution() {
         return bestSolution.get();
     }
 
+    public Solution_ cloneBestSolution() {
+        return scoreDirector.cloneSolution(bestSolution.get());
+    }
+
     /**
      * The {@link PlanningSolution best solution} must never be the same instance
      * as the {@link PlanningSolution working solution}, it should be a (un)changed clone.
@@ -332,7 +336,7 @@ public long getMoveEvaluationSpeed() {
 
     public void setWorkingSolutionFromBestSolution() {
         // The workingSolution must never be the same instance as the bestSolution.
-        scoreDirector.setWorkingSolution(scoreDirector.cloneSolution(getBestSolution()));
+        scoreDirector.setWorkingSolution(cloneBestSolution());
     }
 
     public SolverScope<Solution_> createChildThreadSolverScope(ChildThreadType childThreadType) {

core/src/test/java/ai/timefold/solver/core/impl/solver/BestSolutionHolderTest.java

@@ -13,14 +13,17 @@
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executors;
 
 import ai.timefold.solver.core.api.solver.Solver;
+import ai.timefold.solver.core.api.solver.SolverConfigOverride;
 import ai.timefold.solver.core.api.solver.SolverManager;
 import ai.timefold.solver.core.api.solver.change.ProblemChange;
 import ai.timefold.solver.core.api.solver.change.ProblemChangeDirector;
 import ai.timefold.solver.core.config.solver.SolverConfig;
 import ai.timefold.solver.core.config.solver.SolverManagerConfig;
+import ai.timefold.solver.core.config.solver.termination.TerminationConfig;
 import ai.timefold.solver.core.testdomain.TestdataEasyScoreCalculator;
 import ai.timefold.solver.core.testdomain.TestdataEntity;
 import ai.timefold.solver.core.testdomain.TestdataSolution;
@@ -186,6 +189,34 @@ void problemChangeBarrageIntermediateBestSolutionConsumer() throws InterruptedEx
 
     }
 
+    @Test
+    void testCloningBetweenPhases() throws ExecutionException, InterruptedException {
+        var solverConfig = new SolverConfig()
+                .withSolutionClass(TestdataSolution.class)
+                .withEntityClasses(TestdataEntity.class)
+                .withEasyScoreCalculatorClass(TestdataEasyScoreCalculator.class);
+
+        try (var solverManager = SolverManager.<TestdataSolution, UUID> create(solverConfig, new SolverManagerConfig())) {
+            var problem = TestdataSolution.generateSolution(2, 2);
+            problem.getEntityList().forEach(entity -> entity.setValue(null));
+            var countDownLatch = new CountDownLatch(1);
+            var solution = solverManager.solveBuilder()
+                    .withProblemId(UUID.randomUUID())
+                    .withProblem(problem)
+                    .withBestSolutionConsumer(
+                            intermediate -> {
+                                intermediate.getEntityList().add(new TestdataEntity("Bad Entity 3"));
+                                countDownLatch.countDown();
+                            })
+                    .withConfigOverride(new SolverConfigOverride<TestdataSolution>()
+                            .withTerminationConfig(new TerminationConfig().withMoveCountLimit(100L)))
+                    .run()
+                    .getFinalBestSolution();
+            countDownLatch.await();
+            assertThat(solution.getEntityList()).hasSize(2);
+        }
+    }
+
     private record RecordedFuture(int id, CompletableFuture<Void> future) {
 
         boolean isDone() {