You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: clone solution for first solution and solver started events - #2732
SolverManager passed the solver's live working solution to two consumers:
withFirstInitializedSolutionConsumer(...), called when a phase first initializes the solution
withSolverJobStartedConsumer(...), called when solving starts
These consumers run on the consumer thread while the solver thread keeps changing the same instance. A consumer that reads the solution may see an inconsistent state. A consumer that modifies it corrupts the solver's working state, which can make score assertions fail (for example under PHASE_ASSERT).
Fix
Both events now get a planning clone of the working solution, made via the new SolverScope#cloneWorkingSolution() / AbstractPhaseScope#cloneWorkingSolution().
The clone is made only if the matching consumer is registered. ConsumerSupport takes a Supplier<Solution_> and returns early when there is no consumer, so a plain solve(id, problem) does no extra cloning.
The supplier is called on the solver thread, before the event is handed to the consumer thread. That way the consumer thread never reads the working solution, not even to clone it.
This empty catch violates CONSTITUTION.md:66-70 and can let an interrupted test bypass the synchronization that makes the clone assertion meaningful, so rethrow the interruption with context.
Swallowing this interruption violates CONSTITUTION.md:68 and can let the test proceed without observing the consumer mutation, so rethrow it with context as the preceding test does.
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but before we merge, let's make sure the new tests are not flaky. Let's make sure we've seen each run 100_000 times in a row. @RepeatedTest is your friend.
(I am asking for this, because historically listener-based tests have been the most unreliable of all our tests.)
LGTM, but before we merge, let's make sure the new tests are not flaky. Let's make sure we've seen each run 100_000 times in a row. @RepeatedTest is your friend.
(I am asking for this, because historically listener-based tests have been the most unreliable of all our tests.)
Running the related tests 100,000 times produced no errors.
getInitialSolution() returns the caller's untouched input, whose shadow variables may be stale. The score director explicitly updates shadows only on its cloned working solution (AbstractScoreDirector.java:343-354), so this changes SolverJobStartedEvent from exposing that consistent working state to exposing stale or null shadows. Supply a planning clone of the current working solution here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
triceo
left a comment
Problem
SolverManagerpassed the solver's live working solution to two consumers:withFirstInitializedSolutionConsumer(...), called when a phase first initializes the solutionwithSolverJobStartedConsumer(...), called when solving startsThese consumers run on the consumer thread while the solver thread keeps changing the same instance. A consumer that reads the solution may see an inconsistent state. A consumer that modifies it corrupts the solver's working state, which can make score assertions fail (for example under
PHASE_ASSERT).Fix
SolverScope#cloneWorkingSolution()/AbstractPhaseScope#cloneWorkingSolution().ConsumerSupporttakes aSupplier<Solution_>and returns early when there is no consumer, so a plainsolve(id, problem)does no extra cloning.